From 2112fbaa5c951334f95e4b606815f9bf1d62c405 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 24 Aug 2026 11:05:26 +0200 Subject: [PATCH 1/2] fix(remix): Resolve static routes to their own route template `findMatchingRoutes` returned nothing for a path that exactly matched a static route in the build-time manifest, so `maybeParameterizeRemixRoute` reported it the same way as a path it could not resolve at all: no manifest (the Vite plugin is opt-in, and the classic compiler never injects one), a 404, or a route the file-convention scanner missed. Static routes were therefore emitted with `sentry.source: 'url'` and no `url.template`, even though a static route is its own template - one with zero parameters - and is guaranteed low cardinality by the finite build-time route list. Return the route itself instead. Pageload and navigation spans both derive their name, source and `url.template` from that return value, so static routes now carry `sentry.source: 'route'`. Under span streaming they keep their path as the span name instead of collapsing to `Pageload`, which is now reserved for paths the manifest genuinely cannot resolve. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/client-transactions.test.ts | 5 ++-- .../tests/client-transactions.test.ts | 5 ++-- .../tests/client-transactions.test.ts | 5 ++-- .../tests/client-transactions.test.ts | 5 ++-- packages/remix/src/client/performance.tsx | 8 +++---- .../src/client/remixRouteParameterization.ts | 13 ++++++---- .../client/remixRouteParameterization.test.ts | 24 +++++++++---------- 7 files changed, 31 insertions(+), 34 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tests/client-transactions.test.ts b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tests/client-transactions.test.ts index 8a62126dbe15..ff773075f3fb 100644 --- a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tests/client-transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tests/client-transactions.test.ts @@ -14,13 +14,12 @@ test('Sends a pageload transaction to Sentry', async ({ page }) => { expect(transactionEvent.contexts?.trace?.data).toEqual( expect.objectContaining({ 'sentry.origin': 'auto.pageload.remix', - 'sentry.source': 'url', + 'sentry.source': 'route', 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/), 'url.path': '/', + 'url.template': '/', }), ); - // no url.template because the route isn't parameterized (sentry.source: 'url') - expect(transactionEvent.contexts?.trace?.data).not.toHaveProperty('url.template'); }); test('Sends a navigation transaction to Sentry', async ({ page }) => { diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express/tests/client-transactions.test.ts b/dev-packages/e2e-tests/test-applications/create-remix-app-express/tests/client-transactions.test.ts index f47a001fa7c7..30188a9d4ee8 100644 --- a/dev-packages/e2e-tests/test-applications/create-remix-app-express/tests/client-transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express/tests/client-transactions.test.ts @@ -14,13 +14,12 @@ test('Sends a pageload transaction to Sentry', async ({ page }) => { expect(transactionEvent.contexts?.trace?.data).toEqual( expect.objectContaining({ 'sentry.origin': 'auto.pageload.remix', - 'sentry.source': 'url', + 'sentry.source': 'route', 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/), 'url.path': '/', + 'url.template': '/', }), ); - // no url.template because the route isn't parameterized (sentry.source: 'url') - expect(transactionEvent.contexts?.trace?.data).not.toHaveProperty('url.template'); }); test('Sends a navigation transaction to Sentry', async ({ page }) => { diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-v2/tests/client-transactions.test.ts b/dev-packages/e2e-tests/test-applications/create-remix-app-v2/tests/client-transactions.test.ts index 111902c7dd3c..0f60d08876c6 100644 --- a/dev-packages/e2e-tests/test-applications/create-remix-app-v2/tests/client-transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-v2/tests/client-transactions.test.ts @@ -13,13 +13,12 @@ test('Sends a pageload transaction to Sentry', async ({ page }) => { expect(transactionEvent).toBeDefined(); expect(transactionEvent.contexts?.trace?.data).toEqual( expect.objectContaining({ - 'sentry.source': 'url', + 'sentry.source': 'route', 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/), 'url.path': '/', + 'url.template': '/', }), ); - // no url.template because the route isn't parameterized (sentry.source: 'url') - expect(transactionEvent.contexts?.trace?.data).not.toHaveProperty('url.template'); }); test('Sends a navigation transaction to Sentry', async ({ page }) => { diff --git a/dev-packages/e2e-tests/test-applications/remix-hydrogen/tests/client-transactions.test.ts b/dev-packages/e2e-tests/test-applications/remix-hydrogen/tests/client-transactions.test.ts index 90745d9b777b..bde21cd3673a 100644 --- a/dev-packages/e2e-tests/test-applications/remix-hydrogen/tests/client-transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/remix-hydrogen/tests/client-transactions.test.ts @@ -16,15 +16,14 @@ test('Sends a pageload transaction to Sentry', async ({ page }) => { contexts: { trace: { data: { - 'sentry.source': 'url', + 'sentry.source': 'route', 'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/$/), 'url.path': '/', + 'url.template': '/', }, }, }, }); - // no url.template because the route isn't parameterized (sentry.source: 'url') - expect(transactionEvent.contexts?.trace?.data).not.toHaveProperty('url.template'); }); test('Sends a navigation transaction to Sentry', async ({ page }) => { diff --git a/packages/remix/src/client/performance.tsx b/packages/remix/src/client/performance.tsx index a3ef39d37776..4890eb31773b 100644 --- a/packages/remix/src/client/performance.tsx +++ b/packages/remix/src/client/performance.tsx @@ -62,8 +62,8 @@ function getInitPathName(): string | undefined { /** * Determines the transaction name and source for a route. * Handles three cases: - * 1. Dynamic routes with manifest (Vite apps): Use parameterized path with source 'route' - * 2. Static routes with manifest (Vite apps): Use pathname with source 'url' + * 1. Routes resolved from the manifest (Vite apps): Use the route template with source 'route' + * 2. Routes the manifest doesn't know, e.g. 404s (Vite apps): Use pathname with source 'url' * 3. Legacy apps without manifest: Use route ID with source 'route' */ function getTransactionNameAndSource( @@ -73,13 +73,11 @@ function getTransactionNameAndSource( const parameterizedRoute = pathname ? maybeParameterizeRemixRoute(pathname) : undefined; if (parameterizedRoute) { - // We have a parameterized route from the manifest (dynamic route) return { name: parameterizedRoute, source: 'route' }; } if (hasManifest()) { - // We have a manifest but no parameterization (static route) - // Use the pathname with source 'url' + // The manifest doesn't know this route, so the pathname may well be high cardinality. return { name: pathname || routeId, source: 'url' }; } diff --git a/packages/remix/src/client/remixRouteParameterization.ts b/packages/remix/src/client/remixRouteParameterization.ts index 6a587afffdd9..7cbf2ccd23cf 100644 --- a/packages/remix/src/client/remixRouteParameterization.ts +++ b/packages/remix/src/client/remixRouteParameterization.ts @@ -112,9 +112,12 @@ function findMatchingRoutes( ): string[] { const matches: string[] = []; - // Static routes don't need parameterization - return empty to keep source as 'url' + // A static route is its own template. Returning it rather than nothing lets callers tell + // "known route, zero params" apart from "we could not resolve this path at all" (no manifest, + // 404, route missed by the scanner), which is the difference between a low- and high-cardinality + // span name. if (staticRoutes.some(r => r.path === route)) { - return matches; + return [route]; } // Check dynamic routes @@ -139,10 +142,10 @@ export function hasManifest(): boolean { } /** - * Parameterize a route using the route manifest. + * Resolve the route template for a concrete route using the route manifest. * - * @param route - The route to parameterize. - * @returns The parameterized route or undefined if no parameterization is needed. + * @param route - The route to resolve. + * @returns The route template, or undefined if the route could not be resolved from the manifest. */ export const maybeParameterizeRemixRoute = (route: string): string | undefined => { const manifest = getManifest(); diff --git a/packages/remix/test/client/remixRouteParameterization.test.ts b/packages/remix/test/client/remixRouteParameterization.test.ts index 59eb2c1796c1..dfa30320d775 100644 --- a/packages/remix/test/client/remixRouteParameterization.test.ts +++ b/packages/remix/test/client/remixRouteParameterization.test.ts @@ -25,17 +25,17 @@ describe('maybeParameterizeRemixRoute', () => { }); describe('when manifest has static routes', () => { - it('should return undefined for static routes', () => { + it('should return the route itself for static routes', () => { const manifest: RouteManifest = { staticRoutes: [{ path: '/' }, { path: '/about' }, { path: '/contact' }, { path: '/blog/posts' }], dynamicRoutes: [], }; globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); - expect(maybeParameterizeRemixRoute('/')).toBeUndefined(); - expect(maybeParameterizeRemixRoute('/about')).toBeUndefined(); - expect(maybeParameterizeRemixRoute('/contact')).toBeUndefined(); - expect(maybeParameterizeRemixRoute('/blog/posts')).toBeUndefined(); + expect(maybeParameterizeRemixRoute('/')).toBe('/'); + expect(maybeParameterizeRemixRoute('/about')).toBe('/about'); + expect(maybeParameterizeRemixRoute('/contact')).toBe('/contact'); + expect(maybeParameterizeRemixRoute('/blog/posts')).toBe('/blog/posts'); }); }); @@ -84,8 +84,8 @@ describe('maybeParameterizeRemixRoute', () => { }; globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); - expect(maybeParameterizeRemixRoute('/')).toBeUndefined(); - expect(maybeParameterizeRemixRoute('/about')).toBeUndefined(); + expect(maybeParameterizeRemixRoute('/')).toBe('/'); + expect(maybeParameterizeRemixRoute('/about')).toBe('/about'); }); it('should handle splat/catch-all routes', () => { @@ -222,7 +222,7 @@ describe('maybeParameterizeRemixRoute', () => { }; globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); - expect(maybeParameterizeRemixRoute('/')).toBeUndefined(); + expect(maybeParameterizeRemixRoute('/')).toBe('/'); }); it('should handle complex nested dynamic routes', () => { @@ -246,10 +246,10 @@ describe('maybeParameterizeRemixRoute', () => { describe('realistic Remix patterns', () => { it.each([ - ['/', undefined], - ['/about', undefined], - ['/contact', undefined], - ['/blog/posts', undefined], + ['/', '/'], + ['/about', '/about'], + ['/contact', '/contact'], + ['/blog/posts', '/blog/posts'], ['/users/123', '/users/:id'], ['/users/john-doe', '/users/:id'], From 176c9d43f05f1c32f90f3bed0626ee2cdeaf4085 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 24 Aug 2026 12:53:48 +0200 Subject: [PATCH 2/2] deslop --- .../remix/src/client/remixRouteParameterization.ts | 11 ++++------- .../test/client/remixRouteParameterization.test.ts | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/remix/src/client/remixRouteParameterization.ts b/packages/remix/src/client/remixRouteParameterization.ts index 7cbf2ccd23cf..5e389c2e87ad 100644 --- a/packages/remix/src/client/remixRouteParameterization.ts +++ b/packages/remix/src/client/remixRouteParameterization.ts @@ -112,10 +112,7 @@ function findMatchingRoutes( ): string[] { const matches: string[] = []; - // A static route is its own template. Returning it rather than nothing lets callers tell - // "known route, zero params" apart from "we could not resolve this path at all" (no manifest, - // 404, route missed by the scanner), which is the difference between a low- and high-cardinality - // span name. + // Static routes don't need parameterization, return the route itself as already parameterized if (staticRoutes.some(r => r.path === route)) { return [route]; } @@ -142,10 +139,10 @@ export function hasManifest(): boolean { } /** - * Resolve the route template for a concrete route using the route manifest. + * Parameterize a route using the route manifest. * - * @param route - The route to resolve. - * @returns The route template, or undefined if the route could not be resolved from the manifest. + * @param route - The route to parameterize. + * @returns The parameterized route or undefined if not able to parameterize. */ export const maybeParameterizeRemixRoute = (route: string): string | undefined => { const manifest = getManifest(); diff --git a/packages/remix/test/client/remixRouteParameterization.test.ts b/packages/remix/test/client/remixRouteParameterization.test.ts index dfa30320d775..128d50a363aa 100644 --- a/packages/remix/test/client/remixRouteParameterization.test.ts +++ b/packages/remix/test/client/remixRouteParameterization.test.ts @@ -25,7 +25,7 @@ describe('maybeParameterizeRemixRoute', () => { }); describe('when manifest has static routes', () => { - it('should return the route itself for static routes', () => { + it('returns the route itself for static routes', () => { const manifest: RouteManifest = { staticRoutes: [{ path: '/' }, { path: '/about' }, { path: '/contact' }, { path: '/blog/posts' }], dynamicRoutes: [],