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 for state not updating on route change #40

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix for state not updating on route change #40

wants to merge 2 commits into from

Conversation

zackify
Copy link

@zackify zackify commented Jan 13, 2016

Turns out, this was the problem. When the route changes, the update is not being forced. Also removed RoutingContext from the readme for you.

@ChiefORZ
Copy link

👍 👍 👍

@ChiefORZ ChiefORZ mentioned this pull request Jan 14, 2016
@zackify
Copy link
Author

zackify commented Jan 14, 2016

@ryanflorence really hoping to get this merged and pushed up, I would rather not have to use a fork of this for a work demo :)

@rexxars
Copy link
Contributor

rexxars commented Jan 18, 2016

This will also fix that annoying "RoutingContext has been renamed to RouterContext."-warning - would really love to see this merged soon 👍

@zackify
Copy link
Author

zackify commented Jan 18, 2016

@rexxars @ChiefORZ I needed this and a few other unmerged PR's so I forked it and published to npm with all the latest code: https://github.com/NavJobs/async-props

@quicksnap
Copy link

Thanks @zackify!

@ryanflorence
Copy link
Owner

what does this fix?

@ryanflorence
Copy link
Owner

quick history on force.

If you click on a link, and then click on another link before the async props are done we could have two requests (or more) that are inflight simultaneously.

Imagine the props for the first request are slower than the props for the second. In these scenarios the first props land last, and then would callback and cause problems. So there's a check that the location is the same as when the request started, if the location has changed, we just ignore the data that comes back.

This change puts the race condition right back into the app.

So what exactly is this fixing?

@zackify
Copy link
Author

zackify commented Mar 8, 2016

It's been a while now. But switching pages was just not working. Nothing
would happen. Can't remember if this was due to the way routes were setup
but it was obviously not doing an update on route change and a few other
people noticed this too. Wish I could remember exactly what was
happening...
On Tue, Mar 8, 2016 at 02:28 Ryan Florence notifications@github.com wrote:

quick history on force.

If you click on a link, and then click on another link before the async
props are done we could have two requests (or more) that are inflight
simultaneously.

Imagine the props for the first request are slower than the props for the
second. In these scenarios the first props land last, and then would
callback and cause problems. So there's a check that the location is the
same as when the request started, if the location has changed, we just
ignore the data that comes back.

This change puts the race condition right back into the app.

So what exactly is this fixing?


Reply to this email directly or view it on GitHub
#40 (comment)
.

@ChiefORZ
Copy link

ChiefORZ commented Mar 8, 2016

My Issue #34 describes it in a way...Instant Callbacks - or Routes where no Props are needed - were not loading at all... Nothing happened.

@ryanflorence
Copy link
Owner

Can you folks try out 0.3.0 and see if it's still buggy in your app?

@zackify
Copy link
Author

zackify commented Mar 10, 2016

@ryanflorence so far so good, 0.3.0 is awesome! Don't need my fork anymore :D

@quicksnap
Copy link

We will update soon.. appreciate you cutting a new release--thanks @ryanflorence!

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

5 participants