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

can.route.ready called multiple times #473

Closed
wants to merge 2 commits into from
Closed

Conversation

ccummings
Copy link
Contributor

can.route.ready is being called twice (or more if the user calls it themselves) which calls setState and binds to the hashChange event each time.

can.route.ready is the handler for the ready event (L503) and is then called on L508. This results in 2 bindings to the hashChange event (more if user calls can.route.ready themselves).

The other issue is that any modifications to the route before the ready event are wiped out by one of the other setState calls.

I've added a flag that only allows ready to be called once.

@ccummings
Copy link
Contributor Author

I realized my first attempt at a fix was pretty ham fisted so I've changed it to have a lighter touch.

If you are using jQuery, it will fire a ready event so we don't need to also check the readyState of the DOM. This new fix allows a user to call can.route.ready as much as they want, but it will only be called once on page load.

@daffl
Copy link
Contributor

daffl commented Sep 12, 2013

We can get that into 1.1.8 I do however still think that we never should call can.route.ready automatically for 1.2. There is so much confusion around when to use it and when not that it will be a lot easier to explain one way of doing things (if you want to start routing call can.route.read(true)).

@ccummings
Copy link
Contributor Author

+1
Absolutely agree that ready should have to be called explicitly. I'll create a new issue for that.

@justinbmeyer
Copy link
Contributor

I don't think there is going to be a 1.8. So this is probably not worth bringing in.

@ccummings ccummings deleted the route-ready-fix branch April 11, 2014 06:04
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.

None yet

3 participants