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

Fix initial routing state after match #2966

Closed
wants to merge 3 commits into from

Conversation

timdorr
Copy link
Member

@timdorr timdorr commented Jan 24, 2016

Continuation of #2965

#2883 was actually busted and had a test that didn't cover what it was supposed to.

In an ideal world we'd just use the same history and transitionManager for match() and on the client side, but the deprecation wrapping API messes with that too much, so this might be the best we can do for now.

@timdorr timdorr force-pushed the createTransitionManager-initial-state branch from 68e0e38 to 5a8b362 Compare January 24, 2016 04:07
@taion
Copy link
Contributor

taion commented Jan 24, 2016

I think I have a different approach that makes it a bit more explicit. I'm really scared of going through any of these new branches when not coming from match, because that can only ever cause bugs.

@timdorr
Copy link
Member Author

timdorr commented Jan 24, 2016

It's the same thing as your changes, just cleaned up and with an invariant check that explicitly tells you what's going on. I couldn't contribute to the last PR because you did it on a private fork, so this is my way of participating.

@taion
Copy link
Contributor

taion commented Jan 24, 2016

I know, sorry – old habit from before we fixed the Travis build not to spammily run on branches. The edge case I'm still concerned with on even 5a8b362 is the case where somehow a user passes in a transitionManager but not a history, or a transitionManager plus a routing history or something like that, so I changed it to just go from taion@6ab8316 and move the router objects into a different opaque prop, but I didn't want to clobber your commits.

@timdorr timdorr closed this Jan 24, 2016
@timdorr timdorr deleted the createTransitionManager-initial-state branch January 24, 2016 16:36
@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants