Repository navigation
fix(history): stamp pushed depth from the current entry, not history.length - #695
Open
sarankumar1325 wants to merge 1 commit into
Open
sarankumar1325 wants to merge 1 commit into
sarankumar1325 wants to merge 1 commit into
Conversation
…length Browsers cap session history (50 entries in Chromium and Firefox). Once the cap is reached, pushState drops the oldest entry and history.length stays the same, so `history.length - 1` gave every new entry the depth of the one below it. Back then computed a zero delta (useBeforeLeave was skipped) and scroll restoration shared one slot between entries. browserHistory() and hashHistory() now stamp a pushed entry with the current depth + 1. Replace keeps the current depth as before. Fixes solidjs#694
🦋 Changeset detectedLatest commit: 1865ccc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #694
Problem
saveCurrentDepth()stamps an unstamped entry with_depth: history.length - 1, andbrowserHistory().set/hashHistory().setpushed the caller's state without a depth, so every router push ended up stamped that way. Browsers cap session history (50 entries in Chromium and Firefox). Once the cap is reached,pushStatedrops the oldest entry andhistory.lengthstays the same, so each new entry got the same_depthas the one below it. Back then computeddelta = 0(which skippeduseBeforeLeave), and scroll restoration put consecutive entries in the same slot.Change
src/routers/history.ts: on push, bothbrowserHistory()andhashHistory()now stamp the new entry withdepth + 1, wheredepthis the current entry's depth. If no depth is known yet, it falls back tohistory.length - 1. Replace still keeps the current depth (keepDepth). User state is kept as before, andgetSourcestill hides state that holds only_depth.test/browser-history.spec.ts: three tests. Two mockhistory.lengthat the 50-entry cap, push three times, and expect depths 49, 50 and 51 (browser and hash). The third checks that user state is kept next to the stamp. Without the change, the capped tests get[49, 49, 49].Not covered: entries the router doesn't create (for example a plain
#anchorjump handled by the browser) are still stamped bysaveCurrentDepth()fromhistory.length, the same as today.main(1.0.0) has the same pattern inset()ofRouter/HashRouterinsrc/lifecycle.ts. I can open a matching PR there if you want it backported.Testing
pnpm test(client and server configs): 720 + 96 passedpnpm run test:typesprettier --checkon the changed filesAI disclosure: I used an AI assistant to help write this change and its tests. I reviewed the diff and ran the checks above.