-
Notifications
You must be signed in to change notification settings - Fork 176
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
History#replaceState trigger navigation event with React router #19613
Comments
I can confirm that the following code:
cause the following issue: #19540 |
The issue was triaged and currently added to the backlog priority queue for further investigation |
@mcollovati Thanks for looking into this. Is there any chance to get this fixed? I'm stuck on the prerelease version because of this issue and unable to upgrade to the stable version :( |
We will start looking at this issue soon, but currently I cannot provide any estimate on when the bug will be fixed |
This is blocking use from upgrading as well. Is there any update? Thanks |
@alexanoid what is the latest prerelease version that works for you? Looks like we'll need to downgrade as well then for a while. |
24.4.0.beta5 |
I see an issue with the no navigation functionality shown by the example also.
we actually do not get the new path parameter to the server at all. for the query parameter change I would also expect to get it to the server, but the question is should all the events be fired (beforeEnter, beforeLeave, afterNavigation, setParameter) or should a navigation to the same target only execute beforeEnter and setParameter. I feel we should get a comprehensive list of navigation expectations to get the full set working as expected instead of making exceptions here and there for the navigation. |
As a workaround if for some reason the url update needs to happen "outside" of all handling you can just call
But note that this may break forward/back navigation. |
If I want to have all navigation events, I I would just use UI#navigate. Isn’t the whole point of the History class that it only pushes/replaces the state without firing events? |
Well it is used by the framework to handle url updates for navigation etc. so one one hand yes, but on the other hand just directly running the push and replace functions breaks the react-router and unknowingly allowing that through the api is not that good either. If only using Flow and no Hilla/react then falling back to vaadin-router for now is a possibility by setting the |
I can confirm that in my Flow application |
Add feature to support replace and push without getting a callback to the server for the change. fixes #19613
Add feature to support replace and push without getting a callback to the server for the change. fixes #19613
* feat: push and replace without navigation callback Add feature to support replace and push without getting a callback to the server for the change. fixes #19613 * change default callback for replace also to false * history gets same events as previously with vaadin-router. * Forward should callback to update url. * Fix java doc link
* feat: push and replace without navigation callback Add feature to support replace and push without getting a callback to the server for the change. fixes #19613 * change default callback for replace also to false * history gets same events as previously with vaadin-router. * Forward should callback to update url. * Fix java doc link Co-authored-by: caalador <[email protected]>
This ticket/PR has been released with Vaadin 24.4.9. |
This ticket/PR has been released with Vaadin 24.5.0. |
Description of the bug
When using React router, History#pushState and #replaceState trigger navigation event (trigger HISTORY, coming from a dom event "ui-navigate"). Even if only query parameters are changed.
With Flow Router, no navigation event occurs, not matter which part of the path has changed.
May relate to
Expected behavior
No navigation events on History#pushState and #replaceState. If a navigation event is desired, I would use UI.navigate.
Minimal reproducible example
I modified the example from a comment in #19580
Versions
The text was updated successfully, but these errors were encountered: