fix(react): Match TanStack Router pageload against the router location - #23299
Merged
Conversation
JPeer264
requested review from
mydea and
s1gr1d
and removed request for
a team
August 11, 2026 14:12
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Contributor
size-limit report 📦
|
nicohrubec
reviewed
Aug 13, 2026
|
|
||
| await page.goto(`${BASE}/posts/456`); | ||
|
|
||
| await page.waitForTimeout(1000); |
Member
There was a problem hiding this comment.
m: this seems like it could easily flake. can we do this differently? maybe wait for the initial transaction and only then throw an error and see if we get the correct data
|
|
||
| // The scope transaction is set when the pageload span starts, and the later `updateName` in | ||
| // `onResolved` does not rewrite it — so a wrong initial match mislabelled every error for the | ||
| // whole page lifetime. This is the part of the bug the transaction name alone does not reveal. |
Member
There was a problem hiding this comment.
could we clarify this comment, e.g. what does "this is the part of the bug the transaction name alone does not reveal" mean?
Member
Author
There was a problem hiding this comment.
Hope the machinery is clearer now
Contributor
|
👋 @s1gr1d — Please review this PR when you get a chance! |
1 similar comment
Contributor
|
👋 @s1gr1d — Please review this PR when you get a chance! |
The pageload span resolved its route by passing `window.location.pathname` to `router.matchRoutes`. With a router `basepath` that pathname still carries the prefix, which the router itself never sees, so a sufficiently dynamic route absorbed the extra segments and a plausible-but-wrong route id won (`/$a/$b/$c` with `a = "app"` instead of `/posts/$postId`). The transaction name recovers on its own, since `onResolved` renames the span before it is sent. The scope transaction does not: it is pinned when the pageload span starts and never rewritten, so error events kept the wrong transaction for the whole page lifetime. The stale params from the bad match also survive on the span, because `onResolved` merges the correct ones in without clearing them. Match against the router's own `state.location` instead. Stripping the basepath by hand would have fixed only recent routers and broken the documented 1.64.0 minimum, where `matchRoutes` is basepath-aware internally and expects the prefix to still be there. `state.location` is by construction in the form its own version expects — stripped where the router rewrites in `parseLocation`, unstripped where it does not — so no version detection is needed. It also carries an already-parsed `search`, dropping a redundant `parseSearch` call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JPeer264
force-pushed
the
jp/tanstack-pageload-basepath
branch
from
August 23, 2026 08:09
b503ac9 to
8bebcd7
Compare
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
2 participants
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.
closes #23253
closes #23253
When a
basepathis added then thewindow.location.pathnamehas the wrong information. We should actually use the routers pathname instead: https://tanstack.com/router/latest/docs/api/router/ParsedLocationTypeThis should be guaranteed to have the correct info (but still have a fallback in case something goes south). A new
basepathvariant in thetanstack-routerE2E test is now testing if the tests also work with the basepath included (plus two specific tests as regression on top).Description done by AI (and added on the commit):
The pageload span resolved its route by passing
window.location.pathnametorouter.matchRoutes. With a routerbasepaththat pathname still carries the prefix, which the router itself never sees, so a sufficiently dynamic route absorbed the extra segments and a plausible-but-wrong route id won (/$a/$b/$cwitha = "app"instead of/posts/$postId).The transaction name recovers on its own, since
onResolvedrenames the span before it is sent. The scope transaction does not: it is pinned when the pageload span starts and never rewritten, so error events kept the wrong transaction for the whole page lifetime. The stale params from the bad match also survive on the span, becauseonResolvedmerges the correct ones in without clearing them.Match against the router's own
state.locationinstead. Stripping the basepath by hand would have fixed only recent routers and broken the documented 1.64.0 minimum, wherematchRoutesis basepath-aware internally and expects the prefix to still be there.state.locationis by construction in the form its own version expects — stripped where the router rewrites inparseLocation, unstripped where it does not — so no version detection is needed. It also carries an already-parsedsearch, dropping a redundantparseSearchcall.