From a98de0bf06b2e6604277b1ff5d6e34dbee4019af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Peer=20St=C3=B6cklmair?= Date: Sun, 23 Aug 2026 11:25:12 +0300 Subject: [PATCH] fix(v10/react): Match TanStack Router pageload against the router location Backport of: #23299 --- .../tanstack-router/package.json | 13 +++- .../tanstack-router/src/main.tsx | 20 ++++++- .../tanstack-router/tests/basepath.test.ts | 59 +++++++++++++++++++ .../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, 144 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 3c2ed2905383..de4fca6ff30a 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..bd3040c429a3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/tanstack-router/tests/basepath.test.ts @@ -0,0 +1,59 @@ +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 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.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 ea6e9a3ea1f2..de6e8623db90 100644 --- a/packages/react/src/tanstackrouter.ts +++ b/packages/react/src/tanstackrouter.ts @@ -82,10 +82,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,