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

Browser back navigation does not work when using replaceState with React enabled #19839

Open
eberleant opened this issue Aug 27, 2024 · 8 comments · May be fixed by #19985
Open

Browser back navigation does not work when using replaceState with React enabled #19839

eberleant opened this issue Aug 27, 2024 · 8 comments · May be fixed by #19985
Assignees
Labels
BFP Bugfix priority, also known as Warranty bug Impact: High routing Severity: Major

Comments

@eberleant
Copy link

Description of the bug

If the current page used replaceState to modify the page history in an afterNavigation observer (for example, to add a query parameter), page history gets messed up. This causes back navigation using the browser back button to go back to 2 pages ago, instead of one page ago.

This worked in v23 but is broken in v24. A workaround is to disable React.

Expected behavior

Back navigation should only go back one page.

Minimal reproducible example

react-router-back-nav.zip

Run using mvn jetty:run and open localhost:8080.

Versions

  • Vaadin / Flow version: 24.4.10
@mshabarov mshabarov added bug Impact: High Severity: Major routing BFP Bugfix priority, also known as Warranty labels Aug 28, 2024
@tepi tepi self-assigned this Aug 28, 2024
@tepi
Copy link
Contributor

tepi commented Aug 28, 2024

The issue was assigned to a developer and is currently being investigated

@tepi
Copy link
Contributor

tepi commented Aug 29, 2024

Tested the given example. Looks like it is working as intended, since replaceState, as it's name suggests, replaces the URL in history without adding anything to the history. Changing replaceState to pushState in the example project produces expected results. Would this solution be ok for you?

The only difference between the two calls is that replaceState sends replace: true to react router, whereas pushState sends replace: false.

@tepi
Copy link
Contributor

tepi commented Aug 29, 2024

Here's a similar example from https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState. Looks like replaceState in vaadin-router has been working the wrong way all along.

Screenshot 2024-08-29 at 10 37 00

@tepi
Copy link
Contributor

tepi commented Aug 29, 2024

Never mind, I seem to have misunderstood the issue in the first place. Nevertheless, using pushState seems to produce the wanted behavior. I'll continue investigating for a fix.

@eberleant
Copy link
Author

Hi, thanks for your work on this. In the past, we've tended to used replaceState in afterNavigation observers and pushState elsewhere. replaceState was used to prevent the user from getting stuck in a navigation loop where they weren't able to navigate backwards past the page with the afterNavigation observer (ie, the back button could never take them to the page before the afterNavigation observer, due to the page history getting continually added to).

From what I can see, there's been a change in behavior from Vaadin router to React router where pushState, when used within an afterNavigation observer, no longer causes this endless navigation loop.

If that's how it's meant to work, I'm OK with that! It will actually end up making some of our code a little bit cleaner 😄

@tepi
Copy link
Contributor

tepi commented Aug 30, 2024

Thanks for your reply! I see what you mean about navigation loop, it is very annoying. Anyway, looking further there seems to be an issue here, since the first navigate call should clearly add a history entry, and the replaceState call should replace the url for that entry, and that isn't happening now.

Did not dig deeper into react-router internals yet, but right now looks like if there are multiple navigate calls to it within the same server round-trip, it does not act on all of the calls. We've also had a couple of other similar issues with it. Will continue investigating.

@tepi
Copy link
Contributor

tepi commented Sep 4, 2024

This will most likely be fixed when a new version of react-router with this fix has been released.

@tepi
Copy link
Contributor

tepi commented Sep 18, 2024

Status update: Unfortunately latest version of react-router did not fix this issue. We are currently investigating a way to fix this in Flow. Will update once we have something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFP Bugfix priority, also known as Warranty bug Impact: High routing Severity: Major
Projects
Status: ⚒️ In progress
Development

Successfully merging a pull request may close this issue.

3 participants