Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protect against exception when onpopstate/onhashchange are null #217

Closed
wants to merge 1 commit into from
Closed

Protect against exception when onpopstate/onhashchange are null #217

wants to merge 1 commit into from

Conversation

steveluscher
Copy link

Without this patch, the following will throw an exception:

router = Router(routes)
router.configure({
  notfound: function() { router.setRoute('/'); }
});
router.init();

In the early stages of page load, window.onpopstate and window.onhashchange are null (in Chrome, anyway).

Without this patch, the following will throw an exception:

    router = Router(routes)
    router.configure({
      notfound: function() { router.setRoute('/'); }
    });
    router.init();

In the early stages of page load, `window.onpopstate` and `window.onhashchange` are `null` (in Chrome, anyway).
@steveluscher
Copy link
Author

Erm… actually, this still prevents the handler for / in my example from running. What's the proper way to redirect when a route is not found?

@ogradyjd
Copy link

ogradyjd commented Feb 1, 2014

So far as I know, and I've been using director since it was sugarskull, it looks that you should just set the window.location.hash to the route you want. In this case, your code would be:

router.configure({
    notfound: function() {  window.location.hash = '' }
});

Director will pick this up in the hash onChange event and route on it.

@steveluscher
Copy link
Author

But I suppose this won't work if you're trying to use Director in html5history mode.

@ogradyjd
Copy link

ogradyjd commented Feb 2, 2014

No. as far as I know, you'd need to do something like this if you are using history:

window.history.pushState(“object or string”, “Title”, “/another-new-url”);

Considering that IE only started supporting pushState and the related methods in version 10, I'm staying away from history mode for a while longer. I'm not even sure that Director supports pushstate well enough to give you deep linking - for that, you need to change the hash with the third parameter in the above call.

@beaugunderson
Copy link
Contributor

It looks like #229 is also an attempt to fix this issue.

@steveluscher steveluscher closed this by deleting the head repository Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants