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

Improvements to history management #2238

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexspeller
Copy link

@alexspeller alexspeller commented Feb 17, 2025

Before I start going deep down the rabbithole making sure this is all cleaned up and tested etc, I'd like to get some feedback on if you're likely to accept this PR.

I've had some issues with inertia's history handling during a migration to inertia with a legacy app.

The problem is that inertia assumes that all history manipulation is handled with inertia's router.push / router.replace methods, which is not the case.

There are two ways this causes issues:

  1. When handling popstate, it assumes that the current history state should be handled by inertia. However it the state was not pushed by inertia, history.state will not contain a valid page object and should be ignored to prevent errors
  2. When preserving scroll positions, the event handler assum
    es that the url stored in currentPage is the correct current url, which it might not be if some other code has pushed a new state onto the stack in the meantime.

This causes a bug that means that if you are currently on and inertia page http://example.com/foo, and run pushState({}, "", "http://example.com/foo?bar=123"), you end up in a situation that inertia can't handle well
a. If you scroll the page, the scroll handler will pushState with the latest inertia page url it knows about, and the query param will disappear
b. If you hit the back button, inertia will try to handle the pop state event with the empty state with no inertia page in it, and will error out trying to render a component.

I would like to fix these issues to make inertia more interoperable especially during stack migrations, so please let me know if going down this path is likely to align with the project goals and if so I'll improve the handling here to make things a little smoother

Note: I'm not actually sure just an option to turn off scroll preservation makes the most sense here - it might make more sense to make the scroll preservation smarter such that it handles non-inertia history state more gracefully but still works with inertia state

@mochetts
Copy link

I had the same issue recently. 👍🏻

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.

2 participants