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

remove replaceRootHistory #3625

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

Conversation

SteffenDE
Copy link
Collaborator

@SteffenDE SteffenDE commented Jan 10, 2025

Relates to: #3529
Relates to: #3624

replaceRootHistory was initially added by @chrismccord in 36edb48, but lost its purpose after less than three months with 04aaedc removing the only call that would actually set root: true in the history state.

Me not knowing what root: true is supposed to do reused replaceRootHistory in #3335, reintroducing the case where root: true is set in the history state. While #3335 was reverted later due to issues (#3508), I reworked the back/forward navigation problem in
#3539, which again used replaceRootHistory. As root: true would now be set in the history, we'd call replaceRootHistory on live navigation. The problem is that it was setting type: "patch" in the history, which leads to LiveView assuming that it can patch when navigating using popstate, while the actual navigation was type: "navigate". After looking into it, I don't really see a reason for replaceRootHistory to exist any more.

Relates to: #3529

`replaceRootHistory` was initially added by @josevalim in
36edb48,
but lost its purpose after less than three months with 04aaedc
removing the only call that would actually set `root: true` in the history
state.

Me not knowing what `root: true` is supposed to do reused replaceRootHistory
in #3335, reintroducing
the case where `root: true`` is set in the history state. While #3335 was reverted
later due to issues (#3508),
I reworked the back/forward navigation problem in
#3539, which again
used `replaceRootHistory`. As `root: true` would now be set in the history,
we'd call `replaceRootHistory` on live navigation. The problem is that it
was setting `type: "patch"` in the history, which leads to LiveView assuming
that it can patch when navigating using popstate, while the actual navigation
was `type: "navigate"`. After looking into it, I don't really see a reason
for replaceRootHistory to exist any more.
@SteffenDE SteffenDE force-pushed the sd-remove-replace-root-history branch from 94b2df3 to 2ef54e2 Compare January 10, 2025 13:26
@SteffenDE SteffenDE marked this pull request as ready for review January 10, 2025 19:19
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.

1 participant