From e3cd28e581a6e151f25c0146e1d35e7cca4c147b Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Tue, 11 Aug 2026 15:46:44 +0200 Subject: [PATCH 1/2] fix(react): Match TanStack Router pageload against the router location MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../tanstack-router/package.json | 13 ++++- .../tanstack-router/src/main.tsx | 20 ++++++- .../tanstack-router/tests/basepath.test.ts | 55 +++++++++++++++++++ .../tests/routing-instrumentation.test.ts | 16 +++--- .../tanstack-router/vite.config.ts | 4 ++ packages/react/src/tanstackrouter.ts | 11 ++-- .../react/src/vendor/tanstackrouter-types.ts | 1 + packages/react/test/tanstackrouter.test.ts | 34 ++++++++++++ 8 files changed, 140 insertions(+), 14 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/tanstack-router/tests/basepath.test.ts diff --git a/dev-packages/e2e-tests/test-applications/tanstack-router/package.json b/dev-packages/e2e-tests/test-applications/tanstack-router/package.json index 65086e5b4953..9e9f8660a899 100644 --- a/dev-packages/e2e-tests/test-applications/tanstack-router/package.json +++ b/dev-packages/e2e-tests/test-applications/tanstack-router/package.json @@ -9,7 +9,9 @@ "test": "playwright test", "clean": "npx rimraf node_modules pnpm-lock.yaml", "test:build": "pnpm install && pnpm build", - "test:assert": "pnpm test" + "test:assert": "pnpm test", + "test:build:basepath": "E2E_TEST_BASEPATH=/app pnpm test:build", + "test:assert:basepath": "E2E_TEST_BASEPATH=/app pnpm test:assert" }, "dependencies": { "@sentry/react": "file:../../packed/sentry-react-packed.tgz", @@ -30,5 +32,14 @@ }, "volta": { "extends": "../../package.json" + }, + "sentryTest": { + "variants": [ + { + "build-command": "pnpm test:build:basepath", + "assert-command": "pnpm test:assert:basepath", + "label": "tanstack-router (basepath)" + } + ] } } diff --git a/dev-packages/e2e-tests/test-applications/tanstack-router/src/main.tsx b/dev-packages/e2e-tests/test-applications/tanstack-router/src/main.tsx index 23425faee718..3e79c090c563 100644 --- a/dev-packages/e2e-tests/test-applications/tanstack-router/src/main.tsx +++ b/dev-packages/e2e-tests/test-applications/tanstack-router/src/main.tsx @@ -87,9 +87,25 @@ const redirectRoute = createRoute({ }, }); -const routeTree = rootRoute.addChildren([indexRoute, redirectRoute, postsRoute.addChildren([postIdRoute])]); +// Dynamic enough to absorb basepath segments if they ever leak into route matching (see #23253). +const catchAllRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '$a/$b/$c', + component: function CatchAll() { + return
Catch all
; + }, +}); + +const routeTree = rootRoute.addChildren([ + indexRoute, + redirectRoute, + catchAllRoute, + postsRoute.addChildren([postIdRoute]), +]); + +declare const __APP_BASEPATH__: string; -const router = createRouter({ routeTree }); +const router = createRouter({ routeTree, ...(__APP_BASEPATH__ ? { basepath: __APP_BASEPATH__ } : {}) }); declare const __APP_DSN__: string; diff --git a/dev-packages/e2e-tests/test-applications/tanstack-router/tests/basepath.test.ts b/dev-packages/e2e-tests/test-applications/tanstack-router/tests/basepath.test.ts new file mode 100644 index 000000000000..99bf808dfde3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/tanstack-router/tests/basepath.test.ts @@ -0,0 +1,55 @@ +import { expect, test } from '@playwright/test'; +import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; + +// Only meaningful in the `tanstack-router (basepath)` variant, where the router is created with +// `basepath: '/app'`. The rest of the suite runs in both variants. +const BASE = process.env.E2E_TEST_BASEPATH || ''; + +test.describe('router basepath', () => { + test.skip(!BASE, 'Only runs in the basepath variant'); + + // `window.location.pathname` carries the basepath, but the router never sees it. Matching the + // pageload against the raw browser path let the catch-all `/$a/$b/$c` route absorb `app` as a + // param instead of matching `/posts/$postId`. + test('does not leak the basepath into the matched route params', async ({ page }) => { + const transactionPromise = waitForTransaction('tanstack-router', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + await page.goto(`${BASE}/posts/456`); + + const rootSpan = await transactionPromise; + + // `onResolved` later merges the correct params in, but never clears the ones the bad initial + // match already set, so the stale `a`/`b`/`c` params survive on the span. Keys are passed as + // arrays because `toHaveProperty` would otherwise read the dots as a nested lookup. + const traceData = rootSpan.contexts?.trace?.data; + expect(traceData).not.toHaveProperty(['url.path.params.a']); + expect(traceData).not.toHaveProperty(['url.path.params.b']); + expect(traceData).not.toHaveProperty(['url.path.params.c']); + expect(traceData).toHaveProperty(['url.path.params.postId'], '456'); + expect(traceData).toHaveProperty(['url.template'], '/posts/$postId'); + }); + + // 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. + test('attributes errors to the matched route for the whole page lifetime', async ({ page }) => { + const errorPromise = waitForError('tanstack-router', async errorEvent => { + return errorEvent.exception?.values?.[0]?.value === 'Error thrown after pageload'; + }); + + await page.goto(`${BASE}/posts/456`); + + await page.waitForTimeout(1000); + await page.evaluate(() => { + setTimeout(() => { + throw new Error('Error thrown after pageload'); + }, 0); + }); + + const errorEvent = await errorPromise; + + expect(errorEvent.transaction).toBe('/posts/$postId'); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/tanstack-router/tests/routing-instrumentation.test.ts b/dev-packages/e2e-tests/test-applications/tanstack-router/tests/routing-instrumentation.test.ts index 06708292089e..13a1d859a828 100644 --- a/dev-packages/e2e-tests/test-applications/tanstack-router/tests/routing-instrumentation.test.ts +++ b/dev-packages/e2e-tests/test-applications/tanstack-router/tests/routing-instrumentation.test.ts @@ -1,12 +1,14 @@ import { expect, test } from '@playwright/test'; import { waitForTransaction } from '@sentry-internal/test-utils'; +const BASE = process.env.E2E_TEST_BASEPATH || ''; + test('sends a pageload transaction with a parameterized URL', async ({ page }) => { const transactionPromise = waitForTransaction('tanstack-router', async transactionEvent => { return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; }); - await page.goto(`/posts/456`); + await page.goto(`${BASE}/posts/456`); const rootSpan = await transactionPromise; @@ -43,7 +45,7 @@ test('sends pageload transaction with web vitals measurements', async ({ page }) return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; }); - await page.goto(`/`); + await page.goto(`${BASE}/`); const transaction = await transactionPromise; @@ -94,7 +96,7 @@ test('sends a navigation transaction with a parameterized URL', async ({ page }) return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation'; }); - await page.goto(`/`); + await page.goto(`${BASE}/`); await pageloadTxnPromise; await page.waitForTimeout(5000); @@ -138,7 +140,7 @@ test('sends a pageload transaction with resolved URL attrs after same-route redi }); // `/posts/999` matches `/posts/$postId` initially, then `beforeLoad` redirects to `/posts/2`. - await page.goto(`/posts/999`); + await page.goto(`${BASE}/posts/999`); const pageloadTxn = await pageloadTxnPromise; @@ -174,7 +176,7 @@ test('sends a pageload transaction named after the resolved route when a redirec // Visiting `/redirect` directly throws `redirect({ to: '/posts/$postId', params: { postId: '1' } })` // in `beforeLoad` during the initial pageload, so the pageload span must be renamed to the target route. - await page.goto(`/redirect`); + await page.goto(`${BASE}/redirect`); const pageloadTxn = await pageloadTxnPromise; @@ -210,7 +212,7 @@ test('sends a navigation transaction when a redirect is thrown in beforeLoad', a return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation'; }); - await page.goto(`/`); + await page.goto(`${BASE}/`); await pageloadTxnPromise; await page.locator('#redirect-link').click(); @@ -247,7 +249,7 @@ test('sends a navigation transaction for a normal navigation that happens after return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; }); - await page.goto(`/`); + await page.goto(`${BASE}/`); await pageloadTxnPromise; // First trigger a redirect-driven navigation. Upstream (TanStack/router#3920) this leaves the diff --git a/dev-packages/e2e-tests/test-applications/tanstack-router/vite.config.ts b/dev-packages/e2e-tests/test-applications/tanstack-router/vite.config.ts index bd51f2f9679a..f863bf6d3085 100644 --- a/dev-packages/e2e-tests/test-applications/tanstack-router/vite.config.ts +++ b/dev-packages/e2e-tests/test-applications/tanstack-router/vite.config.ts @@ -1,11 +1,15 @@ import react from '@vitejs/plugin-react-swc'; import { defineConfig } from 'vite'; +const basepath = process.env.E2E_TEST_BASEPATH || ''; + // https://vitejs.dev/config/ export default defineConfig({ + base: basepath ? `${basepath}/` : '/', plugins: [react()], define: { __APP_DSN__: JSON.stringify(process.env.E2E_TEST_DSN), + __APP_BASEPATH__: JSON.stringify(basepath), }, preview: { port: 3030, diff --git a/packages/react/src/tanstackrouter.ts b/packages/react/src/tanstackrouter.ts index 839c880e4526..6073de1688d6 100644 --- a/packages/react/src/tanstackrouter.ts +++ b/packages/react/src/tanstackrouter.ts @@ -83,10 +83,13 @@ export function tanstackRouterBrowserTracingIntegration( const initialWindowLocation = WINDOW.location; if (instrumentPageLoad && initialWindowLocation) { - const routeMatch = resolveRouteMatch( - initialWindowLocation.pathname, - castRouterInstance.options.parseSearch(initialWindowLocation.search), - ); + const initialRouterLocation = castRouterInstance.state?.location; + const routeMatch = initialRouterLocation + ? resolveRouteMatch(initialRouterLocation.pathname, initialRouterLocation.search) + : resolveRouteMatch( + initialWindowLocation.pathname, + castRouterInstance.options.parseSearch(initialWindowLocation.search), + ); const pageloadSpan = startBrowserTracingPageLoadSpan(client, { name: routeMatch ? routeMatch.routeId : initialWindowLocation.pathname, diff --git a/packages/react/src/vendor/tanstackrouter-types.ts b/packages/react/src/vendor/tanstackrouter-types.ts index 3936c429cd5d..c8b296707d10 100644 --- a/packages/react/src/vendor/tanstackrouter-types.ts +++ b/packages/react/src/vendor/tanstackrouter-types.ts @@ -45,6 +45,7 @@ interface VendoredTanstackRouterHistory { interface VendoredTanstackRouterState { matches: Array; pendingMatches?: Array; + location?: VendoredTanstackRouterLocation; } export interface VendoredTanstackRouterRouteMatch { diff --git a/packages/react/test/tanstackrouter.test.ts b/packages/react/test/tanstackrouter.test.ts index c45bd83753c0..c75e9156166c 100644 --- a/packages/react/test/tanstackrouter.test.ts +++ b/packages/react/test/tanstackrouter.test.ts @@ -58,6 +58,7 @@ describe('tanstackRouterBrowserTracingIntegration', () => { beforeEach(() => { vi.clearAllMocks(); startBrowserTracingPageLoadSpanSpy.mockReturnValue(mockPageloadSpan as any); + (SentryBrowser.WINDOW as any).location = { pathname: '/posts/999', search: '' }; vi.stubGlobal('window', { location: { @@ -91,6 +92,39 @@ describe('tanstackRouterBrowserTracingIntegration', () => { }); }); + describe('pageload route matching', () => { + // `window.location.pathname` carries the router basepath, but whether `matchRoutes` wants it is + // version-dependent (newer routers strip it in `parseLocation`, older ones inside `matchRoutes`). + // `state.location` is always in the form the router itself expects, so we match against that. + it('matches against the router location, not window.location', () => { + (SentryBrowser.WINDOW as any).location = { pathname: '/app/posts/999', search: '?q=1' }; + + const integration = tanstackRouterBrowserTracingIntegration( + { ...mockRouter, state: { location: { pathname: '/posts/999', search: { q: 1 } } } }, + { instrumentPageLoad: true, instrumentNavigation: false }, + ); + + integration.afterAllSetup!(mockClient as any); + + expect(mockRouter.matchRoutes).toHaveBeenCalledWith('/posts/999', { q: 1 }, expect.any(Object)); + expect(mockRouter.options.parseSearch).not.toHaveBeenCalled(); + }); + + it('falls back to window.location when the router exposes no location', () => { + (SentryBrowser.WINDOW as any).location = { pathname: '/posts/999', search: '?q=1' }; + + const integration = tanstackRouterBrowserTracingIntegration(mockRouter, { + instrumentPageLoad: true, + instrumentNavigation: false, + }); + + integration.afterAllSetup!(mockClient as any); + + expect(mockRouter.options.parseSearch).toHaveBeenCalledWith('?q=1'); + expect(mockRouter.matchRoutes).toHaveBeenCalledWith('/posts/999', {}, expect.any(Object)); + }); + }); + it('updates pageload span URL attributes on redirect to the same route template', () => { const integration = tanstackRouterBrowserTracingIntegration(mockRouter, { instrumentPageLoad: true, From 8bebcd7929764e8d8da2aaa5c13a7a72000dce36 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Sun, 23 Aug 2026 09:55:41 +0300 Subject: [PATCH 2/2] fixup! fix(react): Match TanStack Router pageload against the router location --- .../tanstack-router/tests/basepath.test.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/tanstack-router/tests/basepath.test.ts b/dev-packages/e2e-tests/test-applications/tanstack-router/tests/basepath.test.ts index 99bf808dfde3..bd3040c429a3 100644 --- a/dev-packages/e2e-tests/test-applications/tanstack-router/tests/basepath.test.ts +++ b/dev-packages/e2e-tests/test-applications/tanstack-router/tests/basepath.test.ts @@ -31,17 +31,21 @@ test.describe('router basepath', () => { expect(traceData).toHaveProperty(['url.template'], '/posts/$postId'); }); - // 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. + // The first test only checks the span. The scope transaction is a separate value: it is set once + // when the pageload span starts, and the later `updateName` in `onResolved` does not rewrite it. + // So even when the sent transaction name is correct, errors captured after the pageload still + // carry the name from the initial match. This test checks that scope transaction. test('attributes errors to the matched route for the whole page lifetime', async ({ page }) => { + const transactionPromise = waitForTransaction('tanstack-router', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); const errorPromise = waitForError('tanstack-router', async errorEvent => { return errorEvent.exception?.values?.[0]?.value === 'Error thrown after pageload'; }); await page.goto(`${BASE}/posts/456`); + await transactionPromise; - await page.waitForTimeout(1000); await page.evaluate(() => { setTimeout(() => { throw new Error('Error thrown after pageload');