From 06d5119444a2db5384aa26ccee4a02af50c56828 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 30 Jan 2026 19:38:41 +0000 Subject: [PATCH] feat(react-router): Add React Router Instrumentation API support --- .../.gitignore | 106 ++++ .../app/root.tsx | 43 ++ .../app/routes.ts | 8 + .../app/routes/about.tsx | 8 + .../app/routes/contact.tsx | 8 + .../app/routes/home.tsx | 8 + .../package.json | 27 + .../react-router.config.ts | 5 + .../tsconfig.json | 31 + .../vite.config.ts | 6 + .../react-router-instrumentation-api.test.ts | 239 ++++++++ e2e-tests/tests/react-router.test.ts | 432 +++++++++++++- src/react-router/codemods/client.entry.ts | 135 ++++- src/react-router/codemods/server-entry.ts | 441 ++++++++++---- src/react-router/codemods/vite.ts | 56 +- src/react-router/react-router-wizard.ts | 89 ++- src/react-router/sdk-setup.ts | 172 +++++- src/react-router/templates.ts | 87 ++- .../codemods/client-entry.test.ts | 127 +++- .../codemods/server-entry.test.ts | 542 +++++++++++++++++- test/react-router/codemods/vite.test.ts | 77 +++ test/react-router/sdk-setup.test.ts | 252 ++++++++ test/react-router/templates.test.ts | 143 +++++ 23 files changed, 2865 insertions(+), 177 deletions(-) create mode 100644 e2e-tests/test-applications/react-router-instrumentation-api-test-app/.gitignore create mode 100644 e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/root.tsx create mode 100644 e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes.ts create mode 100644 e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/about.tsx create mode 100644 e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/contact.tsx create mode 100644 e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/home.tsx create mode 100644 e2e-tests/test-applications/react-router-instrumentation-api-test-app/package.json create mode 100644 e2e-tests/test-applications/react-router-instrumentation-api-test-app/react-router.config.ts create mode 100644 e2e-tests/test-applications/react-router-instrumentation-api-test-app/tsconfig.json create mode 100644 e2e-tests/test-applications/react-router-instrumentation-api-test-app/vite.config.ts create mode 100644 e2e-tests/tests/react-router-instrumentation-api.test.ts diff --git a/e2e-tests/test-applications/react-router-instrumentation-api-test-app/.gitignore b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/.gitignore new file mode 100644 index 000000000..2920b0087 --- /dev/null +++ b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/.gitignore @@ -0,0 +1,106 @@ +# Dependencies +node_modules/ +/.pnp +.pnp.* + +# Build outputs +/build +/dist +/.react-router + +# Environment variables +.env +.env.local +.env.development.local +.env.test.local +.env.production.local + +# Logs +npm-debug.log* +yarn-debug.log* +yarn-error.log* +pnpm-debug.log* +lerna-debug.log* + +# Runtime data +pids +*.pid +*.seed +*.pid.lock + +# Coverage directory used by tools like istanbul +coverage/ +*.lcov + +# nyc test coverage +.nyc_output + +# Dependency directories +node_modules/ +jspm_packages/ + +# TypeScript cache +*.tsbuildinfo + +# Optional npm cache directory +.npm + +# Optional eslint cache +.eslintcache + +# Optional stylelint cache +.stylelintcache + +# Microbundle cache +.rpt2_cache/ +.rts2_cache_cjs/ +.rts2_cache_es/ +.rts2_cache_umd/ + +# Optional REPL history +.node_repl_history + +# Output of 'npm pack' +*.tgz + +# Yarn Integrity file +.yarn-integrity + +# parcel-bundler cache (https://parceljs.org/) +.cache +.parcel-cache + +# Next.js build output +.next + +# Nuxt.js build / generate output +.nuxt +dist + +# Storybook build outputs +.out +.storybook-out + +# Temporary folders +tmp/ +temp/ + +# Editor directories and files +.vscode/* +!.vscode/extensions.json +.idea +*.suo +*.ntvs* +*.njsproj +*.sln +*.sw? + +# OS generated files +.DS_Store +.DS_Store? +._* +.Spotlight-V100 +.Trashes +ehthumbs.db +Thumbs.db + diff --git a/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/root.tsx b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/root.tsx new file mode 100644 index 000000000..09c97e884 --- /dev/null +++ b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/root.tsx @@ -0,0 +1,43 @@ +import { + Links, + Meta, + Outlet, + Scripts, + ScrollRestoration, +} from "react-router"; + +export default function App() { + return ( + + + + + + + + +
+ + +
+ +
+
+ + + + + ); +} diff --git a/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes.ts b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes.ts new file mode 100644 index 000000000..fe4f425c7 --- /dev/null +++ b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes.ts @@ -0,0 +1,8 @@ +import type { RouteConfig } from "@react-router/dev/routes"; +import { index, route } from "@react-router/dev/routes"; + +export default [ + index("routes/home.tsx"), + route("/about", "routes/about.tsx"), + route("/contact", "routes/contact.tsx"), +] satisfies RouteConfig; diff --git a/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/about.tsx b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/about.tsx new file mode 100644 index 000000000..d9f6ece6d --- /dev/null +++ b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/about.tsx @@ -0,0 +1,8 @@ +export default function About() { + return ( +
+

About

+

This is a test application for React Router.

+
+ ); +} diff --git a/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/contact.tsx b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/contact.tsx new file mode 100644 index 000000000..ade65b990 --- /dev/null +++ b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/contact.tsx @@ -0,0 +1,8 @@ +export default function Contact() { + return ( +
+

Contact

+

Contact us for more information.

+
+ ); +} diff --git a/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/home.tsx b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/home.tsx new file mode 100644 index 000000000..c20ca8ef7 --- /dev/null +++ b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/app/routes/home.tsx @@ -0,0 +1,8 @@ +export default function Home() { + return ( +
+

Home

+

Welcome to the React Router test app!

+
+ ); +} diff --git a/e2e-tests/test-applications/react-router-instrumentation-api-test-app/package.json b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/package.json new file mode 100644 index 000000000..8fe1d18f9 --- /dev/null +++ b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/package.json @@ -0,0 +1,27 @@ +{ + "name": "react-router-instrumentation-api-test-app", + "private": true, + "version": "0.0.0", + "type": "module", + "scripts": { + "build": "react-router build", + "dev": "react-router dev", + "start": "react-router-serve ./build/server/index.js", + "typecheck": "react-router typegen && tsc" + }, + "dependencies": { + "@react-router/dev": "^7.9.5", + "@react-router/node": "^7.9.5", + "@react-router/serve": "^7.9.5", + "isbot": "^4.4.0", + "react": "^18.3.1", + "react-dom": "^18.3.1", + "react-router": "^7.9.5" + }, + "devDependencies": { + "@types/react": "^18.3.9", + "@types/react-dom": "^18.3.0", + "typescript": "^5.6.2", + "vite": "^6.0.1" + } +} diff --git a/e2e-tests/test-applications/react-router-instrumentation-api-test-app/react-router.config.ts b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/react-router.config.ts new file mode 100644 index 000000000..ad35e921f --- /dev/null +++ b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/react-router.config.ts @@ -0,0 +1,5 @@ +import type { Config } from "@react-router/dev/config"; + +export default { + // Config options +} satisfies Config; diff --git a/e2e-tests/test-applications/react-router-instrumentation-api-test-app/tsconfig.json b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/tsconfig.json new file mode 100644 index 000000000..31edcf03a --- /dev/null +++ b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/tsconfig.json @@ -0,0 +1,31 @@ +{ + "include": [ + "**/*.ts", + "**/*.tsx", + "**/.server/**/*.ts", + "**/.server/**/*.tsx", + "**/.client/**/*.ts", + "**/.client/**/*.tsx" + ], + "compilerOptions": { + "lib": ["DOM", "DOM.Iterable", "ES6"], + "isolatedModules": true, + "esModuleInterop": true, + "jsx": "react-jsx", + "module": "ESNext", + "moduleResolution": "Bundler", + "resolveJsonModule": true, + "target": "ES2022", + "strict": true, + "allowJs": true, + "skipLibCheck": true, + "forceConsistentCasingInFileNames": true, + "baseUrl": ".", + "paths": { + "~/*": ["./app/*"] + }, + + // React Router v7 specific options + "types": ["@react-router/dev"] + } +} diff --git a/e2e-tests/test-applications/react-router-instrumentation-api-test-app/vite.config.ts b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/vite.config.ts new file mode 100644 index 000000000..7ffae0548 --- /dev/null +++ b/e2e-tests/test-applications/react-router-instrumentation-api-test-app/vite.config.ts @@ -0,0 +1,6 @@ +import { reactRouter } from "@react-router/dev/vite"; +import { defineConfig } from "vite"; + +export default defineConfig({ + plugins: [reactRouter()], +}); diff --git a/e2e-tests/tests/react-router-instrumentation-api.test.ts b/e2e-tests/tests/react-router-instrumentation-api.test.ts new file mode 100644 index 000000000..68a7e65b0 --- /dev/null +++ b/e2e-tests/tests/react-router-instrumentation-api.test.ts @@ -0,0 +1,239 @@ +import * as fs from 'node:fs'; +import { Integration } from '../../lib/Constants'; +import { + checkEnvBuildPlugin, + checkFileContents, + checkFileExists, + checkIfBuilds, + checkIfRunsOnDevMode, + checkIfRunsOnProdMode, + checkPackageJson, + createIsolatedTestEnv, + getWizardCommand, +} from '../utils'; +import { afterAll, beforeAll, describe, test, expect } from 'vitest'; + +//@ts-expect-error - clifty is ESM only +import { KEYS, withEnv } from 'clifty'; + +/** + * Run the wizard on a React Router project with Instrumentation API enabled. + * This expects React Router >= 7.9.5 to be installed in the project. + */ +async function runWizardWithInstrumentationAPI( + projectDir: string, +): Promise { + const wizardInteraction = withEnv({ + cwd: projectDir, + }).defineInteraction(); + + wizardInteraction + .whenAsked('Please select your package manager.') + .respondWith(KEYS.DOWN, KEYS.ENTER) + .expectOutput('Installing @sentry/react-router') + .expectOutput('Installed @sentry/react-router', { + timeout: 240_000, + }) + + .whenAsked('Do you want to enable Tracing') + .respondWith(KEYS.ENTER) // Yes + .whenAsked('Do you want to enable Session Replay') + .respondWith(KEYS.ENTER) // Yes + .whenAsked('Do you want to enable Logs') + .respondWith(KEYS.ENTER) // Yes + .whenAsked('Do you want to enable Profiling') + .respondWith(KEYS.ENTER) // Yes + // Instrumentation API is part of feature selection (after profiling prompt, before package installations) + .whenAsked('Do you want to use the Instrumentation API') + .respondWith(KEYS.ENTER) // Yes + .expectOutput('Installing @sentry/profiling-node') + .expectOutput('Installed @sentry/profiling-node', { + timeout: 240_000, + }) + .whenAsked('Do you want to create an example page') + .respondWith(KEYS.ENTER); // Yes + + return wizardInteraction + .whenAsked( + 'Optionally add a project-scoped MCP server configuration for the Sentry MCP?', + ) + .respondWith(KEYS.DOWN, KEYS.ENTER) // No + .expectOutput('Successfully installed the Sentry React Router SDK!') + .run(getWizardCommand(Integration.reactRouter)); +} + +/** + * Run the wizard on a React Router project WITHOUT Instrumentation API. + * This tests the "No" path for the Instrumentation API prompt. + */ +async function runWizardWithoutInstrumentationAPI( + projectDir: string, +): Promise { + const wizardInteraction = withEnv({ + cwd: projectDir, + }).defineInteraction(); + + wizardInteraction + .whenAsked('Please select your package manager.') + .respondWith(KEYS.DOWN, KEYS.ENTER) + .expectOutput('Installing @sentry/react-router') + .expectOutput('Installed @sentry/react-router', { + timeout: 240_000, + }) + + .whenAsked('Do you want to enable Tracing') + .respondWith(KEYS.ENTER) // Yes + .whenAsked('Do you want to enable Session Replay') + .respondWith(KEYS.ENTER) // Yes + .whenAsked('Do you want to enable Logs') + .respondWith(KEYS.ENTER) // Yes + .whenAsked('Do you want to enable Profiling') + .respondWith(KEYS.ENTER) // Yes + // Instrumentation API is part of feature selection (after profiling prompt, before package installations) + .whenAsked('Do you want to use the Instrumentation API') + .respondWith(KEYS.DOWN, KEYS.ENTER) // No + .expectOutput('Installing @sentry/profiling-node') + .expectOutput('Installed @sentry/profiling-node', { + timeout: 240_000, + }) + .whenAsked('Do you want to create an example page') + .respondWith(KEYS.ENTER); // Yes + + return wizardInteraction + .whenAsked( + 'Optionally add a project-scoped MCP server configuration for the Sentry MCP?', + ) + .respondWith(KEYS.DOWN, KEYS.ENTER) // No + .expectOutput('Successfully installed the Sentry React Router SDK!') + .run(getWizardCommand(Integration.reactRouter)); +} + +describe('React Router Instrumentation API', () => { + describe('with Instrumentation API enabled', () => { + let wizardExitCode: number; + const { projectDir, cleanup } = createIsolatedTestEnv( + 'react-router-instrumentation-api-test-app', + ); + + beforeAll(async () => { + wizardExitCode = await runWizardWithInstrumentationAPI(projectDir); + }); + + afterAll(() => { + cleanup(); + }); + + test('exits with exit code 0', () => { + expect(wizardExitCode).toBe(0); + }); + + test('package.json is updated correctly', () => { + checkPackageJson(projectDir, '@sentry/react-router'); + }); + + test('.env.sentry-build-plugin is created and contains the auth token', () => { + checkEnvBuildPlugin(projectDir); + }); + + test('entry.client file contains Instrumentation API setup', () => { + checkFileContents(`${projectDir}/app/entry.client.tsx`, [ + 'import * as Sentry from', + '@sentry/react-router', + // Check for tracing variable with useInstrumentationAPI + 'const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });', + // Check that tracing is passed to integrations + 'integrations: [tracing', + // Check for unstable_instrumentations prop on HydratedRouter + 'unstable_instrumentations={[tracing.clientInstrumentation]}', + ]); + }); + + test('entry.server file contains unstable_instrumentations export', () => { + checkFileContents(`${projectDir}/app/entry.server.tsx`, [ + 'import * as Sentry from', + '@sentry/react-router', + 'export const handleError = Sentry.createSentryHandleError(', + // Check for unstable_instrumentations export with createSentryServerInstrumentation + 'export const unstable_instrumentations = [Sentry.createSentryServerInstrumentation()]', + ]); + }); + + test('instrument.server file exists', () => { + checkFileExists(`${projectDir}/instrument.server.mjs`); + }); + + test('example page exists', () => { + checkFileExists(`${projectDir}/app/routes/sentry-example-page.tsx`); + }); + + test('builds successfully', async () => { + await checkIfBuilds(projectDir); + }, 60_000); + + test('runs on dev mode correctly', async () => { + await checkIfRunsOnDevMode(projectDir, 'to expose'); + }, 30_000); + + test('runs on prod mode correctly', async () => { + await checkIfRunsOnProdMode(projectDir, 'react-router-serve'); + }, 30_000); + }); + + describe('with Instrumentation API disabled', () => { + let wizardExitCode: number; + const { projectDir, cleanup } = createIsolatedTestEnv( + 'react-router-instrumentation-api-test-app', + ); + + beforeAll(async () => { + wizardExitCode = await runWizardWithoutInstrumentationAPI(projectDir); + }); + + afterAll(() => { + cleanup(); + }); + + test('exits with exit code 0', () => { + expect(wizardExitCode).toBe(0); + }); + + test('entry.client file does NOT contain Instrumentation API setup', () => { + const clientContent = fs.readFileSync( + `${projectDir}/app/entry.client.tsx`, + 'utf8', + ); + + // Should have Sentry setup + expect(clientContent).toContain('import * as Sentry from'); + expect(clientContent).toContain('@sentry/react-router'); + + // Should NOT have instrumentation API specific code + expect(clientContent).not.toContain('useInstrumentationAPI: true'); + expect(clientContent).not.toContain('unstable_instrumentations'); + expect(clientContent).not.toContain('tracing.clientInstrumentation'); + + // Should have regular tracing integration + expect(clientContent).toContain('Sentry.reactRouterTracingIntegration()'); + }); + + test('entry.server file does NOT contain unstable_instrumentations export', () => { + const serverContent = fs.readFileSync( + `${projectDir}/app/entry.server.tsx`, + 'utf8', + ); + + // Should have Sentry setup + expect(serverContent).toContain('import * as Sentry from'); + expect(serverContent).toContain('@sentry/react-router'); + expect(serverContent).toContain('handleError'); + + // Should NOT have instrumentation API specific code + expect(serverContent).not.toContain('unstable_instrumentations'); + expect(serverContent).not.toContain('createSentryServerInstrumentation'); + }); + + test('builds successfully', async () => { + await checkIfBuilds(projectDir); + }, 60_000); + }); +}); diff --git a/e2e-tests/tests/react-router.test.ts b/e2e-tests/tests/react-router.test.ts index 7c1c5d7e4..d3395bbd8 100644 --- a/e2e-tests/tests/react-router.test.ts +++ b/e2e-tests/tests/react-router.test.ts @@ -21,16 +21,25 @@ import { KEYS, withEnv } from 'clifty'; async function runWizardOnReactRouterProject( projectDir: string, opts?: { - modifiedFiles?: boolean; + /** + * Set to true when entry files are modified/have existing Sentry - wizard will + * ask about reveal and manual snippet application + */ + modifiedEntryFiles?: boolean; + /** + * Set to true when any files are modified (dirty git state) - wizard will + * ask to continue anyway + */ + dirtyGitState?: boolean; }, ): Promise { - const { modifiedFiles = false } = opts || {}; + const { modifiedEntryFiles = false, dirtyGitState = false } = opts || {}; const wizardInteraction = withEnv({ cwd: projectDir, }).defineInteraction(); - if (modifiedFiles) { + if (dirtyGitState) { wizardInteraction .whenAsked('Do you want to continue anyway?') .respondWith(KEYS.ENTER); @@ -52,6 +61,10 @@ async function runWizardOnReactRouterProject( .respondWith(KEYS.ENTER) .whenAsked('Do you want to enable Profiling') .respondWith(KEYS.ENTER) + // Instrumentation API prompt appears for React Router >= 7.9.5 + // (detected from installed node_modules version, not package.json range) + .whenAsked('Do you want to use the Instrumentation API') + .respondWith(KEYS.ENTER) // Yes .expectOutput('Installing @sentry/profiling-node') .expectOutput('Installed @sentry/profiling-node', { timeout: 240_000, @@ -59,7 +72,7 @@ async function runWizardOnReactRouterProject( .whenAsked('Do you want to create an example page') .respondWith(KEYS.ENTER); - if (modifiedFiles) { + if (modifiedEntryFiles) { wizardInteraction .whenAsked('Would you like to try running npx react-router reveal') .respondWith(KEYS.ENTER) @@ -122,15 +135,19 @@ describe('React Router', () => { checkFileExists(`${projectDir}/instrument.server.mjs`); }); - test('entry.client file contains Sentry initialization', () => { + test('entry.client file contains Sentry initialization with Instrumentation API', () => { checkFileContents(`${projectDir}/app/entry.client.tsx`, [ 'import * as Sentry from', '@sentry/react-router', `Sentry.init({ dsn: "${TEST_ARGS.PROJECT_DSN}",`, - 'integrations: [Sentry.reactRouterTracingIntegration(), Sentry.replayIntegration()]', + // With Instrumentation API enabled, tracing is stored in a variable + 'const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });', + 'integrations: [tracing, Sentry.replayIntegration()]', 'enableLogs: true,', 'tracesSampleRate: 1.0,', + // HydratedRouter should have unstable_instrumentations prop + 'unstable_instrumentations={[tracing.clientInstrumentation]}', ]); }); @@ -141,12 +158,14 @@ describe('React Router', () => { ]); }); - test('entry.server file contains Sentry instrumentation', () => { + test('entry.server file contains Sentry instrumentation with Instrumentation API', () => { checkFileContents(`${projectDir}/app/entry.server.tsx`, [ 'import * as Sentry from', '@sentry/react-router', 'export const handleError = Sentry.createSentryHandleError(', 'export default Sentry.wrapSentryHandleRequest(handleRequest);', + // With Instrumentation API enabled, should have unstable_instrumentations export + 'export const unstable_instrumentations = [Sentry.createSentryServerInstrumentation()];', ]); }); @@ -236,7 +255,8 @@ startTransition(() => { fs.writeFileSync(clientEntryPath, existingContent); wizardExitCode = await runWizardOnReactRouterProject(projectDir, { - modifiedFiles: true, + modifiedEntryFiles: true, + dirtyGitState: true, }); }); @@ -340,5 +360,401 @@ startTransition(() => { checkFileExists(`${projectDir}/instrument.server.mjs`); }); }); + + describe('SPA mode (ssr: false)', () => { + let wizardExitCode: number; + + const { projectDir, cleanup } = createIsolatedTestEnv( + 'react-router-test-app', + ); + + beforeAll(async () => { + // Modify react-router.config.ts to have ssr: false (SPA mode) + const configPath = path.join(projectDir, 'react-router.config.ts'); + const spaConfigContent = `import type { Config } from "@react-router/dev/config"; + +export default { + ssr: false, +} satisfies Config; +`; + fs.writeFileSync(configPath, spaConfigContent); + + wizardExitCode = await runWizardOnReactRouterProject(projectDir, { + dirtyGitState: true, + }); + }); + + afterAll(() => { + cleanup(); + }); + + test('exits with exit code 0', () => { + expect(wizardExitCode).toBe(0); + }); + + test('wizard changes ssr: false to ssr: true for sourcemap uploads', () => { + checkFileContents(`${projectDir}/react-router.config.ts`, [ + 'ssr: true', + 'sentryOnBuildEnd', + ]); + }); + + test('react-router.config contains comment about SSR change', () => { + const configContent = fs.readFileSync( + `${projectDir}/react-router.config.ts`, + 'utf8', + ); + // The wizard should add a comment when changing ssr from false to true + expect(configContent).toContain('ssr'); + expect(configContent).toContain('true'); + }); + + test('builds successfully with changed SSR setting', async () => { + await checkIfBuilds(projectDir); + }, 60_000); + }); + + describe('existing ErrorBoundary function', () => { + let wizardExitCode: number; + + const { projectDir, cleanup } = createIsolatedTestEnv( + 'react-router-test-app', + ); + + beforeAll(async () => { + // Modify root.tsx to have an existing ErrorBoundary function + const rootPath = path.join(projectDir, 'app', 'root.tsx'); + const rootWithErrorBoundary = `import { + Links, + Meta, + Outlet, + Scripts, + ScrollRestoration, + isRouteErrorResponse, +} from "react-router"; + +export function ErrorBoundary({ error }: { error: unknown }) { + // Custom error handling logic + console.error('Custom error handler:', error); + + if (isRouteErrorResponse(error)) { + return ( +
+

{error.status} {error.statusText}

+

{error.data}

+
+ ); + } + + return ( +
+

Something went wrong!

+

{error instanceof Error ? error.message : 'Unknown error'}

+
+ ); +} + +export default function App() { + return ( + + + + + + + + +
+ + +
+ +
+
+ + + + + ); +} +`; + fs.writeFileSync(rootPath, rootWithErrorBoundary); + + wizardExitCode = await runWizardOnReactRouterProject(projectDir, { + dirtyGitState: true, + }); + }); + + afterAll(() => { + cleanup(); + }); + + test('exits with exit code 0', () => { + expect(wizardExitCode).toBe(0); + }); + + test('wizard adds Sentry.captureException to existing ErrorBoundary', () => { + checkFileContents(`${projectDir}/app/root.tsx`, [ + 'import * as Sentry from', + '@sentry/react-router', + 'export function ErrorBoundary', + 'Sentry.captureException(error)', + ]); + }); + + test('preserves existing ErrorBoundary logic', () => { + const rootContent = fs.readFileSync( + `${projectDir}/app/root.tsx`, + 'utf8', + ); + // Should preserve the custom error handling + expect(rootContent).toContain('isRouteErrorResponse'); + // Should only have one ErrorBoundary function + const errorBoundaryCount = ( + rootContent.match(/export function ErrorBoundary/g) || [] + ).length; + expect(errorBoundaryCount).toBe(1); + }); + + test('builds successfully', async () => { + await checkIfBuilds(projectDir); + }, 60_000); + }); + + describe('function-form defineConfig in vite.config', () => { + let wizardExitCode: number; + + const { projectDir, cleanup } = createIsolatedTestEnv( + 'react-router-test-app', + ); + + beforeAll(async () => { + // Modify vite.config.ts to use function-form defineConfig with identifier parameter + // Note: The sentryReactRouter plugin requires access to the full config object, + // so we use a simple identifier parameter (config) rather than destructuring + const viteConfigPath = path.join(projectDir, 'vite.config.ts'); + const functionFormViteConfig = `import { reactRouter } from "@react-router/dev/vite"; +import { defineConfig } from "vite"; + +export default defineConfig((config) => ({ + plugins: [reactRouter()], + define: { + __APP_MODE__: config.mode === 'development' ? '"dev"' : '"prod"', + }, +})); +`; + fs.writeFileSync(viteConfigPath, functionFormViteConfig); + + wizardExitCode = await runWizardOnReactRouterProject(projectDir, { + dirtyGitState: true, + }); + }); + + afterAll(() => { + cleanup(); + }); + + test('exits with exit code 0', () => { + expect(wizardExitCode).toBe(0); + }); + + test('wizard adds sentryReactRouter plugin to function-form config', () => { + checkFileContents(`${projectDir}/vite.config.ts`, [ + 'import { sentryReactRouter } from', + '@sentry/react-router', + 'sentryReactRouter(', + 'authToken: process.env.SENTRY_AUTH_TOKEN', + ]); + }); + + test('preserves function-form defineConfig structure', () => { + const viteContent = fs.readFileSync( + `${projectDir}/vite.config.ts`, + 'utf8', + ); + // Should still be using function form with config parameter + expect(viteContent).toMatch(/defineConfig\s*\(\s*\(?config\)?/); + // Should preserve the custom define + expect(viteContent).toContain('__APP_MODE__'); + }); + + test('builds successfully', async () => { + await checkIfBuilds(projectDir); + }, 60_000); + }); + + describe('destructured parameter in vite.config', () => { + let wizardExitCode: number; + + const { projectDir, cleanup } = createIsolatedTestEnv( + 'react-router-test-app', + ); + + beforeAll(async () => { + // Test critical fix: destructured params like ({ mode }) => ({ define: { x: mode } }) + // The wizard must convert expression body to block statement with destructuring + const viteConfigPath = path.join(projectDir, 'vite.config.ts'); + const destructuredViteConfig = `import { reactRouter } from "@react-router/dev/vite"; +import { defineConfig } from "vite"; + +export default defineConfig(({ mode }) => ({ + plugins: [reactRouter()], + define: { + __IS_DEV__: mode === 'development', + }, +})); +`; + fs.writeFileSync(viteConfigPath, destructuredViteConfig); + + wizardExitCode = await runWizardOnReactRouterProject(projectDir, { + dirtyGitState: true, + }); + }); + + afterAll(() => { + cleanup(); + }); + + test('exits with exit code 0', () => { + expect(wizardExitCode).toBe(0); + }); + + test('wizard rewrites destructured parameter and adds sentryReactRouter plugin', () => { + checkFileContents(`${projectDir}/vite.config.ts`, [ + 'sentryReactRouter(', + 'authToken: process.env.SENTRY_AUTH_TOKEN', + ]); + }); + + test('preserves destructured properties via added const declaration', () => { + const viteContent = fs.readFileSync( + `${projectDir}/vite.config.ts`, + 'utf8', + ); + // Should have config parameter (may or may not have parens around single param) + expect(viteContent).toMatch(/config\s*=>/); + // Should have destructuring statement + expect(viteContent).toContain('const {'); + expect(viteContent).toContain('mode'); + // Should still use mode in define + expect(viteContent).toContain('__IS_DEV__'); + }); + + test('builds successfully with rewritten destructured params', async () => { + await checkIfBuilds(projectDir); + }, 60_000); + }); + + describe('existing ErrorBoundary as arrow function', () => { + let wizardExitCode: number; + + const { projectDir, cleanup } = createIsolatedTestEnv( + 'react-router-test-app', + ); + + beforeAll(async () => { + // Modify root.tsx to have an existing ErrorBoundary as arrow function + const rootPath = path.join(projectDir, 'app', 'root.tsx'); + const rootWithArrowErrorBoundary = `import { + Links, + Meta, + Outlet, + Scripts, + ScrollRestoration, + isRouteErrorResponse, +} from "react-router"; + +export const ErrorBoundary = ({ error }: { error: unknown }) => { + // Custom error handling logic + console.error('Arrow function error handler:', error); + + if (isRouteErrorResponse(error)) { + return ( +
+

{error.status} {error.statusText}

+
+ ); + } + + return ( +
+

Error!

+

{error instanceof Error ? error.message : 'Unknown error'}

+
+ ); +}; + +export default function App() { + return ( + + + + + + + + + + + + + + ); +} +`; + fs.writeFileSync(rootPath, rootWithArrowErrorBoundary); + + wizardExitCode = await runWizardOnReactRouterProject(projectDir, { + dirtyGitState: true, + }); + }); + + afterAll(() => { + cleanup(); + }); + + test('exits with exit code 0', () => { + expect(wizardExitCode).toBe(0); + }); + + test('wizard adds Sentry.captureException to arrow function ErrorBoundary', () => { + checkFileContents(`${projectDir}/app/root.tsx`, [ + 'import * as Sentry from', + '@sentry/react-router', + 'export const ErrorBoundary', + 'Sentry.captureException(error)', + ]); + }); + + test('preserves arrow function ErrorBoundary structure', () => { + const rootContent = fs.readFileSync( + `${projectDir}/app/root.tsx`, + 'utf8', + ); + // Should still be using arrow function syntax + expect(rootContent).toMatch(/export const ErrorBoundary\s*=/); + // The export const should appear exactly once + const exportCount = ( + rootContent.match(/export const ErrorBoundary/g) || [] + ).length; + expect(exportCount).toBe(1); + }); + + test('builds successfully', async () => { + await checkIfBuilds(projectDir); + }, 60_000); + }); }); }); diff --git a/src/react-router/codemods/client.entry.ts b/src/react-router/codemods/client.entry.ts index ec0d52934..16578c16e 100644 --- a/src/react-router/codemods/client.entry.ts +++ b/src/react-router/codemods/client.entry.ts @@ -20,6 +20,7 @@ export async function instrumentClientEntry( enableTracing: boolean, enableReplay: boolean, enableLogs: boolean, + useInstrumentationAPI = false, ): Promise { const clientEntryAst = await loadFile(clientEntryPath); @@ -35,30 +36,58 @@ export async function instrumentClientEntry( local: 'Sentry', }); - const integrations = []; - if (enableTracing) { - integrations.push('Sentry.reactRouterTracingIntegration()'); - } - if (enableReplay) { - integrations.push('Sentry.replayIntegration()'); - } + let initContent: string; + + if (useInstrumentationAPI && enableTracing) { + // When using instrumentation API with tracing enabled + // We need to store the tracing integration in a variable to pass to HydratedRouter + const integrations = ['tracing']; + if (enableReplay) { + integrations.push('Sentry.replayIntegration()'); + } + + initContent = ` +const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true }); - const initContent = ` Sentry.init({ dsn: "${dsn}", sendDefaultPii: true, integrations: [${integrations.join(', ')}], ${enableLogs ? 'enableLogs: true,' : ''} - tracesSampleRate: ${enableTracing ? '1.0' : '0'},${ - enableTracing - ? '\n tracePropagationTargets: [/^\\//, /^https:\\/\\/yourserver\\.io\\/api/],' - : '' - }${ + tracesSampleRate: 1.0, + tracePropagationTargets: [/^\\//, /^https:\\/\\/yourserver\\.io\\/api/],${ enableReplay ? '\n replaysSessionSampleRate: 0.1,\n replaysOnErrorSampleRate: 1.0,' : '' } });`; + } else { + // Standard initialization without instrumentation API + const integrations = []; + if (enableTracing) { + integrations.push('Sentry.reactRouterTracingIntegration()'); + } + if (enableReplay) { + integrations.push('Sentry.replayIntegration()'); + } + + initContent = ` +Sentry.init({ + dsn: "${dsn}", + sendDefaultPii: true, + integrations: [${integrations.join(', ')}], + ${enableLogs ? 'enableLogs: true,' : ''} + tracesSampleRate: ${enableTracing ? '1.0' : '0'},${ + enableTracing + ? '\n tracePropagationTargets: [/^\\//, /^https:\\/\\/yourserver\\.io\\/api/],' + : '' + }${ + enableReplay + ? '\n replaysSessionSampleRate: 0.1,\n replaysOnErrorSampleRate: 1.0,' + : '' + } +});`; + } (clientEntryAst.$ast as t.Program).body.splice( getAfterImportsInsertionIndex(clientEntryAst.$ast as t.Program), @@ -66,5 +95,85 @@ Sentry.init({ ...recast.parse(initContent).program.body, ); + // If using instrumentation API, add unstable_instrumentations prop to HydratedRouter + if (useInstrumentationAPI && enableTracing) { + const hydratedRouterFound = addInstrumentationPropsToHydratedRouter( + clientEntryAst.$ast as t.Program, + ); + + if (!hydratedRouterFound) { + clack.log.warn( + `Could not find ${chalk.cyan( + 'HydratedRouter', + )} component in your client entry file.\n` + + `To use the Instrumentation API, manually add the ${chalk.cyan( + 'unstable_instrumentations', + )} prop:\n` + + ` ${chalk.green( + '', + )}`, + ); + } + } + await writeFile(clientEntryAst.$ast, clientEntryPath); } + +/** + * Finds HydratedRouter JSX element and adds the unstable_instrumentations prop. + * Returns true if HydratedRouter was found and modified, false otherwise. + */ +function addInstrumentationPropsToHydratedRouter(ast: t.Program): boolean { + let found = false; + + recast.visit(ast, { + visitJSXElement(path) { + const openingElement = path.node.openingElement; + + // Check if this is a HydratedRouter element + if ( + openingElement.name.type === 'JSXIdentifier' && + openingElement.name.name === 'HydratedRouter' + ) { + found = true; + + // Check if unstable_instrumentations prop already exists + const hasInstrumentationsProp = openingElement.attributes?.some( + (attr) => + attr.type === 'JSXAttribute' && + attr.name.type === 'JSXIdentifier' && + attr.name.name === 'unstable_instrumentations', + ); + + if (!hasInstrumentationsProp) { + // Create the unstable_instrumentations prop + // unstable_instrumentations={[tracing.clientInstrumentation]} + const instrumentationsProp = recast.types.builders.jsxAttribute( + recast.types.builders.jsxIdentifier('unstable_instrumentations'), + recast.types.builders.jsxExpressionContainer( + recast.types.builders.arrayExpression([ + recast.types.builders.memberExpression( + recast.types.builders.identifier('tracing'), + recast.types.builders.identifier('clientInstrumentation'), + ), + ]), + ), + ); + + // Add the prop to the opening element + if (!openingElement.attributes) { + openingElement.attributes = []; + } + openingElement.attributes.push(instrumentationsProp); + } + + // Stop traversing once we found HydratedRouter + return false; + } + + this.traverse(path); + }, + }); + + return found; +} diff --git a/src/react-router/codemods/server-entry.ts b/src/react-router/codemods/server-entry.ts index 67bd2a2db..d4ce5fcfe 100644 --- a/src/react-router/codemods/server-entry.ts +++ b/src/react-router/codemods/server-entry.ts @@ -27,6 +27,7 @@ import { getAfterImportsInsertionIndex } from './utils'; export async function instrumentServerEntry( serverEntryPath: string, + useInstrumentationAPI = false, ): Promise { const serverEntryAst = await loadFile(serverEntryPath); @@ -41,9 +42,68 @@ export async function instrumentServerEntry( instrumentHandleError(serverEntryAst); instrumentHandleRequest(serverEntryAst); + // Add unstable_instrumentations export when using instrumentation API + if (useInstrumentationAPI) { + instrumentUnstableInstrumentations(serverEntryAst); + } + await writeFile(serverEntryAst.$ast, serverEntryPath); } +/** + * Adds the unstable_instrumentations export for the Instrumentation API + * export const unstable_instrumentations = [Sentry.createSentryServerInstrumentation()]; + */ +export function instrumentUnstableInstrumentations( + originalEntryServerMod: ProxifiedModule, +): void { + const originalEntryServerModAST = originalEntryServerMod.$ast as t.Program; + + // Check if unstable_instrumentations export already exists + const hasUnstableInstrumentations = originalEntryServerModAST.body.some( + (node) => { + if ( + node.type !== 'ExportNamedDeclaration' || + node.declaration?.type !== 'VariableDeclaration' + ) { + return false; + } + + const declarations = node.declaration.declarations; + if (!declarations || declarations.length === 0) { + return false; + } + + const firstDeclaration = declarations[0]; + if (!firstDeclaration || firstDeclaration.type !== 'VariableDeclarator') { + return false; + } + + const id = firstDeclaration.id; + return ( + id && + id.type === 'Identifier' && + id.name === 'unstable_instrumentations' + ); + }, + ); + + if (hasUnstableInstrumentations) { + debug( + 'unstable_instrumentations export already exists, skipping adding it again', + ); + return; + } + + // Create the unstable_instrumentations export + const instrumentationsExport = recast.parse( + `export const unstable_instrumentations = [Sentry.createSentryServerInstrumentation()];`, + ).program.body[0]; + + // Add it at the end of the file + originalEntryServerModAST.body.push(instrumentationsExport); +} + export function instrumentHandleRequest( originalEntryServerMod: ProxifiedModule, ): void { @@ -185,15 +245,79 @@ export function instrumentHandleRequest( }, }); - // Replace the existing default export with the wrapped one - originalEntryServerModAST.body.splice( - defaultExportIndex, - 1, - // @ts-expect-error - declaration works here because the AST is proxified by magicast - defaultExportNode.declaration, - ); + // @ts-expect-error - declaration works here because the AST is proxified by magicast + const declaration = defaultExportNode.declaration; + + // Handle different export patterns: + // 1. `export default handleRequest;` (Identifier) - variable already defined, just remove export + // 2. `export default function handleRequest() {}` (FunctionDeclaration with id) - replace with declaration + // 3. `export default (request) => {}` (ArrowFunctionExpression) - wrap in variable declaration + // 4. `export default function() {}` (FunctionDeclaration without id) - wrap in variable declaration + // Note: `export default function() {}` is always parsed as FunctionDeclaration with id=null, not FunctionExpression - // Adding our wrapped export + // Track the identifier name to use in the wrapper + // Default to 'handleRequest' for anonymous functions + let wrapperIdentifierName = 'handleRequest'; + + if ( + declaration && + declaration.type === 'FunctionDeclaration' && + declaration.id + ) { + // Named function declaration - just remove the export keyword + // Use the function's own name for the wrapper + wrapperIdentifierName = declaration.id.name; + originalEntryServerModAST.body.splice( + defaultExportIndex, + 1, + declaration, + ); + } else if ( + declaration && + (declaration.type === 'ArrowFunctionExpression' || + declaration.type === 'FunctionExpression' || + (declaration.type === 'FunctionDeclaration' && !declaration.id)) + ) { + // Anonymous function - wrap in a variable declaration + // If it's a FunctionDeclaration without id, convert to FunctionExpression + let funcExpression = declaration; + if (declaration.type === 'FunctionDeclaration' && !declaration.id) { + funcExpression = recast.types.builders.functionExpression( + null, // id + declaration.params, + declaration.body, + ); + // Note: recast's functionExpression builder doesn't accept async/generator as constructor args + // (unlike @babel/types), so we must set them after creation to preserve these modifiers + funcExpression.async = declaration.async; + funcExpression.generator = declaration.generator; + } + const variableDeclaration = recast.types.builders.variableDeclaration( + 'const', + [ + recast.types.builders.variableDeclarator( + recast.types.builders.identifier('handleRequest'), + funcExpression, + ), + ], + ); + originalEntryServerModAST.body.splice( + defaultExportIndex, + 1, + variableDeclaration, + ); + // wrapperIdentifierName stays as 'handleRequest' since we created that variable + } else if (declaration && declaration.type === 'Identifier') { + // Identifier - the function is already declared elsewhere with a custom name + // Capture the actual identifier name to use in the wrapper + wrapperIdentifierName = declaration.name; + originalEntryServerModAST.body.splice(defaultExportIndex, 1); + } else { + // Other patterns - just remove export + originalEntryServerModAST.body.splice(defaultExportIndex, 1); + } + + // Adding our wrapped export using the captured identifier name originalEntryServerModAST.body.push( recast.types.builders.exportDefaultDeclaration( recast.types.builders.callExpression( @@ -201,7 +325,7 @@ export function instrumentHandleRequest( recast.types.builders.identifier('Sentry'), recast.types.builders.identifier('wrapSentryHandleRequest'), ), - [recast.types.builders.identifier('handleRequest')], + [recast.types.builders.identifier(wrapperIdentifierName)], ), ), ); @@ -247,9 +371,34 @@ export function instrumentHandleError( return id && id.type === 'Identifier' && id.name === 'handleError'; }); + // Check for named re-export pattern: export { handleError } + const handleErrorNamedReExport = originalEntryServerModAST.body.find( + (node) => { + if (node.type !== 'ExportNamedDeclaration' || node.declaration) { + return false; + } + + // Check specifiers for handleError + const specifiers = (node as any).specifiers; + if (!specifiers || specifiers.length === 0) { + return false; + } + + return specifiers.some( + (spec: any) => + spec.type === 'ExportSpecifier' && + ((spec.exported?.type === 'Identifier' && + spec.exported.name === 'handleError') || + (spec.local?.type === 'Identifier' && + spec.local.name === 'handleError')), + ); + }, + ); + if ( !handleErrorFunctionExport && - !handleErrorFunctionVariableDeclarationExport + !handleErrorFunctionVariableDeclarationExport && + !handleErrorNamedReExport ) { clack.log.warn( `Could not find function ${chalk.cyan( @@ -267,123 +416,177 @@ export function instrumentHandleError( 0, recast.types.builders.exportNamedDeclaration(implementation), ); - } else if ( - (handleErrorFunctionExport && - // @ts-expect-error - StatementKind works here because the AST is proxified by magicast - generateCode(handleErrorFunctionExport).code.includes( - 'captureException', - )) || - (handleErrorFunctionVariableDeclarationExport && - // @ts-expect-error - StatementKind works here because the AST is proxified by magicast - generateCode(handleErrorFunctionVariableDeclarationExport).code.includes( - 'captureException', - )) - ) { - debug( - 'Found captureException inside handleError, skipping adding it again', - ); - } else if ( - (handleErrorFunctionExport && - // @ts-expect-error - StatementKind works here because the AST is proxified by magicast - generateCode(handleErrorFunctionExport).code.includes( - 'createSentryHandleError', - )) || - (handleErrorFunctionVariableDeclarationExport && - // @ts-expect-error - StatementKind works here because the AST is proxified by magicast - generateCode(handleErrorFunctionVariableDeclarationExport).code.includes( - 'createSentryHandleError', - )) - ) { - debug('createSentryHandleError is already used, skipping adding it again'); - } else if (handleErrorFunctionExport) { - // Create the Sentry captureException call as an IfStatement - const sentryCall = recast.parse(`if (!request.signal.aborted) { - Sentry.captureException(error); -}`).program.body[0]; - - // Safely insert the Sentry call at the beginning of the handleError function body - // @ts-expect-error - declaration works here because the AST is proxified by magicast - const declaration = handleErrorFunctionExport.declaration; - if ( - declaration && - declaration.body && - declaration.body.body && - Array.isArray(declaration.body.body) - ) { - declaration.body.body.unshift(sentryCall); - } else { - debug( - 'Cannot safely access handleError function body, skipping instrumentation', + } else { + // For named re-exports, find the original variable declaration + // Note: Using StatementKind (from body.find) which is compatible with generateCode + let handleErrorVariableDeclaration: t.Statement | undefined; + if (handleErrorNamedReExport) { + handleErrorVariableDeclaration = originalEntryServerModAST.body.find( + (node) => { + if (node.type !== 'VariableDeclaration') { + return false; + } + const declarations = (node as any).declarations; + if (!declarations || declarations.length === 0) { + return false; + } + const firstDeclaration = declarations[0]; + return ( + firstDeclaration?.id?.type === 'Identifier' && + firstDeclaration.id.name === 'handleError' + ); + }, ); } - } else if (handleErrorFunctionVariableDeclarationExport) { - // Create the Sentry captureException call as an IfStatement - const sentryCall = recast.parse(`if (!request.signal.aborted) { - Sentry.captureException(error); -}`).program.body[0]; - // Safe access to existing handle error implementation with proper null checks - // We know this is ExportNamedDeclaration with VariableDeclaration from the earlier find - const exportDeclaration = - handleErrorFunctionVariableDeclarationExport as any; - if ( - !exportDeclaration.declaration || - exportDeclaration.declaration.type !== 'VariableDeclaration' || - !exportDeclaration.declaration.declarations || - exportDeclaration.declaration.declarations.length === 0 - ) { + // Check if handleError already has captureException or createSentryHandleError + const hasExistingCaptureException = + (handleErrorFunctionExport && + // @ts-expect-error - StatementKind works here because the AST is proxified by magicast + generateCode(handleErrorFunctionExport).code.includes( + 'captureException', + )) || + (handleErrorFunctionVariableDeclarationExport && + generateCode( + handleErrorFunctionVariableDeclarationExport as Parameters< + typeof generateCode + >[0], + ).code.includes('captureException')) || + (handleErrorVariableDeclaration && + // @ts-expect-error - StatementKind works here because the AST is proxified by magicast + generateCode(handleErrorVariableDeclaration).code.includes( + 'captureException', + )); + + const hasExistingCreateSentryHandleError = + (handleErrorFunctionExport && + // @ts-expect-error - StatementKind works here because the AST is proxified by magicast + generateCode(handleErrorFunctionExport).code.includes( + 'createSentryHandleError', + )) || + (handleErrorFunctionVariableDeclarationExport && + generateCode( + handleErrorFunctionVariableDeclarationExport as Parameters< + typeof generateCode + >[0], + ).code.includes('createSentryHandleError')) || + (handleErrorVariableDeclaration && + // @ts-expect-error - StatementKind works here because the AST is proxified by magicast + generateCode(handleErrorVariableDeclaration).code.includes( + 'createSentryHandleError', + )); + + if (hasExistingCaptureException) { debug( - 'Cannot safely access handleError variable declaration, skipping instrumentation', + 'Found captureException inside handleError, skipping adding it again', ); - return; - } - - const firstDeclaration = exportDeclaration.declaration.declarations[0]; - if ( - !firstDeclaration || - firstDeclaration.type !== 'VariableDeclarator' || - !firstDeclaration.init - ) { + } else if (hasExistingCreateSentryHandleError) { debug( - 'Cannot safely access handleError variable declarator init, skipping instrumentation', + 'createSentryHandleError is already used, skipping adding it again', ); - return; - } - - const existingHandleErrorImplementation = firstDeclaration.init; - const existingParams = existingHandleErrorImplementation.params; - const existingBody = existingHandleErrorImplementation.body; - - const requestParam = { - ...recast.types.builders.property( - 'init', - recast.types.builders.identifier('request'), // key - recast.types.builders.identifier('request'), // value - ), - shorthand: true, - }; - // Add error and {request} parameters to handleError function if not present - // When none of the parameters exist - if (existingParams.length === 0) { - existingParams.push( - recast.types.builders.identifier('error'), - recast.types.builders.objectPattern([requestParam]), + } else if (handleErrorNamedReExport) { + // Named re-export without Sentry instrumentation - we can't easily modify this pattern + // Throw an error to trigger manual instructions in the wizard, ensuring users don't + // proceed without proper error capture configuration + debug( + 'Found handleError as named re-export, cannot auto-instrument this pattern', ); - // When only error parameter exists - } else if (existingParams.length === 1) { - existingParams.push(recast.types.builders.objectPattern([requestParam])); - // When both parameters exist, but request is not destructured - } else if ( - existingParams[1].type === 'ObjectPattern' && - !existingParams[1].properties.some( - (prop: t.ObjectProperty) => - safeGetIdentifierName(prop.key) === 'request', - ) - ) { - existingParams[1].properties.push(requestParam); - } + throw new Error( + `Cannot auto-instrument handleError: found as named re-export (e.g., "export { handleError }"). ` + + `Please update your server entry file manually to capture server-side errors.`, + ); + } else if (handleErrorFunctionExport) { + // Create the Sentry captureException call as an IfStatement + const sentryCall = recast.parse(`if (!request.signal.aborted) { + Sentry.captureException(error); +}`).program.body[0]; + + // Safely insert the Sentry call at the beginning of the handleError function body + // @ts-expect-error - declaration works here because the AST is proxified by magicast + const declaration = handleErrorFunctionExport.declaration; + if ( + declaration && + declaration.body && + declaration.body.body && + Array.isArray(declaration.body.body) + ) { + declaration.body.body.unshift(sentryCall); + } else { + debug( + 'Cannot safely access handleError function body, skipping instrumentation', + ); + } + } else if (handleErrorFunctionVariableDeclarationExport) { + // Create the Sentry captureException call as an IfStatement + const sentryCall = recast.parse(`if (!request.signal.aborted) { + Sentry.captureException(error); +}`).program.body[0]; + + // Safe access to existing handle error implementation with proper null checks + // We know this is ExportNamedDeclaration with VariableDeclaration from the earlier find + const exportDeclaration = + handleErrorFunctionVariableDeclarationExport as any; + if ( + !exportDeclaration.declaration || + exportDeclaration.declaration.type !== 'VariableDeclaration' || + !exportDeclaration.declaration.declarations || + exportDeclaration.declaration.declarations.length === 0 + ) { + debug( + 'Cannot safely access handleError variable declaration, skipping instrumentation', + ); + return; + } + + const firstDeclaration = exportDeclaration.declaration.declarations[0]; + if ( + !firstDeclaration || + firstDeclaration.type !== 'VariableDeclarator' || + !firstDeclaration.init + ) { + debug( + 'Cannot safely access handleError variable declarator init, skipping instrumentation', + ); + return; + } - // Add the Sentry call to the function body - existingBody.body.push(sentryCall); + const existingHandleErrorImplementation = firstDeclaration.init; + const existingParams = existingHandleErrorImplementation.params; + const existingBody = existingHandleErrorImplementation.body; + + const requestParam = { + ...recast.types.builders.property( + 'init', + recast.types.builders.identifier('request'), // key + recast.types.builders.identifier('request'), // value + ), + shorthand: true, + }; + // Add error and {request} parameters to handleError function if not present + // When none of the parameters exist + if (existingParams.length === 0) { + existingParams.push( + recast.types.builders.identifier('error'), + recast.types.builders.objectPattern([requestParam]), + ); + // When only error parameter exists + } else if (existingParams.length === 1) { + existingParams.push( + recast.types.builders.objectPattern([requestParam]), + ); + // When both parameters exist, but request is not destructured + } else if ( + existingParams[1].type === 'ObjectPattern' && + !existingParams[1].properties.some( + (prop: t.ObjectProperty) => + safeGetIdentifierName(prop.key) === 'request', + ) + ) { + existingParams[1].properties.push(requestParam); + } + + // Add the Sentry call to the function body + existingBody.body.push(sentryCall); + } } } diff --git a/src/react-router/codemods/vite.ts b/src/react-router/codemods/vite.ts index 05678bf57..138965f64 100644 --- a/src/react-router/codemods/vite.ts +++ b/src/react-router/codemods/vite.ts @@ -57,11 +57,13 @@ function extractFromFunctionBody( * * @param orgSlug - Sentry organization slug * @param projectSlug - Sentry project slug + * @param configParamName - The parameter name to use for the Vite config (defaults to 'config') * @returns CallExpression node for the Sentry Vite plugin */ function createSentryPluginCall( orgSlug: string, projectSlug: string, + configParamName = 'config', ): t.CallExpression { const b = recast.types.builders; return b.callExpression(b.identifier('sentryReactRouter'), [ @@ -76,10 +78,53 @@ function createSentryPluginCall( ), ), ]), - b.identifier('config'), + b.identifier(configParamName), ]); } +/** + * Extracts the parameter name from a function expression. + * For destructured parameters, rewrites the function to use a simple identifier. + */ +function extractConfigParamName( + func: t.ArrowFunctionExpression | t.FunctionExpression, +): string { + const b = recast.types.builders; + + if (func.params.length === 0) { + func.params.push(b.identifier('config')); + return 'config'; + } + + const firstParam = func.params[0]; + if (firstParam.type === 'Identifier') { + return firstParam.name; + } + + // Destructured parameter - rewrite to use simple identifier + if (firstParam.type === 'ObjectPattern') { + func.params[0] = b.identifier('config'); + + // Convert expression body to block statement to add destructuring + if (func.body.type !== 'BlockStatement') { + const returnStatement = b.returnStatement(func.body); + func.body = b.blockStatement([returnStatement]); + } + + // Add destructuring at the beginning of the block + const destructureStatement = b.variableDeclaration('const', [ + b.variableDeclarator(firstParam, b.identifier('config')), + ]); + func.body.body.unshift(destructureStatement); + + return 'config'; + } + + // Other complex patterns - fall back to 'config' + func.params[0] = b.identifier('config'); + return 'config'; +} + export function addReactRouterPluginToViteConfig( program: t.Program, orgSlug: string, @@ -87,6 +132,7 @@ export function addReactRouterPluginToViteConfig( ): { success: boolean; wasConverted: boolean } { const b = recast.types.builders; let wasConverted = false; + let configParamName: string | undefined = 'config'; const defaultExport = program.body.find( (node) => node.type === 'ExportDefaultDeclaration', @@ -127,6 +173,8 @@ export function addReactRouterPluginToViteConfig( arg.type === 'FunctionExpression' ) { configObj = extractFromFunctionBody(arg.body); + // Extract the parameter name from the existing function + configParamName = extractConfigParamName(arg); } } @@ -135,7 +183,11 @@ export function addReactRouterPluginToViteConfig( } const pluginsProp = findProperty(configObj, 'plugins'); - const sentryPluginCall = createSentryPluginCall(orgSlug, projectSlug); + const sentryPluginCall = createSentryPluginCall( + orgSlug, + projectSlug, + configParamName, + ); if (!pluginsProp) { configObj.properties.push( diff --git a/src/react-router/react-router-wizard.ts b/src/react-router/react-router-wizard.ts index c79357663..b42290bee 100644 --- a/src/react-router/react-router-wizard.ts +++ b/src/react-router/react-router-wizard.ts @@ -24,6 +24,7 @@ import { debug } from '../utils/debug'; import { createExamplePage } from './sdk-example'; import { isReactRouterV7, + supportsInstrumentationAPI, runReactRouterReveal, initializeSentryOnEntryClient, instrumentRootRoute, @@ -62,6 +63,7 @@ async function runReactRouterWizardWithTelemetry( printWelcome({ wizardName: 'Sentry React Router Wizard', promoCode: options.promoCode, + telemetryEnabled: options.telemetryEnabled, }); const packageJson = await getPackageDotJson(); @@ -108,6 +110,12 @@ async function runReactRouterWizardWithTelemetry( alreadyInstalled: sentryAlreadyInstalled, }); + // Check if React Router supports the Instrumentation API (>= 7.9.5) + const instrumentationAPISupported = supportsInstrumentationAPI(packageJson); + + // Build the feature selection prompts + // Only offer instrumentation API if supported by the installed React Router version + // Note: The Instrumentation API requires tracing to be enabled to be useful const featureSelection = await featureSelectionPrompt([ { id: 'performance', @@ -137,7 +145,19 @@ async function runReactRouterWizardWithTelemetry( )} to track application performance in detail?`, enabledHint: 'recommended for production debugging', }, - ]); + ...(instrumentationAPISupported + ? [ + { + id: 'instrumentationAPI', + prompt: `Do you want to use the ${chalk.bold( + 'Instrumentation API', + )} for automatic tracing of loaders, actions, and middleware?`, + enabledHint: + 'recommended for React Router 7.9.5+ (requires Tracing)', + }, + ] + : []), + ] as const); if (featureSelection.profiling) { const profilingAlreadyInstalled = hasPackageInstalled( @@ -164,6 +184,14 @@ Please create your entry files manually using React Router v7 commands.`); } }); + // Determine if we should use the instrumentation API + // Only enable instrumentation API if tracing (performance) is also enabled, + // since the instrumentation API is used for automatic tracing of loaders, actions, etc. + const useInstrumentationAPI = + instrumentationAPISupported && + featureSelection.instrumentationAPI && + featureSelection.performance; + await traceStep('Initialize Sentry on client entry', async () => { try { await initializeSentryOnEntryClient( @@ -172,6 +200,7 @@ Please create your entry files manually using React Router v7 commands.`); featureSelection.replay, featureSelection.logs, typeScriptDetected, + useInstrumentationAPI, ); } catch (e) { clack.log.warn( @@ -187,6 +216,7 @@ Please create your entry files manually using React Router v7 commands.`); featureSelection.performance, featureSelection.replay, featureSelection.logs, + useInstrumentationAPI, ); await showCopyPasteInstructions({ @@ -220,21 +250,41 @@ Please create your entry files manually using React Router v7 commands.`); await traceStep('Instrument server entry', async () => { try { - await instrumentSentryOnEntryServer(typeScriptDetected); - } catch (e) { - clack.log.warn( - `Could not initialize Sentry on server entry automatically.`, + await instrumentSentryOnEntryServer( + typeScriptDetected, + useInstrumentationAPI, ); + } catch (e) { + // Check for specific named re-export error + const isNamedReExportError = + e instanceof Error && e.message.includes('named re-export'); + + if (isNamedReExportError) { + clack.log.warn( + `Found ${chalk.cyan( + 'handleError', + )} as a named re-export. This pattern cannot be auto-instrumented.\n` + + `Please update your server entry file to ensure server-side errors are captured.`, + ); + } else { + clack.log.warn( + `Could not initialize Sentry on server entry automatically.`, + ); + } const serverEntryFilename = `entry.server.${ typeScriptDetected ? 'tsx' : 'jsx' }`; - const manualServerContent = getManualServerEntryContent(); + const manualServerContent = getManualServerEntryContent( + useInstrumentationAPI, + ); await showCopyPasteInstructions({ filename: serverEntryFilename, codeSnippet: manualServerContent, - hint: 'This configures server-side request handling and error tracking', + hint: isNamedReExportError + ? 'Replace your named re-export with this to capture server-side errors' + : 'This configures server-side request handling and error tracking', }); debug(e); @@ -391,13 +441,20 @@ Please create your entry files manually using React Router v7 commands.`); selectedProject.slug, ); - clack.outro( - `${chalk.green('Successfully installed the Sentry React Router SDK!')}${ - createExamplePageSelection - ? `\n\nYou can validate your setup by visiting ${chalk.cyan( - '"/sentry-example-page"', - )} in your application.` - : '' - }`, - ); + clack.outro(buildOutroMessage(createExamplePageSelection)); +} + +function buildOutroMessage(shouldCreateExamplePage: boolean): string { + let msg = chalk.green('Successfully installed the Sentry React Router SDK!'); + + if (shouldCreateExamplePage) { + msg += `\n\nYou can validate your setup by visiting ${chalk.cyan( + '"/sentry-example-page"', + )} in your application.`; + } + + msg += `\n\nCheck out the SDK documentation for further configuration: +https://docs.sentry.io/platforms/javascript/guides/react-router/`; + + return msg; } diff --git a/src/react-router/sdk-setup.ts b/src/react-router/sdk-setup.ts index 493c95863..7b2309b15 100644 --- a/src/react-router/sdk-setup.ts +++ b/src/react-router/sdk-setup.ts @@ -5,11 +5,44 @@ import * as childProcess from 'child_process'; // @ts-expect-error - clack is ESM and TS complains about that. It works though import clack from '@clack/prompts'; import chalk from 'chalk'; -import { gte, minVersion } from 'semver'; +import { gte, minVersion, valid, coerce } from 'semver'; import type { PackageDotJson } from '../utils/package-json'; import { getPackageVersion } from '../utils/package-json'; import { debug } from '../utils/debug'; + +/** + * Attempts to get the actual installed version of a package from node_modules. + * This is more accurate than reading from package.json when users specify loose + * version ranges (like "7.x" or ">=7.0.0"). + * + * @returns The installed version string, or undefined if not found + */ +function getInstalledPackageVersion(packageName: string): string | undefined { + try { + const packageJsonPath = path.join( + process.cwd(), + 'node_modules', + packageName, + 'package.json', + ); + + if (!fs.existsSync(packageJsonPath)) { + return undefined; + } + + const packageJsonContent = fs.readFileSync(packageJsonPath, 'utf-8'); + const packageJson = JSON.parse(packageJsonContent) as { version?: string }; + return packageJson.version; + } catch (e) { + debug( + `Could not read installed version for ${packageName}: ${ + e instanceof Error ? e.message : String(e) + }`, + ); + return undefined; + } +} import { getSentryInstrumentationServerContent } from './templates'; import { instrumentRoot } from './codemods/root'; import { instrumentServerEntry } from './codemods/server-entry'; @@ -147,13 +180,111 @@ export function isReactRouterV7(packageJson: PackageDotJson): boolean { return false; } - const minVer = minVersion(reactRouterVersion); + try { + const minVer = minVersion(reactRouterVersion); + + if (!minVer) { + return false; + } + + // Extract major.minor.patch to handle pre-release versions correctly + // (e.g., 7.0.0-beta.1 should be considered v7) + const baseVersionStr = `${minVer.major}.${minVer.minor}.${minVer.patch}`; + return gte(baseVersionStr, '7.0.0'); + } catch { + // Handle invalid version strings gracefully + debug( + `Invalid version string for @react-router/dev: "${reactRouterVersion}"`, + ); + return false; + } +} + +/** + * Checks if React Router version supports the Instrumentation API (>= 7.9.5) + * The instrumentation API was introduced in React Router 7.9.5 and provides + * automatic span creation for loaders, actions, middleware, navigations, etc. + * + * This function first checks the actually installed version from node_modules, + * which is more accurate when users specify loose version ranges (like "7.x"). + * Falls back to package.json version range analysis if node_modules is unavailable. + */ +export function supportsInstrumentationAPI( + packageJson: PackageDotJson, +): boolean { + // First, try to get the actually installed version from node_modules + // This is more accurate for loose ranges like "7.x" or ">=7.0.0" + const installedVersion = getInstalledPackageVersion('@react-router/dev'); + + if (installedVersion) { + try { + // Use coerce to handle various version formats (including pre-release) + const coercedVersion = coerce(installedVersion); + if (coercedVersion && gte(coercedVersion.version, '7.9.5')) { + debug( + `Detected installed @react-router/dev version ${installedVersion} (>= 7.9.5), Instrumentation API supported`, + ); + return true; + } + + // Direct comparison for valid semver + if (valid(installedVersion)) { + const match = installedVersion.match(/^(\d+\.\d+\.\d+)/); + if (match) { + return gte(match[1], '7.9.5'); + } + return gte(installedVersion, '7.9.5'); + } + } catch (e) { + debug( + `Error checking installed version: ${ + e instanceof Error ? e.message : String(e) + }`, + ); + } + } - if (!minVer) { + // Fallback: Check version range from package.json + const reactRouterVersion = getPackageVersion( + '@react-router/dev', + packageJson, + ); + if (!reactRouterVersion) { return false; } - return gte(minVer, '7.0.0'); + try { + // If it's a concrete version (not a range), use direct comparison + // Note: Pre-release versions like "7.9.5-beta.1" are valid concrete versions + // but semver considers them less than "7.9.5", so we handle them specially + if (valid(reactRouterVersion)) { + // For pre-release versions, extract the base version and compare + // e.g., "7.9.5-beta.1" → compare "7.9.5" >= "7.9.5" + const match = reactRouterVersion.match(/^(\d+\.\d+\.\d+)/); + if (match) { + return gte(match[1], '7.9.5'); + } + return gte(reactRouterVersion, '7.9.5'); + } + + // For version ranges (e.g., "^7.9.5", "~7.10.0", ">=7.9.5") + // Use minVersion to get the lowest satisfying version + // Note: This may be conservative for loose ranges like "7.x" + const minVer = minVersion(reactRouterVersion); + if (!minVer) { + return false; + } + + // Extract major.minor.patch to handle pre-release versions in ranges correctly + const baseVersionStr = `${minVer.major}.${minVer.minor}.${minVer.patch}`; + return gte(baseVersionStr, '7.9.5'); + } catch { + // Handle invalid version strings gracefully + debug( + `Invalid version string for @react-router/dev: "${reactRouterVersion}"`, + ); + return false; + } } export async function initializeSentryOnEntryClient( @@ -162,6 +293,7 @@ export async function initializeSentryOnEntryClient( enableReplay: boolean, enableLogs: boolean, isTS: boolean, + useInstrumentationAPI = false, ): Promise { const clientEntryPath = getAppFilePath('entry.client', isTS); const clientEntryFilename = path.basename(clientEntryPath); @@ -174,10 +306,13 @@ export async function initializeSentryOnEntryClient( enableTracing, enableReplay, enableLogs, + useInstrumentationAPI, ); clack.log.success( - `Updated ${chalk.cyan(clientEntryFilename)} with Sentry initialization.`, + `Successfully updated ${chalk.cyan( + clientEntryFilename, + )} with Sentry initialization.`, ); } @@ -192,7 +327,9 @@ export async function instrumentRootRoute(isTS: boolean): Promise { } await instrumentRoot(rootFilename); - clack.log.success(`Updated ${chalk.cyan(rootFilename)} with ErrorBoundary.`); + clack.log.success( + `Successfully updated ${chalk.cyan(rootFilename)} with ErrorBoundary.`, + ); } export function createServerInstrumentationFile( @@ -214,7 +351,9 @@ export function createServerInstrumentationFile( ); fs.writeFileSync(instrumentationPath, content); - clack.log.success(`Created ${chalk.cyan(INSTRUMENTATION_FILE)}.`); + clack.log.success( + `Successfully created ${chalk.cyan(INSTRUMENTATION_FILE)}.`, + ); return instrumentationPath; } @@ -291,16 +430,19 @@ export async function updatePackageJsonScripts(): Promise { export async function instrumentSentryOnEntryServer( isTS: boolean, + useInstrumentationAPI = false, ): Promise { const serverEntryPath = getAppFilePath('entry.server', isTS); const serverEntryFilename = path.basename(serverEntryPath); await ensureEntryFileExists(serverEntryFilename, serverEntryPath); - await instrumentServerEntry(serverEntryPath); + await instrumentServerEntry(serverEntryPath, useInstrumentationAPI); clack.log.success( - `Updated ${chalk.cyan(serverEntryFilename)} with Sentry error handling.`, + `Successfully updated ${chalk.cyan( + serverEntryFilename, + )} with Sentry error handling.`, ); } @@ -316,7 +458,9 @@ export async function configureReactRouterVitePlugin( try { const { wasConverted } = await instrumentViteConfig(orgSlug, projectSlug); - clack.log.success(`Updated ${filename} with sentryReactRouter plugin.`); + clack.log.success( + `Successfully updated ${filename} with sentryReactRouter plugin.`, + ); if (wasConverted) { clack.log.info( @@ -349,9 +493,13 @@ export async function configureReactRouterConfig(isTS: boolean): Promise { const { ssrWasChanged } = await instrumentReactRouterConfig(isTS); if (fileExistedBefore) { - clack.log.success(`Updated ${filename} with Sentry buildEnd hook.`); + clack.log.success( + `Successfully updated ${filename} with Sentry buildEnd hook.`, + ); } else { - clack.log.success(`Created ${filename} with Sentry buildEnd hook.`); + clack.log.success( + `Successfully created ${filename} with Sentry buildEnd hook.`, + ); } if (ssrWasChanged) { diff --git a/src/react-router/templates.ts b/src/react-router/templates.ts index ccb35dcca..72c701f68 100644 --- a/src/react-router/templates.ts +++ b/src/react-router/templates.ts @@ -130,7 +130,66 @@ export const getManualClientEntryContent = ( enableTracing: boolean, enableReplay: boolean, enableLogs: boolean, + useInstrumentationAPI = false, ) => { + // When using instrumentation API with tracing, we store the integration in a variable + if (useInstrumentationAPI && enableTracing) { + const integrations = ['tracing']; + if (enableReplay) { + integrations.push('Sentry.replayIntegration()'); + } + + const integrationsStr = integrations.join(',\n '); + + return makeCodeSnippet(true, (unchanged, plus) => + unchanged(`${plus("import * as Sentry from '@sentry/react-router';")} +import { startTransition, StrictMode } from 'react'; +import { hydrateRoot } from 'react-dom/client'; +import { HydratedRouter } from 'react-router/dom'; + +${plus(`// Create the tracing integration with useInstrumentationAPI enabled +const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });`)} + +${plus(`Sentry.init({ + dsn: "${dsn}", + + // Adds request headers and IP for users, for more info visit: + // https://docs.sentry.io/platforms/javascript/guides/react-router/configuration/options/#sendDefaultPii + sendDefaultPii: true, + + integrations: [ + ${integrationsStr} + ], + + ${ + enableLogs + ? '// Enable logs to be sent to Sentry\n enableLogs: true,\n\n ' + : '' + }tracesSampleRate: 1.0, // Capture 100% of the transactions + + // Set \`tracePropagationTargets\` to declare which URL(s) should have trace propagation enabled + // In production, replace "yourserver.io" with your actual backend domain + tracePropagationTargets: [/^\\//, /^https:\\/\\/yourserver\\.io\\/api/],${ + enableReplay + ? '\n\n // Capture Replay for 10% of all sessions,\n // plus 100% of sessions with an error\n replaysSessionSampleRate: 0.1,\n replaysOnErrorSampleRate: 1.0,' + : '' + } +});`)} + +startTransition(() => { + hydrateRoot( + document, + + ${plus( + '', + )} + + ); +});`), + ); + } + + // Standard initialization without instrumentation API const integrations = []; if (enableTracing) { @@ -189,7 +248,33 @@ startTransition(() => { ); }; -export const getManualServerEntryContent = () => { +export const getManualServerEntryContent = (useInstrumentationAPI = false) => { + if (useInstrumentationAPI) { + return makeCodeSnippet(true, (unchanged, plus) => + unchanged(`${plus("import * as Sentry from '@sentry/react-router';")} +import { createReadableStreamFromReadable } from '@react-router/node'; +import { renderToPipeableStream } from 'react-dom/server'; +import { ServerRouter } from 'react-router'; + +${plus(`const handleRequest = Sentry.createSentryHandleRequest({ + ServerRouter, + renderToPipeableStream, + createReadableStreamFromReadable, +});`)} + +export default handleRequest; + +${plus(`export const handleError = Sentry.createSentryHandleError({ + logErrors: false +});`)} + +${plus(`// Enable automatic server-side instrumentation for loaders, actions, middleware +export const unstable_instrumentations = [Sentry.createSentryServerInstrumentation()];`)} + +// ... rest of your server entry`), + ); + } + return makeCodeSnippet(true, (unchanged, plus) => unchanged(`${plus("import * as Sentry from '@sentry/react-router';")} import { createReadableStreamFromReadable } from '@react-router/node'; diff --git a/test/react-router/codemods/client-entry.test.ts b/test/react-router/codemods/client-entry.test.ts index 5e5811dbd..9ae0e6679 100644 --- a/test/react-router/codemods/client-entry.test.ts +++ b/test/react-router/codemods/client-entry.test.ts @@ -29,7 +29,7 @@ describe('instrumentClientEntry', () => { tmpDir = path.join( fixturesDir, 'tmp', - `test-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + `test-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`, ); tmpFile = path.join(tmpDir, 'entry.client.tsx'); @@ -247,4 +247,129 @@ describe('instrumentClientEntry', () => { expect(modifiedContent).toContain('hydrateRoot'); expect(modifiedContent).toContain(''); }); + + describe('Instrumentation API', () => { + it('should add tracing variable and unstable_instrumentations prop when useInstrumentationAPI is true', async () => { + const basicContent = fs.readFileSync( + path.join(fixturesDir, 'basic.tsx'), + 'utf8', + ); + + fs.writeFileSync(tmpFile, basicContent); + + await instrumentClientEntry( + tmpFile, + 'test-dsn', + true, + false, + false, + true, + ); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should have the tracing variable + expect(modifiedContent).toContain( + 'const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });', + ); + + // Should pass tracing to integrations + expect(modifiedContent).toContain('integrations: [tracing]'); + + // Should add unstable_instrumentations prop to HydratedRouter + expect(modifiedContent).toContain( + 'unstable_instrumentations={[tracing.clientInstrumentation]}', + ); + }); + + it('should add tracing variable with replay integration when both are enabled', async () => { + const basicContent = fs.readFileSync( + path.join(fixturesDir, 'basic.tsx'), + 'utf8', + ); + + fs.writeFileSync(tmpFile, basicContent); + + await instrumentClientEntry(tmpFile, 'test-dsn', true, true, false, true); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should have the tracing variable + expect(modifiedContent).toContain( + 'const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });', + ); + + // Should pass both tracing and replay to integrations + expect(modifiedContent).toContain( + 'integrations: [tracing, Sentry.replayIntegration()]', + ); + + // Should add unstable_instrumentations prop to HydratedRouter + expect(modifiedContent).toContain( + 'unstable_instrumentations={[tracing.clientInstrumentation]}', + ); + }); + + it('should not use instrumentation API when tracing is disabled', async () => { + const basicContent = fs.readFileSync( + path.join(fixturesDir, 'basic.tsx'), + 'utf8', + ); + + fs.writeFileSync(tmpFile, basicContent); + + // useInstrumentationAPI is true but tracing is disabled + await instrumentClientEntry( + tmpFile, + 'test-dsn', + false, + true, + false, + true, + ); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should NOT have the tracing variable (instrumentation API requires tracing) + expect(modifiedContent).not.toContain( + 'const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });', + ); + + // Should NOT add unstable_instrumentations prop + expect(modifiedContent).not.toContain('unstable_instrumentations'); + }); + + it('should not use instrumentation API when useInstrumentationAPI is false', async () => { + const basicContent = fs.readFileSync( + path.join(fixturesDir, 'basic.tsx'), + 'utf8', + ); + + fs.writeFileSync(tmpFile, basicContent); + + await instrumentClientEntry( + tmpFile, + 'test-dsn', + true, + false, + false, + false, + ); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should NOT have the tracing variable + expect(modifiedContent).not.toContain( + 'const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });', + ); + + // Should have regular tracing integration call + expect(modifiedContent).toContain( + 'Sentry.reactRouterTracingIntegration()', + ); + + // Should NOT add unstable_instrumentations prop + expect(modifiedContent).not.toContain('unstable_instrumentations'); + }); + }); }); diff --git a/test/react-router/codemods/server-entry.test.ts b/test/react-router/codemods/server-entry.test.ts index c44544729..d525cab46 100644 --- a/test/react-router/codemods/server-entry.test.ts +++ b/test/react-router/codemods/server-entry.test.ts @@ -10,6 +10,7 @@ import { instrumentServerEntry, instrumentHandleRequest, instrumentHandleError, + instrumentUnstableInstrumentations, } from '../../../src/react-router/codemods/server-entry'; // @ts-expect-error - magicast is ESM and TS complains about that. It works though @@ -46,7 +47,7 @@ describe('instrumentServerEntry', () => { __dirname, 'fixtures', 'tmp', - `test-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + `test-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`, ); tmpFile = path.join(tmpDir, 'entry.server.tsx'); @@ -163,7 +164,9 @@ describe('instrumentHandleRequest', () => { __dirname, 'fixtures', 'tmp', - `handle-request-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + `handle-request-${Date.now()}-${Math.random() + .toString(36) + .substring(2, 11)}`, ); fs.mkdirSync(tmpDir, { recursive: true }); }); @@ -225,7 +228,9 @@ describe('instrumentHandleError', () => { __dirname, 'fixtures', 'tmp', - `handle-error-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + `handle-error-${Date.now()}-${Math.random() + .toString(36) + .substring(2, 11)}`, ); fs.mkdirSync(tmpDir, { recursive: true }); }); @@ -354,7 +359,9 @@ describe('instrumentHandleError AST manipulation edge cases', () => { __dirname, 'fixtures', 'tmp', - `ast-edge-cases-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + `ast-edge-cases-${Date.now()}-${Math.random() + .toString(36) + .substring(2, 11)}`, ); fs.mkdirSync(tmpDir, { recursive: true }); }); @@ -471,6 +478,217 @@ export function handleError(error: unknown) { return declarations[0]; // This line will throw the error }).toThrow('Cannot read properties of undefined'); }); + + it('should throw error for named re-export pattern: export { handleError }', async () => { + const content = ` +const handleError = (error: unknown, { request }: { request: Request }) => { + console.error('Error occurred:', error); +}; + +export { handleError }; +`; + const tempFile = path.join(tmpDir, 'entry.server.tsx'); + fs.writeFileSync(tempFile, content); + + const mod = await loadFile(tempFile); + + // Should throw error for named re-export pattern without Sentry instrumentation + // This ensures the wizard shows manual instructions to the user + expect(() => instrumentHandleError(mod)).toThrow( + /Cannot auto-instrument handleError.*named re-export/, + ); + }); + + it('should not create duplicate handleError when re-running on named re-export pattern', async () => { + const content = ` +import * as Sentry from "@sentry/react-router"; + +const handleError = (error: unknown, { request }: { request: Request }) => { + if (!request.signal.aborted) { + Sentry.captureException(error); + } +}; + +export { handleError }; +`; + const tempFile = path.join(tmpDir, 'entry.server.tsx'); + fs.writeFileSync(tempFile, content); + + const mod = await loadFile(tempFile); + + // Run instrumentation + expect(() => instrumentHandleError(mod)).not.toThrow(); + + const modifiedCode = generateCode(mod.$ast).code; + // Should not create duplicate handleError declarations + const handleErrorDeclarations = + modifiedCode.match( + /const handleError|let handleError|var handleError/g, + ) || []; + expect(handleErrorDeclarations.length).toBe(1); + + // Should not create an additional handleError export + const handleErrorExports = + modifiedCode.match( + /export \{ handleError \}|export const handleError|export function handleError/g, + ) || []; + expect(handleErrorExports.length).toBe(1); + }); +}); + +describe('Instrumentation API', () => { + const fixturesDir = path.join(__dirname, 'fixtures', 'server-entry'); + let tmpDir: string; + let tmpFile: string; + + beforeEach(() => { + vi.clearAllMocks(); + + // Create unique tmp directory for each test + tmpDir = path.join( + __dirname, + 'fixtures', + 'tmp', + `test-instrumentation-api-${Date.now()}-${Math.random() + .toString(36) + .substring(2, 11)}`, + ); + tmpFile = path.join(tmpDir, 'entry.server.tsx'); + + // Ensure tmp directory exists + fs.mkdirSync(tmpDir, { recursive: true }); + }); + + afterEach(() => { + // Clean up tmp directory + if (fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true }); + } + }); + + it('should add unstable_instrumentations export when useInstrumentationAPI is true', async () => { + const basicContent = fs.readFileSync( + path.join(fixturesDir, 'basic.tsx'), + 'utf8', + ); + + fs.writeFileSync(tmpFile, basicContent); + + await instrumentServerEntry(tmpFile, true); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should add Sentry import + expect(modifiedContent).toContain( + 'import * as Sentry from "@sentry/react-router";', + ); + + // Should add unstable_instrumentations export + expect(modifiedContent).toContain( + 'export const unstable_instrumentations = [Sentry.createSentryServerInstrumentation()];', + ); + }); + + it('should NOT add unstable_instrumentations export when useInstrumentationAPI is false', async () => { + const basicContent = fs.readFileSync( + path.join(fixturesDir, 'basic.tsx'), + 'utf8', + ); + + fs.writeFileSync(tmpFile, basicContent); + + await instrumentServerEntry(tmpFile, false); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should NOT add unstable_instrumentations export + expect(modifiedContent).not.toContain('unstable_instrumentations'); + expect(modifiedContent).not.toContain('createSentryServerInstrumentation'); + }); + + it('should NOT duplicate unstable_instrumentations export if already present', async () => { + const contentWithInstrumentations = ` +import { ServerRouter } from 'react-router'; +import * as Sentry from '@sentry/react-router'; + +export default function handleRequest() {} +export const handleError = () => {}; +export const unstable_instrumentations = [Sentry.createSentryServerInstrumentation()]; +`; + + fs.writeFileSync(tmpFile, contentWithInstrumentations); + + await instrumentServerEntry(tmpFile, true); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should only have one unstable_instrumentations export + const count = (modifiedContent.match(/unstable_instrumentations/g) || []) + .length; + expect(count).toBe(1); + }); +}); + +describe('instrumentUnstableInstrumentations', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = path.join( + __dirname, + 'fixtures', + 'tmp', + `unstable-instrumentations-${Date.now()}-${Math.random() + .toString(36) + .substring(2, 11)}`, + ); + fs.mkdirSync(tmpDir, { recursive: true }); + }); + + afterEach(() => { + if (fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true }); + } + }); + + it('should add unstable_instrumentations export to the end of the file', async () => { + const content = ` +import { ServerRouter } from 'react-router'; +import * as Sentry from '@sentry/react-router'; + +export default function handleRequest() {} +`; + const tempFile = path.join(tmpDir, 'entry.server.tsx'); + fs.writeFileSync(tempFile, content); + + const mod = await loadFile(tempFile); + instrumentUnstableInstrumentations(mod); + + const modifiedCode = generateCode(mod.$ast).code; + + // Should add unstable_instrumentations export + expect(modifiedCode).toContain( + 'export const unstable_instrumentations = [Sentry.createSentryServerInstrumentation()];', + ); + }); + + it('should not add unstable_instrumentations if already present', async () => { + const content = ` +import * as Sentry from '@sentry/react-router'; + +export default function handleRequest() {} +export const unstable_instrumentations = [Sentry.createSentryServerInstrumentation()]; +`; + const tempFile = path.join(tmpDir, 'entry.server.tsx'); + fs.writeFileSync(tempFile, content); + + const mod = await loadFile(tempFile); + const originalBodyLength = (mod.$ast as any).body.length; + + instrumentUnstableInstrumentations(mod); + + // Should not add another export + expect((mod.$ast as any).body.length).toBe(originalBodyLength); + }); }); // Test for Bug #1: Array access vulnerability @@ -486,7 +704,7 @@ describe('Array access vulnerability bugs', () => { __dirname, 'fixtures', 'tmp', - `test-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + `test-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`, ); tmpFile = path.join(tmpDir, 'entry.server.tsx'); @@ -555,3 +773,317 @@ describe('Array access vulnerability bugs', () => { expect(thrownError).toBeNull(); }); }); + +// Test for custom-named default export identifiers +describe('Custom-named default export identifiers', () => { + let tmpDir: string; + let tmpFile: string; + + beforeEach(() => { + vi.clearAllMocks(); + + tmpDir = path.join( + __dirname, + 'fixtures', + 'tmp', + `custom-identifier-${Date.now()}-${Math.random() + .toString(36) + .substring(2, 11)}`, + ); + tmpFile = path.join(tmpDir, 'entry.server.tsx'); + + fs.mkdirSync(tmpDir, { recursive: true }); + }); + + afterEach(() => { + if (fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true }); + } + }); + + it('should preserve custom identifier name when wrapping default export', async () => { + // This test verifies fix for: export default myRequestHandler produces + // export default Sentry.wrapSentryHandleRequest(myRequestHandler) + // instead of incorrectly using handleRequest + const content = `import type { EntryContext } from "react-router"; + +const myRequestHandler = (request: Request) => { + return new Response("hello"); +}; + +export default myRequestHandler;`; + + fs.writeFileSync(tmpFile, content); + + await instrumentServerEntry(tmpFile); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should add Sentry import + expect(modifiedContent).toContain( + 'import * as Sentry from "@sentry/react-router";', + ); + + // Should wrap using the ORIGINAL identifier name, not 'handleRequest' + expect(modifiedContent).toContain( + 'export default Sentry.wrapSentryHandleRequest(myRequestHandler);', + ); + + // Should NOT contain wrapSentryHandleRequest(handleRequest) + expect(modifiedContent).not.toContain( + 'wrapSentryHandleRequest(handleRequest)', + ); + + // Should preserve the original variable declaration + expect(modifiedContent).toContain('const myRequestHandler ='); + }); + + it('should preserve custom identifier name for differently named exports', async () => { + const content = `import type { EntryContext } from "react-router"; + +const serverHandler = async (request: Request) => { + const data = await fetchData(); + return new Response(JSON.stringify(data)); +}; + +export default serverHandler;`; + + fs.writeFileSync(tmpFile, content); + + await instrumentServerEntry(tmpFile); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should wrap using 'serverHandler', not 'handleRequest' + expect(modifiedContent).toContain( + 'export default Sentry.wrapSentryHandleRequest(serverHandler);', + ); + + // Should preserve the original variable + expect(modifiedContent).toContain('const serverHandler ='); + }); + + it('should use the function name when exporting a named function declaration', async () => { + const content = `import type { EntryContext } from "react-router"; + +export default function myCustomHandler(request: Request) { + return new Response("hello"); +}`; + + fs.writeFileSync(tmpFile, content); + + await instrumentServerEntry(tmpFile); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should wrap using 'myCustomHandler' + expect(modifiedContent).toContain( + 'export default Sentry.wrapSentryHandleRequest(myCustomHandler);', + ); + + // Should keep the function declaration without export default + expect(modifiedContent).toContain( + 'function myCustomHandler(request: Request)', + ); + }); +}); + +// Test for various default export patterns +describe('Default export patterns', () => { + let tmpDir: string; + let tmpFile: string; + + beforeEach(() => { + vi.clearAllMocks(); + + tmpDir = path.join( + __dirname, + 'fixtures', + 'tmp', + `export-patterns-${Date.now()}-${Math.random() + .toString(36) + .substring(2, 11)}`, + ); + tmpFile = path.join(tmpDir, 'entry.server.tsx'); + + fs.mkdirSync(tmpDir, { recursive: true }); + }); + + afterEach(() => { + if (fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true }); + } + }); + + it('should handle arrow function with separate export default', async () => { + // This was previously causing "does not match type string" error + const content = `import type { EntryContext } from "react-router"; + +const handleRequest = (request: Request) => { + return new Response("hello"); +}; + +export default handleRequest;`; + + fs.writeFileSync(tmpFile, content); + + await instrumentServerEntry(tmpFile); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should add Sentry import + expect(modifiedContent).toContain( + 'import * as Sentry from "@sentry/react-router";', + ); + + // Should wrap the handleRequest + expect(modifiedContent).toContain( + 'export default Sentry.wrapSentryHandleRequest(handleRequest);', + ); + + // Should preserve the original function + expect(modifiedContent).toContain( + 'const handleRequest = (request: Request)', + ); + }); + + it('should handle inline arrow function export', async () => { + const content = `import type { EntryContext } from "react-router"; + +export default (request: Request) => { + return new Response("hello"); +};`; + + fs.writeFileSync(tmpFile, content); + + await instrumentServerEntry(tmpFile); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should add Sentry import + expect(modifiedContent).toContain( + 'import * as Sentry from "@sentry/react-router";', + ); + + // Should create a variable for the anonymous function + expect(modifiedContent).toContain( + 'const handleRequest = (request: Request)', + ); + + // Should wrap the handleRequest + expect(modifiedContent).toContain( + 'export default Sentry.wrapSentryHandleRequest(handleRequest);', + ); + }); + + it('should handle inline function declaration export', async () => { + const content = `import type { EntryContext } from "react-router"; + +export default function handleRequest(request: Request) { + return new Response("hello"); +}`; + + fs.writeFileSync(tmpFile, content); + + await instrumentServerEntry(tmpFile); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should add Sentry import + expect(modifiedContent).toContain( + 'import * as Sentry from "@sentry/react-router";', + ); + + // Should keep the function declaration (without export default) + expect(modifiedContent).toContain( + 'function handleRequest(request: Request)', + ); + + // Should wrap the handleRequest + expect(modifiedContent).toContain( + 'export default Sentry.wrapSentryHandleRequest(handleRequest);', + ); + }); + + it('should handle anonymous function declaration export (no function name)', async () => { + const content = `import type { EntryContext } from "react-router"; + +export default function(request: Request) { + return new Response("hello"); +}`; + + fs.writeFileSync(tmpFile, content); + + await instrumentServerEntry(tmpFile); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should add Sentry import + expect(modifiedContent).toContain( + 'import * as Sentry from "@sentry/react-router";', + ); + + // Should create a variable for the anonymous function (converted to function expression) + expect(modifiedContent).toContain('const handleRequest = function('); + + // Should wrap the handleRequest + expect(modifiedContent).toContain( + 'export default Sentry.wrapSentryHandleRequest(handleRequest);', + ); + }); + + it('should handle async anonymous function declaration export', async () => { + const content = `import type { EntryContext } from "react-router"; + +export default async function(request: Request) { + await someAsyncOperation(); + return new Response("hello"); +}`; + + fs.writeFileSync(tmpFile, content); + + await instrumentServerEntry(tmpFile); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should add Sentry import + expect(modifiedContent).toContain( + 'import * as Sentry from "@sentry/react-router";', + ); + + // Should create a variable for the async anonymous function (preserving async keyword) + expect(modifiedContent).toContain('const handleRequest = async function('); + + // Should wrap the handleRequest + expect(modifiedContent).toContain( + 'export default Sentry.wrapSentryHandleRequest(handleRequest);', + ); + }); + + it('should handle generator anonymous function declaration export', async () => { + const content = `import type { EntryContext } from "react-router"; + +export default function*(request: Request) { + yield new Response("hello"); +}`; + + fs.writeFileSync(tmpFile, content); + + await instrumentServerEntry(tmpFile); + + const modifiedContent = fs.readFileSync(tmpFile, 'utf8'); + + // Should add Sentry import + expect(modifiedContent).toContain( + 'import * as Sentry from "@sentry/react-router";', + ); + + // Should create a variable for the generator anonymous function (preserving generator) + expect(modifiedContent).toContain('const handleRequest = function*('); + + // Should wrap the handleRequest + expect(modifiedContent).toContain( + 'export default Sentry.wrapSentryHandleRequest(handleRequest);', + ); + }); +}); diff --git a/test/react-router/codemods/vite.test.ts b/test/react-router/codemods/vite.test.ts index 9eba2da71..b7b74c23e 100644 --- a/test/react-router/codemods/vite.test.ts +++ b/test/react-router/codemods/vite.test.ts @@ -171,5 +171,82 @@ export default defineConfig(function(config) { expect(writtenConfig).toContain('org: "my-org"'); expect(writtenConfig).toContain('project: "my-project"'); }); + + it('should handle destructured parameter in expression-body arrow function', async () => { + // This tests the critical fix: defineConfig(({ mode }) => ({ define: { x: mode } })) + // The expression body must be converted to block statement with destructuring + // so the destructured properties remain accessible + const destructuredConfig = `import { defineConfig } from 'vite'; + +export default defineConfig(({ mode }) => ({ + plugins: [], + define: { + __MODE__: mode + } +}));`; + + const writtenFiles: Record = {}; + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.promises.readFile).mockResolvedValue(destructuredConfig); + vi.mocked(fs.promises.writeFile).mockImplementation( + (filePath, content) => { + writtenFiles[filePath as string] = content as string; + return Promise.resolve(); + }, + ); + + const result = await instrumentViteConfig('my-org', 'my-project'); + + expect(result.wasConverted).toBe(false); + + const writtenConfig = Object.values(writtenFiles)[0]; + // Should contain the Sentry plugin + expect(writtenConfig).toContain('sentryReactRouter'); + expect(writtenConfig).toContain('org: "my-org"'); + // Should rewrite to use 'config' parameter (may or may not have parens) + expect(writtenConfig).toMatch(/config\s*=>/); + // Should add destructuring statement inside block body + expect(writtenConfig).toContain('const {'); + expect(writtenConfig).toContain('mode'); + expect(writtenConfig).toContain('} = config'); + // Should convert to block statement with return + expect(writtenConfig).toContain('return'); + }); + + it('should handle destructured parameter with multiple properties', async () => { + const destructuredConfig = `import { defineConfig } from 'vite'; + +export default defineConfig(({ mode, command, isSsrBuild }) => { + console.log(mode, command); + return { + plugins: [] + }; +});`; + + const writtenFiles: Record = {}; + + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.promises.readFile).mockResolvedValue(destructuredConfig); + vi.mocked(fs.promises.writeFile).mockImplementation( + (filePath, content) => { + writtenFiles[filePath as string] = content as string; + return Promise.resolve(); + }, + ); + + const result = await instrumentViteConfig('my-org', 'my-project'); + + expect(result.wasConverted).toBe(false); + + const writtenConfig = Object.values(writtenFiles)[0]; + // Should contain the Sentry plugin + expect(writtenConfig).toContain('sentryReactRouter'); + // Should have rewritten the function to use 'config' parameter + // and added destructuring inside the function body + expect(writtenConfig).toContain('config'); + expect(writtenConfig).toContain('mode'); + expect(writtenConfig).toContain('command'); + }); }); }); diff --git a/test/react-router/sdk-setup.test.ts b/test/react-router/sdk-setup.test.ts index b55987900..d51322182 100644 --- a/test/react-router/sdk-setup.test.ts +++ b/test/react-router/sdk-setup.test.ts @@ -101,6 +101,7 @@ vi.mock('../../src/utils/clack', () => { import { isReactRouterV7, + supportsInstrumentationAPI, runReactRouterReveal, createServerInstrumentationFile, tryRevealAndGetManualInstructions, @@ -158,6 +159,257 @@ describe('React Router SDK Setup', () => { ); expect(isReactRouterV7({})).toBe(false); }); + + it('should return true for pre-release versions of v7', () => { + expect( + isReactRouterV7({ + dependencies: { '@react-router/dev': '7.0.0-beta.1' }, + }), + ).toBe(true); + expect( + isReactRouterV7({ + dependencies: { '@react-router/dev': '7.0.0-alpha.0' }, + }), + ).toBe(true); + expect( + isReactRouterV7({ dependencies: { '@react-router/dev': '7.0.0-0' } }), + ).toBe(true); + }); + + it('should return false for pre-release versions of v6', () => { + expect( + isReactRouterV7({ + dependencies: { '@react-router/dev': '6.28.0-beta.1' }, + }), + ).toBe(false); + }); + + it('should return false gracefully for invalid version strings', () => { + expect( + isReactRouterV7({ dependencies: { '@react-router/dev': 'invalid' } }), + ).toBe(false); + expect( + isReactRouterV7({ dependencies: { '@react-router/dev': '' } }), + ).toBe(false); + }); + }); + + describe('supportsInstrumentationAPI', () => { + it('should return true for React Router v7.9.5 or higher', () => { + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.9.5' }, + }), + ).toBe(true); + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '^7.9.5' }, + }), + ).toBe(true); + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.10.0' }, + }), + ).toBe(true); + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '8.0.0' }, + }), + ).toBe(true); + expect( + supportsInstrumentationAPI({ + devDependencies: { '@react-router/dev': '7.9.5' }, + }), + ).toBe(true); + }); + + it('should return false for React Router versions below v7.9.5', () => { + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.9.4' }, + }), + ).toBe(false); + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.0.0' }, + }), + ).toBe(false); + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.9.0' }, + }), + ).toBe(false); + }); + + it('should return false when @react-router/dev is not installed', () => { + expect( + supportsInstrumentationAPI({ + dependencies: { react: '^18.0.0' }, + }), + ).toBe(false); + expect(supportsInstrumentationAPI({})).toBe(false); + }); + + it('should return true for pre-release versions of v7.9.5 or higher', () => { + // Pre-release versions should be treated as supporting the API + // since they represent builds of the target version + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.9.5-beta.1' }, + }), + ).toBe(true); + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.9.5-alpha.0' }, + }), + ).toBe(true); + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.9.5-0' }, + }), + ).toBe(true); + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.10.0-alpha.0' }, + }), + ).toBe(true); + }); + + it('should return false for pre-release versions below v7.9.5', () => { + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.9.4-beta.1' }, + }), + ).toBe(false); + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.0.0-alpha.0' }, + }), + ).toBe(false); + }); + + it('should handle build metadata versions correctly', () => { + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.9.5+build123' }, + }), + ).toBe(true); + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.9.4+build123' }, + }), + ).toBe(false); + }); + + it('should return false gracefully for invalid version strings', () => { + // Invalid versions should not throw, just return false + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': 'invalid' }, + }), + ).toBe(false); + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '' }, + }), + ).toBe(false); + }); + + it('should handle semver range specifiers correctly', () => { + // Caret range + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '^7.9.5' }, + }), + ).toBe(true); + // Tilde range + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '~7.9.5' }, + }), + ).toBe(true); + // Greater than or equal + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '>=7.9.5' }, + }), + ).toBe(true); + // X-range (7.x starts at 7.0.0) - when node_modules is not available + // this falls back to minVersion which returns false + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.x' }, + }), + ).toBe(false); + }); + + it('should detect support from installed node_modules version for loose ranges', () => { + // Mock the installed version from node_modules + existsSyncMock.mockImplementation((filePath: string) => { + if (filePath.includes('node_modules/@react-router/dev/package.json')) { + return true; + } + return false; + }); + + readFileSyncMock.mockImplementation((filePath: string) => { + if (filePath.includes('node_modules/@react-router/dev/package.json')) { + return JSON.stringify({ version: '7.12.0' }); + } + return ''; + }); + + // With loose range "7.x" in package.json but actual installed version 7.12.0 + // the function should return true because the installed version >= 7.9.5 + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.x' }, + }), + ).toBe(true); + }); + + it('should fall back to package.json range when node_modules is unavailable', () => { + // Mock that node_modules doesn't exist + existsSyncMock.mockReturnValue(false); + + // Falls back to minVersion which returns false for "7.x" + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.x' }, + }), + ).toBe(false); + + // Falls back to minVersion which returns true for "^7.9.5" + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '^7.9.5' }, + }), + ).toBe(true); + }); + + it('should return false when installed version is below 7.9.5 even for loose ranges', () => { + // Mock an old installed version + existsSyncMock.mockImplementation((filePath: string) => { + if (filePath.includes('node_modules/@react-router/dev/package.json')) { + return true; + } + return false; + }); + + readFileSyncMock.mockImplementation((filePath: string) => { + if (filePath.includes('node_modules/@react-router/dev/package.json')) { + return JSON.stringify({ version: '7.5.0' }); + } + return ''; + }); + + // Even with loose range, if installed version is old, return false + expect( + supportsInstrumentationAPI({ + dependencies: { '@react-router/dev': '7.x' }, + }), + ).toBe(false); + }); }); describe('generateServerInstrumentation', () => { diff --git a/test/react-router/templates.test.ts b/test/react-router/templates.test.ts index d9d6fe4c6..a964d55fe 100644 --- a/test/react-router/templates.test.ts +++ b/test/react-router/templates.test.ts @@ -177,6 +177,119 @@ describe('React Router Templates', () => { expect(result).not.toContain('enableLogs: true'); expect(result).toContain('integrations: ['); }); + + describe('Instrumentation API', () => { + it('should generate client entry with instrumentation API enabled', () => { + const dsn = 'https://test.sentry.io/123'; + const enableTracing = true; + const enableReplay = false; + const enableLogs = false; + const useInstrumentationAPI = true; + + const result = getManualClientEntryContent( + dsn, + enableTracing, + enableReplay, + enableLogs, + useInstrumentationAPI, + ); + + // Should have the tracing variable + expect(result).toContain( + 'const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });', + ); + + // Should pass tracing to integrations + expect(result).toContain('integrations: ['); + expect(result).toContain('tracing'); + + // Should add unstable_instrumentations prop to HydratedRouter + expect(result).toContain( + 'unstable_instrumentations={[tracing.clientInstrumentation]}', + ); + }); + + it('should generate client entry with instrumentation API and replay enabled', () => { + const dsn = 'https://test.sentry.io/123'; + const enableTracing = true; + const enableReplay = true; + const enableLogs = false; + const useInstrumentationAPI = true; + + const result = getManualClientEntryContent( + dsn, + enableTracing, + enableReplay, + enableLogs, + useInstrumentationAPI, + ); + + // Should have the tracing variable + expect(result).toContain( + 'const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });', + ); + + // Should pass both tracing and replay to integrations + expect(result).toContain('tracing'); + expect(result).toContain('Sentry.replayIntegration()'); + + // Should add unstable_instrumentations prop to HydratedRouter + expect(result).toContain( + 'unstable_instrumentations={[tracing.clientInstrumentation]}', + ); + }); + + it('should not use instrumentation API when tracing is disabled', () => { + const dsn = 'https://test.sentry.io/123'; + const enableTracing = false; + const enableReplay = true; + const enableLogs = false; + const useInstrumentationAPI = true; + + const result = getManualClientEntryContent( + dsn, + enableTracing, + enableReplay, + enableLogs, + useInstrumentationAPI, + ); + + // Should NOT have the tracing variable (instrumentation API requires tracing) + expect(result).not.toContain( + 'const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });', + ); + + // Should NOT add unstable_instrumentations prop + expect(result).not.toContain('unstable_instrumentations'); + }); + + it('should not use instrumentation API when explicitly disabled', () => { + const dsn = 'https://test.sentry.io/123'; + const enableTracing = true; + const enableReplay = false; + const enableLogs = false; + const useInstrumentationAPI = false; + + const result = getManualClientEntryContent( + dsn, + enableTracing, + enableReplay, + enableLogs, + useInstrumentationAPI, + ); + + // Should NOT have the tracing variable + expect(result).not.toContain( + 'const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });', + ); + + // Should have regular tracing integration call + expect(result).toContain('Sentry.reactRouterTracingIntegration()'); + + // Should NOT add unstable_instrumentations prop + expect(result).not.toContain('unstable_instrumentations'); + }); + }); }); describe('getManualServerEntryContent', () => { @@ -199,6 +312,36 @@ describe('React Router Templates', () => { expect(result).toContain('export default handleRequest'); expect(result).toContain('rest of your server entry'); }); + + describe('Instrumentation API', () => { + it('should generate server entry with unstable_instrumentations when useInstrumentationAPI is true', () => { + const result = getManualServerEntryContent(true); + + expect(result).toContain( + "+ import * as Sentry from '@sentry/react-router'", + ); + expect(result).toContain( + 'export const unstable_instrumentations = [Sentry.createSentryServerInstrumentation()];', + ); + expect(result).toContain( + 'Enable automatic server-side instrumentation for loaders, actions, middleware', + ); + }); + + it('should NOT include unstable_instrumentations when useInstrumentationAPI is false', () => { + const result = getManualServerEntryContent(false); + + expect(result).not.toContain('unstable_instrumentations'); + expect(result).not.toContain('createSentryServerInstrumentation'); + }); + + it('should default to no instrumentation API when parameter is omitted', () => { + const result = getManualServerEntryContent(); + + expect(result).not.toContain('unstable_instrumentations'); + expect(result).not.toContain('createSentryServerInstrumentation'); + }); + }); }); describe('getManualRootContent', () => {