-
Notifications
You must be signed in to change notification settings - Fork 485
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
window.onpopstate is null error in fire() #229
base: master
Are you sure you want to change the base?
Conversation
…Route called before onpopstate is set. When fire is triggered (e.g. through setRoute) an error occurs if we haven't set onpopstate. Thus, make sure to record any fire calls, set onpopstate as soon as DOM is ready (+1 event loop), and then resume, possibly triggering a recorded fire
Did you guys have an opinion on this. Would appreciate either a merge or a comment. |
Hey @staeke, I'm helping to maintain this library now. Do you have an example of how to reproduce the undesired behavior? I can help to create a test case if so and then will work to get this merged. |
…y quickly after first router initialisation.
@beaugunderson I have also had problems caused by this so thought I'd contribute. See staeke#1 for a test that will both demonstrate the problem and the efficacy of the solution proposed by @staeke |
Unit test to demonstrate onpopstate failure when navigating very quickly
I'm encountering this in mobile Safari as well when |
I fixed this one a little differently: |
Better handling of cases where there's a race condition on start, setRoute called before onpopstate is set. When fire is triggered (e.g. through setRoute) an error occurs if we haven't set onpopstate. Thus, make sure to record any fire calls, set onpopstate as soon as DOM is ready (+1 event loop), and then resume, possibly triggering a recorded fire call.