fix: skip navigation hooks on hashchange #8730
Conversation
🦋 Changeset detectedLatest commit: f736b2a 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 |
Rich-Harris
changed the title
| const scroll = scroll_positions[event.state[INDEX_KEY]]; | ||
|
|
||
| // if the only change is the hash, we don't need to do anything... | ||
| if (current.url.href.split('#')[0] === location.href.split('#')[0]) { |
There was a problem hiding this comment.
Do we need to worry about the case where the old or the new (or both!) URLs don't have a # portion? Are we treating a link from /foo to /foo as a no-op? (I remember this coming up before, but don't remember what the decision was.) What happens in native browser navigation when there's a link from /foo#bar to /foo?
There was a problem hiding this comment.
Native navigation:
/foo#barto/foo(or the other way around) only resets the scroll position/footo/foodoesn't create a new history entry, so that case doesn't occur
--> maybe the solution is to usereplaceStateinupdatein case the URL is the same? Then we don't need to worry about these cases
There was a problem hiding this comment.
yep, we're in the clear — this check will work the same for /foo#x <-> /foo as for /foo#x <-> /foo#y, and we already treat /foo <-> /foo as a replaceState inside the click handler:
kit/packages/kit/src/runtime/client/client.js
Line 1465 in 1a0f82a
closes #8726
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.