From 80bdca713a32a9b7589768017834512f7af637ee Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 14 Oct 2025 15:56:23 +0100 Subject: [PATCH 1/5] feat(remix): Add parameterized transaction naming for routes --- .../tests/client-transactions.test.ts | 20 +- .../tests/server-transactions.test.ts | 2 +- .../vite.config.ts | 5 +- .../tests/client-transactions.test.ts | 4 +- .../tests/server-transactions.test.ts | 4 +- .../create-remix-app-express/vite.config.ts | 5 +- .../create-remix-app-v2-legacy/.eslintrc.js | 4 + .../create-remix-app-v2-legacy/.gitignore | 6 + .../create-remix-app-v2-legacy/.npmrc | 2 + .../app/entry.client.tsx | 48 ++ .../app/entry.server.tsx | 144 +++++ .../create-remix-app-v2-legacy/app/root.tsx | 80 +++ .../app/routes/_index.tsx | 27 + .../app/routes/client-error.tsx | 13 + .../app/routes/navigate.tsx | 20 + .../app/routes/user.$id.tsx | 3 + .../create-remix-app-v2-legacy/globals.d.ts | 7 + .../instrument.server.cjs | 8 + .../create-remix-app-v2-legacy/package.json | 39 ++ .../playwright.config.mjs | 8 + .../remix.config.js | 9 + .../create-remix-app-v2-legacy/remix.env.d.ts | 2 + .../start-event-proxy.mjs | 6 + .../tests/client-errors.test.ts | 29 + .../tests/client-transactions.test.ts | 58 ++ .../tests/server-transactions.test.ts | 58 ++ .../create-remix-app-v2-legacy/tsconfig.json | 22 + .../create-remix-app-v2/package.json | 9 +- .../tests/client-transactions.test.ts | 4 +- .../tests/server-transactions.test.ts | 2 +- .../create-remix-app-v2/vite.config.ts | 15 + .../tests/client-transactions.test.ts | 6 +- .../tests/server-transactions.test.ts | 4 +- .../remix-hydrogen/vite.config.ts | 2 + packages/cloudflare/src/request.ts | 4 + packages/remix/src/client/performance.tsx | 37 +- .../src/client/remixRouteParameterization.ts | 153 ++++++ .../src/config/createRemixRouteManifest.ts | 224 ++++++++ .../remix/src/config/remixRouteManifest.ts | 33 ++ packages/remix/src/config/vite.ts | 137 +++++ packages/remix/src/index.server.ts | 5 + packages/remix/src/server/instrumentServer.ts | 64 ++- packages/remix/src/utils/utils.ts | 96 +++- .../client/remixRouteParameterization.test.ts | 416 +++++++++++++++ .../manifest/createRemixRouteManifest.test.ts | 136 +++++ packages/remix/test/config/vite.test.ts | 505 ++++++++++++++++++ .../app/routes/api.v1.data.$id.tsx | 10 + .../routes/deeply.$nested.$structure.$id.tsx | 26 + .../app/routes/manual-tracing.$id.tsx | 14 +- .../products.$productId.reviews.$reviewId.tsx | 24 + .../routes/users.$userId.posts.$postId.tsx | 24 + packages/remix/test/integration/package.json | 9 +- .../regexPatternValidation.test.ts | 235 ++++++++ .../remix/test/integration/remix.config.js | 7 - .../routeConversionConsistency.test.ts | 156 ++++++ .../test/client/capture-exception.test.ts | 2 +- .../test/client/capture-message.test.ts | 2 +- .../test/client/errorboundary.test.ts | 4 +- .../test/client/manualtracing.test.ts | 2 +- .../client/navigation-transactions.test.ts | 72 +++ .../integration/test/client/pageload.test.ts | 4 +- .../test/client/root-loader.test.ts | 91 +--- .../instrumentation/nested-routes.test.ts | 128 +++++ .../integration/test/server/utils/helpers.ts | 3 +- packages/remix/test/integration/tsconfig.json | 13 +- .../remix/test/integration/vite.config.ts | 42 ++ packages/remix/test/utils/utils.test.ts | 95 ++++ 67 files changed, 3296 insertions(+), 152 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.eslintrc.js create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.gitignore create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.npmrc create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/entry.client.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/entry.server.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/root.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/routes/_index.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/routes/client-error.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/routes/navigate.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/routes/user.$id.tsx create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/globals.d.ts create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/instrument.server.cjs create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/package.json create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/playwright.config.mjs create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/remix.config.js create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/remix.env.d.ts create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/start-event-proxy.mjs create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/tests/client-errors.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/tests/client-transactions.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/tests/server-transactions.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/tsconfig.json create mode 100644 dev-packages/e2e-tests/test-applications/create-remix-app-v2/vite.config.ts create mode 100644 packages/remix/src/client/remixRouteParameterization.ts create mode 100644 packages/remix/src/config/createRemixRouteManifest.ts create mode 100644 packages/remix/src/config/remixRouteManifest.ts create mode 100644 packages/remix/src/config/vite.ts create mode 100644 packages/remix/test/client/remixRouteParameterization.test.ts create mode 100644 packages/remix/test/config/manifest/createRemixRouteManifest.test.ts create mode 100644 packages/remix/test/config/vite.test.ts create mode 100644 packages/remix/test/integration/app/routes/api.v1.data.$id.tsx create mode 100644 packages/remix/test/integration/app/routes/deeply.$nested.$structure.$id.tsx create mode 100644 packages/remix/test/integration/app/routes/products.$productId.reviews.$reviewId.tsx create mode 100644 packages/remix/test/integration/app/routes/users.$userId.posts.$postId.tsx create mode 100644 packages/remix/test/integration/regexPatternValidation.test.ts delete mode 100644 packages/remix/test/integration/remix.config.js create mode 100644 packages/remix/test/integration/routeConversionConsistency.test.ts create mode 100644 packages/remix/test/integration/test/client/navigation-transactions.test.ts create mode 100644 packages/remix/test/integration/test/server/instrumentation/nested-routes.test.ts create mode 100644 packages/remix/test/integration/vite.config.ts create mode 100644 packages/remix/test/utils/utils.test.ts 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 9a5caf46fd66..d85d9d82747d 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 @@ -3,7 +3,7 @@ import { waitForTransaction } from '@sentry-internal/test-utils'; test('Sends a pageload transaction to Sentry', async ({ page }) => { const transactionPromise = waitForTransaction('create-remix-app-express-vite-dev', transactionEvent => { - return transactionEvent.contexts?.trace?.op === 'pageload' && transactionEvent.transaction === 'routes/_index'; + return transactionEvent.contexts?.trace?.op === 'pageload' && transactionEvent.transaction === '/'; }); await page.goto('/'); @@ -15,7 +15,7 @@ test('Sends a pageload transaction to Sentry', async ({ page }) => { test('Sends a navigation transaction to Sentry', async ({ page }) => { const transactionPromise = waitForTransaction('create-remix-app-express-vite-dev', transactionEvent => { - return transactionEvent.contexts?.trace?.op === 'navigation' && transactionEvent.transaction === 'routes/user.$id'; + return transactionEvent.contexts?.trace?.op === 'navigation' && transactionEvent.transaction === '/user/:id'; }); await page.goto('/'); @@ -28,6 +28,22 @@ test('Sends a navigation transaction to Sentry', async ({ page }) => { expect(transactionEvent).toBeDefined(); }); +test('Sends a navigation transaction with parameterized route to Sentry', async ({ page }) => { + const transactionPromise = waitForTransaction('create-remix-app-express-vite-dev', transactionEvent => { + return transactionEvent.contexts?.trace?.op === 'navigation'; + }); + + await page.goto('/'); + + const linkElement = page.locator('id=navigation'); + await linkElement.click(); + + const transactionEvent = await transactionPromise; + + expect(transactionEvent).toBeDefined(); + expect(transactionEvent.transaction).toBeTruthy(); +}); + test('Renders `sentry-trace` and `baggage` meta tags for the root route', async ({ page }) => { await page.goto('/'); diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tests/server-transactions.test.ts b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tests/server-transactions.test.ts index 58e81eeee529..4213aae3e3de 100644 --- a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tests/server-transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tests/server-transactions.test.ts @@ -48,7 +48,7 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page const pageLoadParentSpanId = pageloadTransaction.contexts?.trace?.parent_span_id; expect(httpServerTransaction.transaction).toBe('GET http://localhost:3030/'); - expect(pageloadTransaction.transaction).toBe('routes/_index'); + expect(pageloadTransaction.transaction).toBe('/'); expect(httpServerTraceId).toBeDefined(); expect(httpServerSpanId).toBeDefined(); diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/vite.config.ts b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/vite.config.ts index 13de9243b22a..6ebb8eacc6a5 100644 --- a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/vite.config.ts +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/vite.config.ts @@ -1,14 +1,15 @@ +import { installGlobals } from '@remix-run/node'; import { vitePlugin as remix } from '@remix-run/dev'; +import { sentryRemixVitePlugin } from '@sentry/remix'; import { defineConfig } from 'vite'; import tsconfigPaths from 'vite-tsconfig-paths'; -import { installGlobals } from '@remix-run/node'; - installGlobals(); export default defineConfig({ plugins: [ remix(), + sentryRemixVitePlugin(), tsconfigPaths({ // The dev server config errors are not relevant to this test app // https://github.com/aleclarson/vite-tsconfig-paths?tab=readme-ov-file#options 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 a06aa02ceb9c..68237301b635 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 @@ -3,7 +3,7 @@ import { waitForTransaction } from '@sentry-internal/test-utils'; test('Sends a pageload transaction to Sentry', async ({ page }) => { const transactionPromise = waitForTransaction('create-remix-app-express', transactionEvent => { - return transactionEvent.contexts?.trace?.op === 'pageload' && transactionEvent.transaction === 'routes/_index'; + return transactionEvent.contexts?.trace?.op === 'pageload' && transactionEvent.transaction === '/'; }); await page.goto('/'); @@ -15,7 +15,7 @@ test('Sends a pageload transaction to Sentry', async ({ page }) => { test('Sends a navigation transaction to Sentry', async ({ page }) => { const transactionPromise = waitForTransaction('create-remix-app-express', transactionEvent => { - return transactionEvent.contexts?.trace?.op === 'navigation' && transactionEvent.transaction === 'routes/user.$id'; + return transactionEvent.contexts?.trace?.op === 'navigation' && transactionEvent.transaction === '/user/:id'; }); await page.goto('/'); diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express/tests/server-transactions.test.ts b/dev-packages/e2e-tests/test-applications/create-remix-app-express/tests/server-transactions.test.ts index d57c45545caf..ddb866e5dbaa 100644 --- a/dev-packages/e2e-tests/test-applications/create-remix-app-express/tests/server-transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express/tests/server-transactions.test.ts @@ -90,7 +90,7 @@ test('Propagates trace when ErrorBoundary is triggered', async ({ page }) => { const pageLoadParentSpanId = pageloadTransaction.contexts?.trace?.parent_span_id; expect(httpServerTransaction.transaction).toBe('GET client-error'); - expect(pageloadTransaction.transaction).toBe('routes/client-error'); + expect(pageloadTransaction.transaction).toBe('/client-error'); expect(httpServerTraceId).toBeDefined(); expect(httpServerSpanId).toBeDefined(); @@ -132,7 +132,7 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page const pageLoadParentSpanId = pageloadTransaction.contexts?.trace?.parent_span_id; expect(httpServerTransaction.transaction).toBe('GET http://localhost:3030/'); - expect(pageloadTransaction.transaction).toBe('routes/_index'); + expect(pageloadTransaction.transaction).toBe('/'); expect(httpServerTraceId).toBeDefined(); expect(httpServerSpanId).toBeDefined(); diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express/vite.config.ts b/dev-packages/e2e-tests/test-applications/create-remix-app-express/vite.config.ts index 13de9243b22a..6ebb8eacc6a5 100644 --- a/dev-packages/e2e-tests/test-applications/create-remix-app-express/vite.config.ts +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-express/vite.config.ts @@ -1,14 +1,15 @@ +import { installGlobals } from '@remix-run/node'; import { vitePlugin as remix } from '@remix-run/dev'; +import { sentryRemixVitePlugin } from '@sentry/remix'; import { defineConfig } from 'vite'; import tsconfigPaths from 'vite-tsconfig-paths'; -import { installGlobals } from '@remix-run/node'; - installGlobals(); export default defineConfig({ plugins: [ remix(), + sentryRemixVitePlugin(), tsconfigPaths({ // The dev server config errors are not relevant to this test app // https://github.com/aleclarson/vite-tsconfig-paths?tab=readme-ov-file#options diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.eslintrc.js b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.eslintrc.js new file mode 100644 index 000000000000..f2faf1470fd8 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.eslintrc.js @@ -0,0 +1,4 @@ +/** @type {import('eslint').Linter.Config} */ +module.exports = { + extends: ['@remix-run/eslint-config', '@remix-run/eslint-config/node'], +}; diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.gitignore b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.gitignore new file mode 100644 index 000000000000..3f7bf98da3e1 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.gitignore @@ -0,0 +1,6 @@ +node_modules + +/.cache +/build +/public/build +.env diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.npmrc b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/.npmrc @@ -0,0 +1,2 @@ +@sentry:registry=http://127.0.0.1:4873 +@sentry-internal:registry=http://127.0.0.1:4873 diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/entry.client.tsx b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/entry.client.tsx new file mode 100644 index 000000000000..f540f3c35c1d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/entry.client.tsx @@ -0,0 +1,48 @@ +/** + * By default, Remix will handle hydrating your app on the client for you. + * You are free to delete this file if you'd like to, but if you ever want it revealed again, you can run `npx remix reveal` ✨ + * For more information, see https://remix.run/file-conventions/entry.client + */ + +// Extend the Window interface to include ENV +declare global { + interface Window { + ENV: { + SENTRY_DSN: string; + [key: string]: unknown; + }; + } +} + +import { RemixBrowser, useLocation, useMatches } from '@remix-run/react'; +import * as Sentry from '@sentry/remix'; +import { StrictMode, startTransition, useEffect } from 'react'; +import { hydrateRoot } from 'react-dom/client'; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: window.ENV.SENTRY_DSN, + integrations: [ + Sentry.browserTracingIntegration({ + useEffect, + useLocation, + useMatches, + }), + Sentry.replayIntegration(), + ], + // Performance Monitoring + tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production! + // Session Replay + replaysSessionSampleRate: 0.1, // This sets the sample rate at 10%. You may want to change it to 100% while in development and then sample at a lower rate in production. + replaysOnErrorSampleRate: 1.0, // If you're not already sampling the entire session, change the sample rate to 100% when sampling sessions where errors occur. + tunnel: 'http://localhost:3032/', // proxy server +}); + +startTransition(() => { + hydrateRoot( + document, + + + , + ); +}); diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/entry.server.tsx b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/entry.server.tsx new file mode 100644 index 000000000000..c49e814246a8 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/entry.server.tsx @@ -0,0 +1,144 @@ +import * as Sentry from '@sentry/remix'; + +Sentry.init({ + tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production! + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.E2E_TEST_DSN, + tunnel: 'http://localhost:3031/', // proxy server +}); + +/** + * By default, Remix will handle generating the HTTP Response for you. + * You are free to delete this file if you'd like to, but if you ever want it revealed again, you can run `npx remix reveal` ✨ + * For more information, see https://remix.run/file-conventions/entry.server + */ + +import { PassThrough } from 'node:stream'; + +import type { AppLoadContext, EntryContext } from '@remix-run/node'; +import { createReadableStreamFromReadable } from '@remix-run/node'; +import { installGlobals } from '@remix-run/node'; +import { RemixServer } from '@remix-run/react'; +import isbot from 'isbot'; +import { renderToPipeableStream } from 'react-dom/server'; + +installGlobals(); + +const ABORT_DELAY = 5_000; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.E2E_TEST_DSN, + // Performance Monitoring + tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production! +}); + +const handleErrorImpl = () => { + Sentry.setTag('remix-test-tag', 'remix-test-value'); +}; + +export const handleError = Sentry.wrapHandleErrorWithSentry(handleErrorImpl); + +export default function handleRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + remixContext: EntryContext, + loadContext: AppLoadContext, +) { + return isbot(request.headers.get('user-agent')) + ? handleBotRequest(request, responseStatusCode, responseHeaders, remixContext) + : handleBrowserRequest(request, responseStatusCode, responseHeaders, remixContext); +} + +function handleBotRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + remixContext: EntryContext, +) { + return new Promise((resolve, reject) => { + let shellRendered = false; + const { pipe, abort } = renderToPipeableStream( + , + { + onAllReady() { + shellRendered = true; + const body = new PassThrough(); + const stream = createReadableStreamFromReadable(body); + + responseHeaders.set('Content-Type', 'text/html'); + + resolve( + new Response(stream, { + headers: responseHeaders, + status: responseStatusCode, + }), + ); + + pipe(body); + }, + onShellError(error: unknown) { + reject(error); + }, + onError(error: unknown) { + responseStatusCode = 500; + // Log streaming rendering errors from inside the shell. Don't log + // errors encountered during initial shell rendering since they'll + // reject and get logged in handleDocumentRequest. + if (shellRendered) { + console.error(error); + } + }, + }, + ); + + setTimeout(abort, ABORT_DELAY); + }); +} + +function handleBrowserRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + remixContext: EntryContext, +) { + return new Promise((resolve, reject) => { + let shellRendered = false; + const { pipe, abort } = renderToPipeableStream( + , + { + onShellReady() { + shellRendered = true; + const body = new PassThrough(); + const stream = createReadableStreamFromReadable(body); + + responseHeaders.set('Content-Type', 'text/html'); + + resolve( + new Response(stream, { + headers: responseHeaders, + status: responseStatusCode, + }), + ); + + pipe(body); + }, + onShellError(error: unknown) { + reject(error); + }, + onError(error: unknown) { + responseStatusCode = 500; + // Log streaming rendering errors from inside the shell. Don't log + // errors encountered during initial shell rendering since they'll + // reject and get logged in handleDocumentRequest. + if (shellRendered) { + console.error(error); + } + }, + }, + ); + + setTimeout(abort, ABORT_DELAY); + }); +} diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/root.tsx b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/root.tsx new file mode 100644 index 000000000000..517a37a9d76b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-v2-legacy/app/root.tsx @@ -0,0 +1,80 @@ +import { cssBundleHref } from '@remix-run/css-bundle'; +import { LinksFunction, MetaFunction, json } from '@remix-run/node'; +import { + Links, + LiveReload, + Meta, + Outlet, + Scripts, + ScrollRestoration, + useLoaderData, + useRouteError, +} from '@remix-run/react'; +import { captureRemixErrorBoundaryError, withSentry } from '@sentry/remix'; +import type { SentryMetaArgs } from '@sentry/remix'; + +export const links: LinksFunction = () => [...(cssBundleHref ? [{ rel: 'stylesheet', href: cssBundleHref }] : [])]; + +export const loader = () => { + return json({ + ENV: { + SENTRY_DSN: process.env.E2E_TEST_DSN, + }, + }); +}; + +export const meta = ({ data }: SentryMetaArgs>) => { + return [ + { + env: data.ENV, + }, + { + name: 'sentry-trace', + content: data.sentryTrace, + }, + { + name: 'baggage', + content: data.sentryBaggage, + }, + ]; +}; + +export function ErrorBoundary() { + const error = useRouteError(); + const eventId = captureRemixErrorBoundaryError(error); + + return ( +
+ ErrorBoundary Error + {eventId} +
+ ); +} + +function App() { + const { ENV } = useLoaderData(); + + return ( + + + + + `; + + if (//i.test(html)) { + return html.replace(//i, match => `${match}\n ${script}`); + } + + if (/]*>/i.test(html)) { + return html.replace(/]*>/i, match => `${match}\n${script}`); + } + + return `${script}${html}`; + }, + }, + + transform(code, id) { + if (!routeManifestJson) { + return null; + } + + const isClientEntry = + /entry[.-]client\.[jt]sx?$/.test(id) || + // Also handle Remix's default entry.client location + id.includes('/entry.client.') || + id.includes('/entry-client.'); + + if (isClientEntry) { + // XSS Prevention: Double-stringify strategy (same as transformIndexHtml above) + const injectedCode = ` +// Sentry Remix Route Manifest - Auto-injected +if (typeof window !== 'undefined') { + window.${MANIFEST_GLOBAL_KEY} = window.${MANIFEST_GLOBAL_KEY} || ${JSON.stringify(routeManifestJson)}; +} +${code}`; + + return { + code: injectedCode, + map: null, + }; + } + + return null; + }, + }; +} diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index 4ba64d609317..e4eb8ad6236e 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -1,4 +1,9 @@ export * from './server'; export { captureRemixErrorBoundaryError, withSentry, ErrorBoundary, browserTracingIntegration } from './client'; +export { sentryRemixVitePlugin } from './config/vite'; +export { createRemixRouteManifest } from './config/createRemixRouteManifest'; +export type { CreateRemixRouteManifestOptions } from './config/createRemixRouteManifest'; + export type { SentryMetaArgs } from './utils/types'; +export type { RouteManifest, RouteInfo } from './config/remixRouteManifest'; diff --git a/packages/remix/src/server/instrumentServer.ts b/packages/remix/src/server/instrumentServer.ts index fda9b3f10b75..a0e803b243d8 100644 --- a/packages/remix/src/server/instrumentServer.ts +++ b/packages/remix/src/server/instrumentServer.ts @@ -149,14 +149,49 @@ function makeWrappedDocumentRequestFunction(instrumentTracing?: boolean) { }; } +/** + * Updates the root span name with the parameterized route name. + * This is necessary for runtimes like Cloudflare Workers/Hydrogen where + * the request handler is not wrapped by Remix's wrapRequestHandler. + */ +function updateSpanWithRoute(args: DataFunctionArgs, build: ServerBuild): void { + try { + const activeSpan = getActiveSpan(); + const rootSpan = activeSpan && getRootSpan(activeSpan); + + if (!rootSpan) { + return; + } + + const routes = createRoutes(build.routes); + const url = new URL(args.request.url); + const [transactionName] = getTransactionName(routes, url); + + // Preserve the HTTP method prefix if the span already has one + const method = args.request.method.toUpperCase(); + const currentSpanName = spanToJSON(rootSpan).description; + const newSpanName = currentSpanName?.startsWith(method) ? `${method} ${transactionName}` : transactionName; + + rootSpan.updateName(newSpanName); + } catch (e) { + DEBUG_BUILD && debug.warn('Failed to update span name with route', e); + } +} + function makeWrappedDataFunction( origFn: DataFunction, id: string, name: 'action' | 'loader', instrumentTracing?: boolean, + build?: ServerBuild, ): DataFunction { return async function (this: unknown, args: DataFunctionArgs): Promise { if (instrumentTracing) { + // Update span name for Cloudflare Workers/Hydrogen environments + if (build) { + updateSpanWithRoute(args, build); + } + return startSpan( { op: `function.remix.${name}`, @@ -177,20 +212,26 @@ function makeWrappedDataFunction( } const makeWrappedAction = - (id: string, instrumentTracing?: boolean) => + (id: string, instrumentTracing?: boolean, build?: ServerBuild) => (origAction: DataFunction): DataFunction => { - return makeWrappedDataFunction(origAction, id, 'action', instrumentTracing); + return makeWrappedDataFunction(origAction, id, 'action', instrumentTracing, build); }; const makeWrappedLoader = - (id: string, instrumentTracing?: boolean) => + (id: string, instrumentTracing?: boolean, build?: ServerBuild) => (origLoader: DataFunction): DataFunction => { - return makeWrappedDataFunction(origLoader, id, 'loader', instrumentTracing); + return makeWrappedDataFunction(origLoader, id, 'loader', instrumentTracing, build); }; -function makeWrappedRootLoader() { +function makeWrappedRootLoader(instrumentTracing?: boolean, build?: ServerBuild) { return function (origLoader: DataFunction): DataFunction { return async function (this: unknown, args: DataFunctionArgs): Promise { + // Update span name for Cloudflare Workers/Hydrogen environments + // The root loader always runs, even for routes that don't have their own loaders + if (instrumentTracing && build) { + updateSpanWithRoute(args, build); + } + const res = await origLoader.call(this, args); const traceAndBaggage = getTraceAndBaggage(); @@ -283,6 +324,13 @@ function wrapRequestHandler ServerBuild | Promise [name, source] = getTransactionName(resolvedRoutes, url); isolationScope.setTransactionName(name); + + // Update the span name if we're running inside an existing span + const parentSpan = getActiveSpan(); + if (parentSpan) { + const rootSpan = getRootSpan(parentSpan); + rootSpan?.updateName(name); + } } isolationScope.setSDKProcessingMetadata({ normalizedRequest }); @@ -365,18 +413,18 @@ function instrumentBuildCallback( } if (!(wrappedRoute.module.loader as WrappedFunction).__sentry_original__) { - fill(wrappedRoute.module, 'loader', makeWrappedRootLoader()); + fill(wrappedRoute.module, 'loader', makeWrappedRootLoader(options?.instrumentTracing, build)); } } const routeAction = wrappedRoute.module.action as undefined | WrappedFunction; if (routeAction && !routeAction.__sentry_original__) { - fill(wrappedRoute.module, 'action', makeWrappedAction(id, options?.instrumentTracing)); + fill(wrappedRoute.module, 'action', makeWrappedAction(id, options?.instrumentTracing, build)); } const routeLoader = wrappedRoute.module.loader as undefined | WrappedFunction; if (routeLoader && !routeLoader.__sentry_original__) { - fill(wrappedRoute.module, 'loader', makeWrappedLoader(id, options?.instrumentTracing)); + fill(wrappedRoute.module, 'loader', makeWrappedLoader(id, options?.instrumentTracing, build)); } routes[id] = wrappedRoute; diff --git a/packages/remix/src/utils/utils.ts b/packages/remix/src/utils/utils.ts index 5485cff5e0a3..591b2f4009af 100644 --- a/packages/remix/src/utils/utils.ts +++ b/packages/remix/src/utils/utils.ts @@ -1,14 +1,14 @@ import type { ActionFunctionArgs, LoaderFunctionArgs, ServerBuild } from '@remix-run/node'; import type { AgnosticRouteObject } from '@remix-run/router'; import type { Span, TransactionSource } from '@sentry/core'; -import { debug } from '@sentry/core'; +import { debug, GLOBAL_OBJ } from '@sentry/core'; import { DEBUG_BUILD } from './debug-build'; import { getRequestMatch, matchServerRoutes } from './vendor/response'; type ServerRouteManifest = ServerBuild['routes']; /** - * + * Store configured FormData keys as span attributes for Remix actions. */ export async function storeFormDataKeys( args: LoaderFunctionArgs | ActionFunctionArgs, @@ -43,13 +43,103 @@ export async function storeFormDataKeys( } } +/** + * Converts Remix route IDs to parameterized paths at runtime. + * (e.g., "routes/users.$id" -> "/users/:id") + * + * @param routeId - The Remix route ID + * @returns The parameterized path + * @internal + */ +export function convertRemixRouteIdToPath(routeId: string): string { + // Remove the "routes/" prefix if present + const path = routeId.replace(/^routes\//, ''); + + // Handle root index route + if (path === 'index' || path === '_index') { + return '/'; + } + + // Split by dots to get segments + const segments = path.split('.'); + const pathSegments: string[] = []; + + for (let i = 0; i < segments.length; i++) { + const segment = segments[i]; + + if (!segment) { + continue; + } + + // Skip layout route segments (prefixed with _) + if (segment.startsWith('_') && segment !== '_index') { + continue; + } + + // Handle 'index' segments at the end (they don't add to the path) + if (segment === 'index' && i === segments.length - 1 && pathSegments.length > 0) { + continue; + } + + // Handle splat routes (catch-all) + // Remix accesses splat params via params["*"] at runtime + if (segment === '$') { + pathSegments.push(':*'); + continue; + } + + // Handle dynamic segments (prefixed with $) + if (segment.startsWith('$')) { + const paramName = segment.substring(1); + pathSegments.push(`:${paramName}`); + } else if (segment !== 'index') { + // Static segment (skip remaining 'index' segments) + pathSegments.push(segment); + } + } + + // Return with leading slash for consistency with client-side URL paths + const routePath = pathSegments.length > 0 ? `/${pathSegments.join('/')}` : '/'; + return routePath; +} + +/** + * Check if the Vite plugin manifest is available. + * @returns True if the manifest is available, false otherwise. + */ +function hasManifest(): boolean { + const globalWithInjectedManifest = GLOBAL_OBJ as typeof GLOBAL_OBJ & { + _sentryRemixRouteManifest: string | undefined; + }; + + return ( + !!globalWithInjectedManifest?._sentryRemixRouteManifest && + typeof globalWithInjectedManifest._sentryRemixRouteManifest === 'string' + ); +} + /** * Get transaction name from routes and url */ export function getTransactionName(routes: AgnosticRouteObject[], url: URL): [string, TransactionSource] { const matches = matchServerRoutes(routes, url.pathname); const match = matches && getRequestMatch(url, matches); - return match === null ? [url.pathname, 'url'] : [match.route.id || 'no-route-id', 'route']; + + if (match === null) { + return [url.pathname, 'url']; + } + + const routeId = match.route.id || 'no-route-id'; + + // Only use parameterized path if the Vite plugin manifest is available + // This ensures backward compatibility - without the plugin, we use the old behavior + if (hasManifest()) { + const parameterizedPath = convertRemixRouteIdToPath(routeId); + return [parameterizedPath, 'route']; + } + + // Fallback to old behavior (route ID) when manifest is not available + return [routeId, 'route']; } /** diff --git a/packages/remix/test/client/remixRouteParameterization.test.ts b/packages/remix/test/client/remixRouteParameterization.test.ts new file mode 100644 index 000000000000..59eb2c1796c1 --- /dev/null +++ b/packages/remix/test/client/remixRouteParameterization.test.ts @@ -0,0 +1,416 @@ +import { GLOBAL_OBJ } from '@sentry/core'; +import { afterEach, describe, expect, it } from 'vitest'; +import { maybeParameterizeRemixRoute } from '../../src/client/remixRouteParameterization'; +import type { RouteManifest } from '../../src/config/remixRouteManifest'; + +const globalWithInjectedManifest = GLOBAL_OBJ as typeof GLOBAL_OBJ & { + _sentryRemixRouteManifest: string | undefined; +}; + +describe('maybeParameterizeRemixRoute', () => { + const originalManifest = globalWithInjectedManifest._sentryRemixRouteManifest; + + afterEach(() => { + globalWithInjectedManifest._sentryRemixRouteManifest = originalManifest; + }); + + describe('when no manifest is available', () => { + it('should return undefined', () => { + globalWithInjectedManifest._sentryRemixRouteManifest = undefined; + + expect(maybeParameterizeRemixRoute('/users/123')).toBeUndefined(); + expect(maybeParameterizeRemixRoute('/blog/my-post')).toBeUndefined(); + expect(maybeParameterizeRemixRoute('/')).toBeUndefined(); + }); + }); + + describe('when manifest has static routes', () => { + it('should return undefined 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(); + }); + }); + + describe('when manifest has dynamic routes', () => { + it('should return parameterized routes for matching dynamic routes', () => { + const manifest: RouteManifest = { + staticRoutes: [{ path: '/' }, { path: '/about' }], + dynamicRoutes: [ + { + path: '/users/:id', + regex: '^/users/([^/]+)$', + paramNames: ['id'], + }, + { + path: '/blog/:slug', + regex: '^/blog/([^/]+)$', + paramNames: ['slug'], + }, + { + path: '/users/:userId/posts/:postId', + regex: '^/users/([^/]+)/posts/([^/]+)$', + paramNames: ['userId', 'postId'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRemixRoute('/users/123')).toBe('/users/:id'); + expect(maybeParameterizeRemixRoute('/users/john-doe')).toBe('/users/:id'); + expect(maybeParameterizeRemixRoute('/blog/my-post')).toBe('/blog/:slug'); + expect(maybeParameterizeRemixRoute('/blog/hello-world')).toBe('/blog/:slug'); + expect(maybeParameterizeRemixRoute('/users/123/posts/456')).toBe('/users/:userId/posts/:postId'); + expect(maybeParameterizeRemixRoute('/users/john/posts/my-post')).toBe('/users/:userId/posts/:postId'); + }); + + it('should return undefined for static routes even when dynamic routes exist', () => { + const manifest: RouteManifest = { + staticRoutes: [{ path: '/' }, { path: '/about' }], + dynamicRoutes: [ + { + path: '/users/:id', + regex: '^/users/([^/]+)$', + paramNames: ['id'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRemixRoute('/')).toBeUndefined(); + expect(maybeParameterizeRemixRoute('/about')).toBeUndefined(); + }); + + it('should handle splat/catch-all routes', () => { + const manifest: RouteManifest = { + staticRoutes: [{ path: '/' }], + dynamicRoutes: [ + { + path: '/docs/:*', + regex: '^/docs/(.+)$', + paramNames: ['*'], + }, + { + path: '/:*', + regex: '^/(.+)$', + paramNames: ['*'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRemixRoute('/docs/intro')).toBe('/docs/:*'); + expect(maybeParameterizeRemixRoute('/docs/guide/getting-started')).toBe('/docs/:*'); + expect(maybeParameterizeRemixRoute('/anything/else')).toBe('/:*'); + }); + + it('should handle routes with special characters', () => { + const manifest: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/users/:id/settings', + regex: '^/users/([^/]+)/settings$', + paramNames: ['id'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRemixRoute('/users/user-with-dashes/settings')).toBe('/users/:id/settings'); + expect(maybeParameterizeRemixRoute('/users/user_with_underscores/settings')).toBe('/users/:id/settings'); + expect(maybeParameterizeRemixRoute('/users/123/settings')).toBe('/users/:id/settings'); + }); + + it('should return the first matching dynamic route when sorted by specificity', () => { + const manifest: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/:*', + regex: '^/(.+)$', + paramNames: ['*'], + }, + { + path: '/users/:id', + regex: '^/users/([^/]+)$', + paramNames: ['id'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + // Should prefer more specific route over catch-all + expect(maybeParameterizeRemixRoute('/users/123')).toBe('/users/:id'); + expect(maybeParameterizeRemixRoute('/about/something')).toBe('/:*'); + }); + + it('should return undefined for dynamic routes without regex', () => { + const manifest: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/users/:id', + paramNames: ['id'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRemixRoute('/users/123')).toBeUndefined(); + }); + + it('should handle invalid regex patterns gracefully', () => { + const manifest: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/users/:id', + regex: '[invalid-regex', + paramNames: ['id'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRemixRoute('/users/123')).toBeUndefined(); + }); + }); + + describe('when route does not match any pattern', () => { + it('should return undefined for unknown routes', () => { + const manifest: RouteManifest = { + staticRoutes: [{ path: '/about' }], + dynamicRoutes: [ + { + path: '/users/:id', + regex: '^/users/([^/]+)$', + paramNames: ['id'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRemixRoute('/unknown')).toBeUndefined(); + expect(maybeParameterizeRemixRoute('/posts/123')).toBeUndefined(); + expect(maybeParameterizeRemixRoute('/users/123/extra')).toBeUndefined(); + }); + }); + + describe('edge cases', () => { + it('should handle empty route', () => { + const manifest: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRemixRoute('')).toBeUndefined(); + }); + + it('should handle root route', () => { + const manifest: RouteManifest = { + staticRoutes: [{ path: '/' }], + dynamicRoutes: [], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRemixRoute('/')).toBeUndefined(); + }); + + it('should handle complex nested dynamic routes', () => { + const manifest: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/api/v1/users/:id/posts/:postId/comments/:commentId', + regex: '^/api/v1/users/([^/]+)/posts/([^/]+)/comments/([^/]+)$', + paramNames: ['id', 'postId', 'commentId'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRemixRoute('/api/v1/users/123/posts/456/comments/789')).toBe( + '/api/v1/users/:id/posts/:postId/comments/:commentId', + ); + }); + }); + + describe('realistic Remix patterns', () => { + it.each([ + ['/', undefined], + ['/about', undefined], + ['/contact', undefined], + ['/blog/posts', undefined], + + ['/users/123', '/users/:id'], + ['/users/john-doe', '/users/:id'], + ['/blog/my-post', '/blog/:slug'], + ['/blog/hello-world', '/blog/:slug'], + ['/users/123/posts/456', '/users/:userId/posts/:postId'], + ['/users/john/posts/my-post', '/users/:userId/posts/:postId'], + + ['/docs/intro', '/docs/:*'], + ['/docs/guide/getting-started', '/docs/:*'], + + ['/unknown-route', undefined], + ['/api/unknown', undefined], + ])('should handle route "%s" and return %s', (inputRoute, expectedRoute) => { + const manifest: RouteManifest = { + staticRoutes: [{ path: '/' }, { path: '/about' }, { path: '/contact' }, { path: '/blog/posts' }], + dynamicRoutes: [ + { + path: '/users/:id', + regex: '^/users/([^/]+)$', + paramNames: ['id'], + }, + { + path: '/blog/:slug', + regex: '^/blog/([^/]+)$', + paramNames: ['slug'], + }, + { + path: '/users/:userId/posts/:postId', + regex: '^/users/([^/]+)/posts/([^/]+)$', + paramNames: ['userId', 'postId'], + }, + { + path: '/docs/:*', + regex: '^/docs/(.+)$', + paramNames: ['*'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + if (expectedRoute === undefined) { + expect(maybeParameterizeRemixRoute(inputRoute)).toBeUndefined(); + } else { + expect(maybeParameterizeRemixRoute(inputRoute)).toBe(expectedRoute); + } + }); + }); + + describe('route specificity and precedence', () => { + it('should prefer more specific routes over catch-all routes', () => { + const manifest: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/:parameter', + regex: '^/([^/]+)$', + paramNames: ['parameter'], + }, + { + path: '/:*', + regex: '^/(.+)$', + paramNames: ['*'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + // Single segment should match the specific route, not the catch-all + expect(maybeParameterizeRemixRoute('/123')).toBe('/:parameter'); + expect(maybeParameterizeRemixRoute('/abc')).toBe('/:parameter'); + + // Multiple segments should match the catch-all + expect(maybeParameterizeRemixRoute('/123/456')).toBe('/:*'); + }); + + it('should prefer routes with more static segments', () => { + const manifest: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/api/users/:id', + regex: '^/api/users/([^/]+)$', + paramNames: ['id'], + }, + { + path: '/api/:resource/:id', + regex: '^/api/([^/]+)/([^/]+)$', + paramNames: ['resource', 'id'], + }, + { + path: '/:*', + regex: '^/(.+)$', + paramNames: ['*'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + // More specific route with static segments should win + expect(maybeParameterizeRemixRoute('/api/users/123')).toBe('/api/users/:id'); + + // Less specific but still targeted route should win over catch-all + expect(maybeParameterizeRemixRoute('/api/posts/456')).toBe('/api/:resource/:id'); + + // Unmatched patterns should fall back to catch-all + expect(maybeParameterizeRemixRoute('/some/other/path')).toBe('/:*'); + }); + }); + + describe('caching behavior', () => { + it('should cache route results', () => { + const manifest: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/users/:id', + regex: '^/users/([^/]+)$', + paramNames: ['id'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest); + + const result1 = maybeParameterizeRemixRoute('/users/123'); + const result2 = maybeParameterizeRemixRoute('/users/123'); + + expect(result1).toBe(result2); + expect(result1).toBe('/users/:id'); + }); + + it('should clear cache when manifest changes', () => { + const manifest1: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/users/:id', + regex: '^/users/([^/]+)$', + paramNames: ['id'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest1); + + expect(maybeParameterizeRemixRoute('/users/123')).toBe('/users/:id'); + + // Change manifest + const manifest2: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/members/:id', + regex: '^/members/([^/]+)$', + paramNames: ['id'], + }, + ], + }; + globalWithInjectedManifest._sentryRemixRouteManifest = JSON.stringify(manifest2); + + expect(maybeParameterizeRemixRoute('/users/123')).toBeUndefined(); + expect(maybeParameterizeRemixRoute('/members/123')).toBe('/members/:id'); + }); + }); +}); diff --git a/packages/remix/test/config/manifest/createRemixRouteManifest.test.ts b/packages/remix/test/config/manifest/createRemixRouteManifest.test.ts new file mode 100644 index 000000000000..2af7e6d2ad00 --- /dev/null +++ b/packages/remix/test/config/manifest/createRemixRouteManifest.test.ts @@ -0,0 +1,136 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterAll, describe, expect, it } from 'vitest'; +import { createRemixRouteManifest } from '../../../src/config/createRemixRouteManifest'; + +describe('createRemixRouteManifest', () => { + const tempDirs: string[] = []; + + function createTestDir(): { tempDir: string; appDir: string; routesDir: string } { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'remix-test-')); + const appDir = path.join(tempDir, 'app'); + const routesDir = path.join(appDir, 'routes'); + fs.mkdirSync(routesDir, { recursive: true }); + tempDirs.push(tempDir); + return { tempDir, appDir, routesDir }; + } + + afterAll(() => { + // Clean up all temporary directories + tempDirs.forEach(dir => { + if (fs.existsSync(dir)) { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + }); + + describe('flat route structure', () => { + it('should handle basic flat routes', () => { + const { tempDir, routesDir } = createTestDir(); + + // Create test route files + fs.writeFileSync(path.join(routesDir, 'index.tsx'), '// index route'); + fs.writeFileSync(path.join(routesDir, 'about.tsx'), '// about route'); + fs.writeFileSync(path.join(routesDir, 'users.$id.tsx'), '// users dynamic route'); + + const manifest = createRemixRouteManifest({ rootDir: tempDir }); + + expect(manifest.staticRoutes).toHaveLength(2); + expect(manifest.staticRoutes).toContainEqual({ path: '/' }); + expect(manifest.staticRoutes).toContainEqual({ path: '/about' }); + + expect(manifest.dynamicRoutes).toHaveLength(1); + expect(manifest.dynamicRoutes[0]).toMatchObject({ + path: '/users/:id', + regex: '^/users/([^/]+)$', + paramNames: ['id'], + }); + + // Clean up + fs.unlinkSync(path.join(routesDir, 'index.tsx')); + fs.unlinkSync(path.join(routesDir, 'about.tsx')); + fs.unlinkSync(path.join(routesDir, 'users.$id.tsx')); + }); + }); + + describe('nested route structure', () => { + it('should handle nested directory routes', () => { + const { tempDir, routesDir } = createTestDir(); + const usersDir = path.join(routesDir, 'users'); + fs.mkdirSync(usersDir, { recursive: true }); + + fs.writeFileSync(path.join(routesDir, 'index.tsx'), '// root index'); + fs.writeFileSync(path.join(usersDir, '$id.tsx'), '// user id'); + fs.writeFileSync(path.join(usersDir, 'index.tsx'), '// users index'); + + const manifest = createRemixRouteManifest({ rootDir: tempDir }); + + expect(manifest.staticRoutes).toContainEqual({ path: '/' }); + expect(manifest.staticRoutes).toContainEqual({ path: '/users' }); + + expect(manifest.dynamicRoutes).toContainEqual( + expect.objectContaining({ + path: '/users/:id', + regex: '^/users/([^/]+)$', + paramNames: ['id'], + }), + ); + }); + + it('should handle deeply nested routes', () => { + const { tempDir, routesDir } = createTestDir(); + const usersDir = path.join(routesDir, 'users'); + const userIdDir = path.join(usersDir, '$id'); + const postsDir = path.join(userIdDir, 'posts'); + + fs.mkdirSync(postsDir, { recursive: true }); + + fs.writeFileSync(path.join(userIdDir, 'index.tsx'), '// user index'); + fs.writeFileSync(path.join(postsDir, '$postId.tsx'), '// post id'); + + const manifest = createRemixRouteManifest({ rootDir: tempDir }); + + // users/$id/index.tsx should map to /users/:id (dynamic route) + expect(manifest.dynamicRoutes).toContainEqual( + expect.objectContaining({ + path: '/users/:id', + regex: '^/users/([^/]+)$', + paramNames: ['id'], + }), + ); + + // users/$id/posts/$postId.tsx should map to /users/:id/posts/:postId + expect(manifest.dynamicRoutes).toContainEqual( + expect.objectContaining({ + path: '/users/:id/posts/:postId', + regex: '^/users/([^/]+)/posts/([^/]+)$', + paramNames: ['id', 'postId'], + }), + ); + }); + + it('should handle mixed flat and nested routes', () => { + const { tempDir, routesDir } = createTestDir(); + const usersDir = path.join(routesDir, 'users'); + fs.mkdirSync(usersDir, { recursive: true }); + + fs.writeFileSync(path.join(routesDir, 'index.tsx'), '// root'); + fs.writeFileSync(path.join(routesDir, 'about.tsx'), '// about'); + fs.writeFileSync(path.join(usersDir, '$id.tsx'), '// user'); + + const manifest = createRemixRouteManifest({ rootDir: tempDir }); + + expect(manifest.staticRoutes).toContainEqual({ path: '/' }); + expect(manifest.staticRoutes).toContainEqual({ path: '/about' }); + + expect(manifest.dynamicRoutes).toContainEqual( + expect.objectContaining({ + path: '/users/:id', + regex: '^/users/([^/]+)$', + paramNames: ['id'], + }), + ); + }); + }); +}); diff --git a/packages/remix/test/config/vite.test.ts b/packages/remix/test/config/vite.test.ts new file mode 100644 index 000000000000..c20888c81818 --- /dev/null +++ b/packages/remix/test/config/vite.test.ts @@ -0,0 +1,505 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import type { Plugin, ResolvedConfig } from 'vite'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { sentryRemixVitePlugin } from '../../src/config/vite'; + +describe('sentryRemixVitePlugin', () => { + let tempDir: string; + let appDir: string; + let routesDir: string; + let consoleLogSpy: ReturnType; + let consoleErrorSpy: ReturnType; + + beforeEach(() => { + // Create a temporary directory for test fixtures + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vite-plugin-test-')); + appDir = path.join(tempDir, 'app'); + routesDir = path.join(appDir, 'routes'); + fs.mkdirSync(routesDir, { recursive: true }); + + // Spy on console methods + consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + // Clean up + if (fs.existsSync(tempDir)) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + consoleLogSpy.mockRestore(); + consoleErrorSpy.mockRestore(); + }); + + describe('plugin configuration', () => { + it('should return a valid Vite plugin with correct name', () => { + const plugin = sentryRemixVitePlugin(); + + expect(plugin).toBeDefined(); + expect(plugin.name).toBe('sentry-remix-route-manifest'); + expect(plugin.enforce).toBe('post'); + }); + + it('should accept custom appDirPath option', () => { + const plugin = sentryRemixVitePlugin({ appDirPath: '/custom/path' }); + + expect(plugin).toBeDefined(); + }); + + it('should work with no options', () => { + const plugin = sentryRemixVitePlugin(); + + expect(plugin).toBeDefined(); + }); + }); + + describe('configResolved hook', () => { + it('should generate route manifest from routes directory', () => { + // Create test routes + fs.writeFileSync(path.join(routesDir, 'index.tsx'), '// index'); + fs.writeFileSync(path.join(routesDir, 'about.tsx'), '// about'); + fs.writeFileSync(path.join(routesDir, 'users.$id.tsx'), '// users'); + + const plugin = sentryRemixVitePlugin() as Plugin & { + configResolved: (config: ResolvedConfig) => void; + }; + + const mockConfig: Partial = { + root: tempDir, + command: 'build', + mode: 'production', + }; + + plugin.configResolved(mockConfig as ResolvedConfig); + + // Should not log in production mode + expect(consoleLogSpy).not.toHaveBeenCalled(); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('should log manifest info in development mode', () => { + fs.writeFileSync(path.join(routesDir, 'index.tsx'), '// index'); + fs.writeFileSync(path.join(routesDir, 'users.$id.tsx'), '// users'); + + const plugin = sentryRemixVitePlugin() as Plugin & { + configResolved: (config: ResolvedConfig) => void; + }; + + const mockConfig: Partial = { + root: tempDir, + command: 'serve', + mode: 'development', + }; + + plugin.configResolved(mockConfig as ResolvedConfig); + + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining('[Sentry Remix] Found 1 static and 1 dynamic routes'), + ); + }); + + it('should handle errors gracefully and set empty manifest', () => { + const plugin = sentryRemixVitePlugin({ appDirPath: '/nonexistent/path' }) as Plugin & { + configResolved: (config: ResolvedConfig) => void; + }; + + const mockConfig: Partial = { + root: tempDir, + command: 'build', + mode: 'production', + }; + + // Should not throw + expect(() => plugin.configResolved(mockConfig as ResolvedConfig)).not.toThrow(); + + // Should log error but not crash + expect(consoleErrorSpy).not.toHaveBeenCalled(); // No error if directory doesn't exist + }); + + it('should use custom appDirPath when provided', () => { + const customAppDir = path.join(tempDir, 'custom-app'); + const customRoutesDir = path.join(customAppDir, 'routes'); + fs.mkdirSync(customRoutesDir, { recursive: true }); + fs.writeFileSync(path.join(customRoutesDir, 'index.tsx'), '// index'); + + const plugin = sentryRemixVitePlugin({ appDirPath: customAppDir }) as Plugin & { + configResolved: (config: ResolvedConfig) => void; + }; + + const mockConfig: Partial = { + root: tempDir, + command: 'serve', + mode: 'development', + }; + + plugin.configResolved(mockConfig as ResolvedConfig); + + expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining('[Sentry Remix] Found 1 static')); + }); + }); + + describe('transformIndexHtml hook', () => { + it('should inject manifest into HTML with tag', () => { + fs.writeFileSync(path.join(routesDir, 'index.tsx'), '// index'); + + const plugin = sentryRemixVitePlugin() as Plugin & { + configResolved: (config: ResolvedConfig) => void; + transformIndexHtml: { + order: string; + handler: (html: string) => string; + }; + }; + + const mockConfig: Partial = { + root: tempDir, + command: 'build', + mode: 'production', + }; + + plugin.configResolved(mockConfig as ResolvedConfig); + + const html = 'Test'; + const result = plugin.transformIndexHtml.handler(html); + + expect(result).toContain(''); + expect(result).toContain('window._sentryRemixRouteManifest'); + expect(result).toContain('