fix: a guard redirecting as the held route lands is a redirect hop - #604
Open
ryansolid wants to merge 1 commit into
Open
fix: a guard redirecting as the held route lands is a redirect hop#604ryansolid wants to merge 1 commit into
ryansolid wants to merge 1 commit into
Conversation
navigate() from render in the flush that lands a held navigation saw isPending(source) === false — the location signal has committed the destination even though nothing has painted and history.set has not run — so the hop was declared as a fresh navigation: the first destination settled `superseded`, the second `committed` in 1ms with no interaction. The integration now tracks the write between its location commit and its history commit (RouterIntegration.inflight); navigateFromRoute counts a hop when the previous navigation is pending or in flight. One navigation, one `redirects` entry, timed from the click; replace/scroll inherit as for a pending hop. Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
🦋 Changeset detectedLatest commit: b57bfcd 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
1 participant
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.
What
A
navigate()issued while the previous navigation has landed but not yet reached history is a redirect hop of that navigation, not a new one.RouterIntegration.inflight?: () => LocationChange | undefined— the integration keeps the write betweenwrite(next)and theonSettledthat callshistory.set(next), clearing it in that callback whether the write won or was replaced.navigateFromRoute: hop depth isheaded._navigationwhenisPending(source) || integration.inflight?.() === headed(was:isPending(source)only). The_navigationcounter,replace/scrollinheritance andMAX_REDIRECTSall key off the same depth, so those follow.Why
The app-authored guard shape — read the session (held), then
navigate("/login")from render once it lands — ran in the flush that lands/private. In that flushisPending(source)is alreadyfalse(the location signal has committed the value) even though nothing has painted andhistory.sethas not run. So the second write got_navigation: 1and, on the observe tier, was declared towithOriginwithoutredirect: the attribution engine recorded/privateassupersededunder the click and/loginas a separatecommittednavigation of 1ms with no interaction — the 124ms the user actually waited attributed to nothing.The existing hop test (a
redirect()thrown from a query) never hit this because it fires while the query is still pending. The router's own behaviour already treated the first destination as unrealized (onSettledskipshistory.setbecauseread() !== next); the depth logic just didn't know it.Found by the Sentry spike's record-driven adapter on next.24. Verified there: one navigation
/login,redirects: [/private],from: /,held, 124ms, under the click.Tests
New: "a guard that navigates as the held destination lands is a hop of that navigation, not a new one" (
observe-navigation.spec.tsx) — fails onnext(two records), passes here (writes 2,redirects [/private],from "/",outcome held). 402 client + 35 server tests,test:typespass.Companion engine fix from the same run: solidjs/solid#3380.