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

yew_router::attach_route_listener Event listener provides previuos route instead of current #2116

Closed
1 of 3 tasks
Tracked by #2052
voidpumpkin opened this issue Oct 15, 2021 · 4 comments
Closed
1 of 3 tasks
Tracked by #2052
Labels
A-yew-router Area: The yew-router crate A-yew-router-macro Area: The yew-router-macro crate bug

Comments

@voidpumpkin
Copy link
Member

Problem
yew_router::attach_route_listener Event listener provides previuos route isntead of current

Originally noticed in #2109

Steps To Reproduce
Steps to reproduce the behavior:
Use this repo: https://github.com/voidpumpkin/yew-playground/tree/stale-agents
Everytime you click a link, console shows got X and X is previous route instead of current

Expected behavior
expected to get current route

Screenshots
fixed2

Environment:

  • Yew version: [master]
  • Rust version: [1.55]
  • Target, if relevant: -
  • Build tool, if relevant: -
  • OS, if relevant: -
  • Browser and version, if relevant: -

Questionnaire

  • I'm interested in fixing this myself but don't know where to start
  • I would like to fix and I have a solution
  • I don't have time to fix this right now, but maybe later
@mc1098 mc1098 added A-yew-router Area: The yew-router crate A-yew-router-macro Area: The yew-router-macro crate labels Oct 15, 2021
@futursolo
Copy link
Member

futursolo commented Oct 16, 2021

I have encountered this problem a couple days ago as well.

The root cause of this problem is that attach_route_listener is a simple event listener that listens to popstate event which the same event is used for updating the route by the router so the callback can be called before the route is updated.

This can be mitigated by using Route::recognize() before a fix is available.

This can be fixed by making current route to be provided as a context value instead of held in a thread_local!.

This is the approach that react-router is using and it also enables rendering in a non-browser environment which could also be beneficial to the upcoming SSR.

I have updated the proposal in #2113 to make <Router /> a context provider.

@voidpumpkin
Copy link
Member Author

@futursolo let me know how I can help with providing the needed changes. I saw you have most of it done in your PR?

@futursolo
Copy link
Member

@voidpumpkin Thank you.

For now I am trying to get #2107 landed so I can continue working on it as my fork has changes in this PR on master.

@voidpumpkin voidpumpkin changed the title yew_router::attach_route_listener Event listener provides previuos route isntead of current yew_router::attach_route_listener Event listener provides previuos route instead of current Oct 24, 2021
@voidpumpkin
Copy link
Member Author

Fixed by #2118

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew-router Area: The yew-router crate A-yew-router-macro Area: The yew-router-macro crate bug
Projects
None yet
Development

No branches or pull requests

3 participants