diff --git a/dev-packages/e2e-tests/test-applications/nextjs-16/tests/build-output.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-16/tests/build-output.test.ts index 735e04649c32..8c8ed57fc7f9 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-16/tests/build-output.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-16/tests/build-output.test.ts @@ -1,9 +1,32 @@ import { expect, test } from '@playwright/test'; import { findAbsolutePathImports } from '@sentry-internal/test-utils'; +import * as fs from 'fs'; import * as path from 'path'; +import { isDevMode } from './isDevMode'; test('emits no absolute-path imports into the server output', () => { const leaks = findAbsolutePathImports({ outputDir: path.join(process.cwd(), '.next', 'server') }); expect(leaks).toEqual([]); }); + +// This app has no `pages` directory, so `withSentryConfig` lets the bundler drop the Pages Router navigation +// instrumentation and its `next/router` import (~80 KB raw). +test('does not ship the Pages Router runtime in the App Router client bundle', () => { + test.skip(isDevMode, 'Only production builds are tree-shaken'); + + const buildManifest = JSON.parse(fs.readFileSync(path.join(process.cwd(), '.next', 'build-manifest.json'), 'utf8')); + const rootMainFiles: string[] = buildManifest.rootMainFiles; + expect(rootMainFiles.length).toBeGreaterThan(0); + + const appClientBundle = rootMainFiles + .map(file => fs.readFileSync(path.join(process.cwd(), '.next', file), 'utf8')) + .join('\n'); + + // Control: the Sentry client is in these files. + expect(appClientBundle).toContain('auto.pageload.nextjs.pages_router_instrumentation'); + + expect(appClientBundle).not.toContain('auto.navigation.nextjs.pages_router_instrumentation'); + // Only Next.js' Pages Router runtime contains this event name. + expect(appClientBundle).not.toContain('beforeHistoryChange'); +}); diff --git a/packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts index 72a15550172c..f9a31f3f0e79 100644 --- a/packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts @@ -1,7 +1,8 @@ import type { Client } from '@sentry/core'; import { WINDOW } from '@sentry/react'; import { appRouterInstrumentNavigation, appRouterInstrumentPageLoad } from './appRouterRoutingInstrumentation'; -import { pagesRouterInstrumentNavigation, pagesRouterInstrumentPageLoad } from './pagesRouterRoutingInstrumentation'; +import { pagesRouterInstrumentNavigation } from './pagesRouterNavigationInstrumentation'; +import { pagesRouterInstrumentPageLoad } from './pagesRouterRoutingInstrumentation'; /** * Instruments the Next.js Client Router for page loads. @@ -22,7 +23,10 @@ export function nextRouterInstrumentNavigation(client: Client): void { const isAppRouter = !WINDOW.document.getElementById('__NEXT_DATA__'); if (isAppRouter) { appRouterInstrumentNavigation(client); - } else { + } else if (process.env._sentryHasPagesRouter !== 'false') { + // `withSentryConfig` inlines `'false'` for App Router-only projects, so bundlers drop this module and its + // `next/router` import (the whole Pages Router runtime). Pageload stays: App Router builds still serve + // `404.html`/`500.html` through the Pages Router, and it does not need `next/router`. pagesRouterInstrumentNavigation(client); } } diff --git a/packages/nextjs/src/client/routing/pagesRouterNavigationInstrumentation.ts b/packages/nextjs/src/client/routing/pagesRouterNavigationInstrumentation.ts new file mode 100644 index 000000000000..b94eadc5c134 --- /dev/null +++ b/packages/nextjs/src/client/routing/pagesRouterNavigationInstrumentation.ts @@ -0,0 +1,116 @@ +import type { Client, TransactionSource } from '@sentry/core'; +import { + hasSpanStreamingEnabled, + NAVIGATION_SPAN_NAME_FALLBACK, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + stripUrlQueryAndFragment, +} from '@sentry/core'; +import { getAbsoluteUrl, startBrowserTracingNavigationSpan, WINDOW } from '@sentry/react'; +import RouterImport from 'next/router'; +import { SENTRY_OP, SENTRY_SEGMENT_NAME_SOURCE, URL_TEMPLATE } from '@sentry/conventions/attributes'; +import { NAVIGATION } from '@sentry/conventions/op'; + +// next/router v10 is CJS +// +// For ESM/CJS interoperability 'reasons', depending on how this file is loaded, Router might be on the default export +const Router: typeof RouterImport = RouterImport.events + ? RouterImport + : (RouterImport as unknown as { default: typeof RouterImport }).default; + +const globalObject = WINDOW; + +/** + * Instruments the Next.js pages router for navigation. + * Only supported for client side routing. Works for Next >= 10. + * + * Leverages the SingletonRouter from the `next/router` to + * generate pageload/navigation transactions and parameterize + * transaction names. + */ +export function pagesRouterInstrumentNavigation(client: Client): void { + Router.events.on('routeChangeStart', (navigationTarget: string) => { + const strippedNavigationTarget = stripUrlQueryAndFragment(navigationTarget); + const matchedRoute = getNextRouteFromPathname(strippedNavigationTarget); + + let newLocation: string; + let spanSource: TransactionSource; + + if (matchedRoute) { + newLocation = matchedRoute; + spanSource = 'route'; + } else { + newLocation = strippedNavigationTarget; + spanSource = 'url'; + } + + startBrowserTracingNavigationSpan( + client, + { + // With span streaming, span names have to be low cardinality, so we can't fall back to the URL. + name: spanSource === 'route' || !hasSpanStreamingEnabled(client) ? newLocation : NAVIGATION_SPAN_NAME_FALLBACK, + attributes: { + [SENTRY_OP]: NAVIGATION, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.nextjs.pages_router_instrumentation', + [SENTRY_SEGMENT_NAME_SOURCE]: spanSource, + ...(spanSource === 'route' && { [URL_TEMPLATE]: newLocation }), + }, + }, + { url: getAbsoluteUrl(navigationTarget) }, + ); + }); +} + +function getNextRouteFromPathname(pathname: string): string | undefined { + const pageRoutes = globalObject.__BUILD_MANIFEST?.sortedPages; + + // Page route should in 99.999% of the cases be defined by now but just to be sure we make a check here + if (!pageRoutes) { + return; + } + + return pageRoutes.find(route => { + const routeRegExp = convertNextRouteToRegExp(route); + return pathname.match(routeRegExp); + }); +} + +/** + * Converts a Next.js style route to a regular expression that matches on pathnames (no query params or URL fragments). + * + * In general this involves replacing any instances of square brackets in a route with a wildcard: + * e.g. "/users/[id]/info" becomes /\/users\/([^/]+?)\/info/ + * + * Some additional edgecases need to be considered: + * - All routes have an optional slash at the end, meaning users can navigate to "/users/[id]/info" or + * "/users/[id]/info/" - both will be resolved to "/users/[id]/info". + * - Non-optional "catchall"s at the end of a route must be considered when matching (e.g. "/users/[...params]"). + * - Optional "catchall"s at the end of a route must be considered when matching (e.g. "/users/[[...params]]"). + * + * @param route A Next.js style route as it is found in `global.__BUILD_MANIFEST.sortedPages` + */ +function convertNextRouteToRegExp(route: string): RegExp { + // We can assume a route is at least "/". + const routeParts = route.split('/'); + + let optionalCatchallWildcardRegex = ''; + if (routeParts[routeParts.length - 1]?.match(/^\[\[\.\.\..+\]\]$/)) { + // If last route part has pattern "[[...xyz]]" we pop the latest route part to get rid of the required trailing + // slash that would come before it if we didn't pop it. + routeParts.pop(); + optionalCatchallWildcardRegex = '(?:/(.+?))?'; + } + + const rejoinedRouteParts = routeParts + .map( + routePart => + routePart + .replace(/^\[\.\.\..+\]$/, '(.+?)') // Replace catch all wildcard with regex wildcard + .replace(/^\[.*\]$/, '([^/]+?)'), // Replace route wildcards with lazy regex wildcards + ) + .join('/'); + + // oxlint-disable-next-line sdk/no-regexp-constructor -- routeParts are from the build manifest, so no raw user input + return new RegExp( + `^${rejoinedRouteParts}${optionalCatchallWildcardRegex}(?:/)?$`, // optional slash at the end + ); +} diff --git a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts index 1aee7a2e0c58..8cbf116334e4 100644 --- a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts @@ -1,32 +1,17 @@ -import type { Client, TransactionSource } from '@sentry/core'; +import type { Client } from '@sentry/core'; import { debug, hasSpanStreamingEnabled, - NAVIGATION_SPAN_NAME_FALLBACK, PAGELOAD_SPAN_NAME_FALLBACK, parseBaggageHeader, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - stripUrlQueryAndFragment, } from '@sentry/core'; -import { - getAbsoluteUrl, - startBrowserTracingNavigationSpan, - startBrowserTracingPageLoadSpan, - WINDOW, -} from '@sentry/react'; +import { startBrowserTracingPageLoadSpan, WINDOW } from '@sentry/react'; import type { NEXT_DATA } from 'next/dist/shared/lib/utils'; -import RouterImport from 'next/router'; import type { ParsedUrlQuery } from 'querystring'; import { DEBUG_BUILD } from '../../common/debug-build'; import { SENTRY_OP, SENTRY_SEGMENT_NAME_SOURCE, URL_TEMPLATE } from '@sentry/conventions/attributes'; -import { NAVIGATION, PAGELOAD } from '@sentry/conventions/op'; - -// next/router v10 is CJS -// -// For ESM/CJS interoperability 'reasons', depending on how this file is loaded, Router might be on the default export -const Router: typeof RouterImport = RouterImport.events - ? RouterImport - : (RouterImport as unknown as { default: typeof RouterImport }).default; +import { PAGELOAD } from '@sentry/conventions/op'; const globalObject = WINDOW; @@ -137,99 +122,3 @@ export function pagesRouterInstrumentPageLoad(client: Client): void { { sentryTrace, baggage }, ); } - -/** - * Instruments the Next.js pages router for navigation. - * Only supported for client side routing. Works for Next >= 10. - * - * Leverages the SingletonRouter from the `next/router` to - * generate pageload/navigation transactions and parameterize - * transaction names. - */ -export function pagesRouterInstrumentNavigation(client: Client): void { - Router.events.on('routeChangeStart', (navigationTarget: string) => { - const strippedNavigationTarget = stripUrlQueryAndFragment(navigationTarget); - const matchedRoute = getNextRouteFromPathname(strippedNavigationTarget); - - let newLocation: string; - let spanSource: TransactionSource; - - if (matchedRoute) { - newLocation = matchedRoute; - spanSource = 'route'; - } else { - newLocation = strippedNavigationTarget; - spanSource = 'url'; - } - - startBrowserTracingNavigationSpan( - client, - { - // With span streaming, span names have to be low cardinality, so we can't fall back to the URL. - name: spanSource === 'route' || !hasSpanStreamingEnabled(client) ? newLocation : NAVIGATION_SPAN_NAME_FALLBACK, - attributes: { - [SENTRY_OP]: NAVIGATION, - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.nextjs.pages_router_instrumentation', - [SENTRY_SEGMENT_NAME_SOURCE]: spanSource, - ...(spanSource === 'route' && { [URL_TEMPLATE]: newLocation }), - }, - }, - { url: getAbsoluteUrl(navigationTarget) }, - ); - }); -} - -function getNextRouteFromPathname(pathname: string): string | undefined { - const pageRoutes = globalObject.__BUILD_MANIFEST?.sortedPages; - - // Page route should in 99.999% of the cases be defined by now but just to be sure we make a check here - if (!pageRoutes) { - return; - } - - return pageRoutes.find(route => { - const routeRegExp = convertNextRouteToRegExp(route); - return pathname.match(routeRegExp); - }); -} - -/** - * Converts a Next.js style route to a regular expression that matches on pathnames (no query params or URL fragments). - * - * In general this involves replacing any instances of square brackets in a route with a wildcard: - * e.g. "/users/[id]/info" becomes /\/users\/([^/]+?)\/info/ - * - * Some additional edgecases need to be considered: - * - All routes have an optional slash at the end, meaning users can navigate to "/users/[id]/info" or - * "/users/[id]/info/" - both will be resolved to "/users/[id]/info". - * - Non-optional "catchall"s at the end of a route must be considered when matching (e.g. "/users/[...params]"). - * - Optional "catchall"s at the end of a route must be considered when matching (e.g. "/users/[[...params]]"). - * - * @param route A Next.js style route as it is found in `global.__BUILD_MANIFEST.sortedPages` - */ -function convertNextRouteToRegExp(route: string): RegExp { - // We can assume a route is at least "/". - const routeParts = route.split('/'); - - let optionalCatchallWildcardRegex = ''; - if (routeParts[routeParts.length - 1]?.match(/^\[\[\.\.\..+\]\]$/)) { - // If last route part has pattern "[[...xyz]]" we pop the latest route part to get rid of the required trailing - // slash that would come before it if we didn't pop it. - routeParts.pop(); - optionalCatchallWildcardRegex = '(?:/(.+?))?'; - } - - const rejoinedRouteParts = routeParts - .map( - routePart => - routePart - .replace(/^\[\.\.\..+\]$/, '(.+?)') // Replace catch all wildcard with regex wildcard - .replace(/^\[.*\]$/, '([^/]+?)'), // Replace route wildcards with lazy regex wildcards - ) - .join('/'); - - // oxlint-disable-next-line sdk/no-regexp-constructor -- routeParts are from the build manifest, so no raw user input - return new RegExp( - `^${rejoinedRouteParts}${optionalCatchallWildcardRegex}(?:/)?$`, // optional slash at the end - ); -} diff --git a/packages/nextjs/src/config/withSentryConfig/buildTime.ts b/packages/nextjs/src/config/withSentryConfig/buildTime.ts index c468b4a1f18e..b93d9ea2fc9c 100644 --- a/packages/nextjs/src/config/withSentryConfig/buildTime.ts +++ b/packages/nextjs/src/config/withSentryConfig/buildTime.ts @@ -2,6 +2,7 @@ import * as childProcess from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; import type { NextConfigObject, SentryBuildOptions } from '../types'; +import { hasOnlyAppRouterPages } from './pagesRouterDetection'; /** * Adds Sentry-related build-time variables to `nextConfig.env`. @@ -11,11 +12,13 @@ import type { NextConfigObject, SentryBuildOptions } from '../types'; * @param userNextConfig - The user's Next.js config object * @param userSentryOptions - The Sentry build options passed to `withSentryConfig` * @param releaseName - The resolved release name, if any + * @param projectDir - The Next.js project root */ export function setUpBuildTimeVariables( userNextConfig: NextConfigObject, userSentryOptions: SentryBuildOptions, releaseName: string | undefined, + projectDir: string = process.cwd(), ): void { const assetPrefix = userNextConfig.assetPrefix || userNextConfig.basePath || ''; const basePath = userNextConfig.basePath ?? ''; @@ -66,6 +69,11 @@ export function setUpBuildTimeVariables( buildTimeVariables._sentryRelease = releaseName; } + // See `client/routing/nextRoutingInstrumentation.ts`. + if (hasOnlyAppRouterPages(projectDir)) { + buildTimeVariables._sentryHasPagesRouter = 'false'; + } + if (typeof userNextConfig.env === 'object') { userNextConfig.env = { ...buildTimeVariables, ...userNextConfig.env }; } else if (userNextConfig.env === undefined) { diff --git a/packages/nextjs/src/config/withSentryConfig/pagesRouterDetection.ts b/packages/nextjs/src/config/withSentryConfig/pagesRouterDetection.ts new file mode 100644 index 000000000000..de1d94f191e5 --- /dev/null +++ b/packages/nextjs/src/config/withSentryConfig/pagesRouterDetection.ts @@ -0,0 +1,33 @@ +import * as fs from 'fs'; +import * as path from 'path'; + +/** + * Whether the project has an `app` directory and no page files outside `pages/api`. + * + * A false positive would silently drop Pages Router navigation spans, so any file outside `pages/api` counts as a + * page (covers custom `pageExtensions` and `_app`/`_document`), and no `app` directory means `false`. + */ +export function hasOnlyAppRouterPages(projectDir: string): boolean { + const hasAppDir = ['app', path.join('src', 'app')].some(dir => isDirectory(path.join(projectDir, dir))); + if (!hasAppDir) { + return false; + } + + return ['pages', path.join('src', 'pages')].every(dir => !containsNonApiPages(path.join(projectDir, dir))); +} + +function containsNonApiPages(pagesDir: string): boolean { + if (!isDirectory(pagesDir)) { + return false; + } + + return fs.readdirSync(pagesDir).some(entry => entry !== 'api'); +} + +function isDirectory(dir: string): boolean { + try { + return fs.statSync(dir).isDirectory(); + } catch { + return false; + } +} diff --git a/packages/nextjs/test/config/withSentryConfig/buildTime.test.ts b/packages/nextjs/test/config/withSentryConfig/buildTime.test.ts new file mode 100644 index 000000000000..b81e3c48ee86 --- /dev/null +++ b/packages/nextjs/test/config/withSentryConfig/buildTime.test.ts @@ -0,0 +1,60 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { setUpBuildTimeVariables } from '../../../src/config/withSentryConfig/buildTime'; + +const tmpDirs: string[] = []; + +function makeProject(files: string[]): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'sentry-nextjs-build-time-')); + tmpDirs.push(dir); + for (const file of files) { + const absolute = path.join(dir, file); + fs.mkdirSync(path.dirname(absolute), { recursive: true }); + fs.writeFileSync(absolute, ''); + } + return dir; +} + +afterEach(() => { + for (const dir of tmpDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +describe('setUpBuildTimeVariables', () => { + describe('_sentryHasPagesRouter', () => { + it('is set to "false" for a project that only has App Router pages', () => { + const nextConfig = {}; + + setUpBuildTimeVariables(nextConfig, {}, undefined, makeProject(['app/page.tsx', 'pages/api/hello.ts'])); + + expect(nextConfig).toEqual({ env: expect.objectContaining({ _sentryHasPagesRouter: 'false' }) }); + }); + + it('is left unset for a project with Pages Router pages', () => { + const nextConfig = {}; + + setUpBuildTimeVariables(nextConfig, {}, undefined, makeProject(['app/page.tsx', 'pages/index.tsx'])); + + expect(nextConfig).toEqual({ env: expect.not.objectContaining({ _sentryHasPagesRouter: expect.anything() }) }); + }); + + it('is left unset when the project layout cannot be determined', () => { + const nextConfig = {}; + + setUpBuildTimeVariables(nextConfig, {}, undefined, makeProject(['package.json'])); + + expect(nextConfig).toEqual({ env: expect.not.objectContaining({ _sentryHasPagesRouter: expect.anything() }) }); + }); + + it('does not override a value the user set themselves', () => { + const nextConfig = { env: { _sentryHasPagesRouter: 'true' } }; + + setUpBuildTimeVariables(nextConfig, {}, undefined, makeProject(['app/page.tsx'])); + + expect(nextConfig.env._sentryHasPagesRouter).toBe('true'); + }); + }); +}); diff --git a/packages/nextjs/test/config/withSentryConfig/pagesRouterDetection.test.ts b/packages/nextjs/test/config/withSentryConfig/pagesRouterDetection.test.ts new file mode 100644 index 000000000000..dbf8d39ce872 --- /dev/null +++ b/packages/nextjs/test/config/withSentryConfig/pagesRouterDetection.test.ts @@ -0,0 +1,60 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { hasOnlyAppRouterPages } from '../../../src/config/withSentryConfig/pagesRouterDetection'; + +const tmpDirs: string[] = []; + +function makeProject(files: string[]): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'sentry-nextjs-router-detection-')); + tmpDirs.push(dir); + for (const file of files) { + const absolute = path.join(dir, file); + fs.mkdirSync(path.dirname(absolute), { recursive: true }); + fs.writeFileSync(absolute, ''); + } + return dir; +} + +afterEach(() => { + for (const dir of tmpDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +describe('hasOnlyAppRouterPages', () => { + it.each([ + ['an `app` dir and no `pages` dir', ['app/page.tsx']], + ['a `src/app` dir and no `pages` dir', ['src/app/page.tsx']], + [ + 'an `app` dir and only API routes under `pages/api`', + ['app/page.tsx', 'pages/api/hello.ts', 'pages/api/v1/nested.ts'], + ], + ['an `app` dir and only API routes under `src/pages/api`', ['src/app/page.tsx', 'src/pages/api/hello.ts']], + ])('returns true for %s', (_description, files) => { + expect(hasOnlyAppRouterPages(makeProject(files))).toBe(true); + }); + + it.each([ + ['a page in `pages`', ['app/page.tsx', 'pages/index.tsx']], + ['a nested page in `pages`', ['app/page.tsx', 'pages/blog/[slug].tsx']], + ['a page in `src/pages`', ['src/app/page.tsx', 'src/pages/about.tsx']], + ['`pages` special files only, which still render through the Pages Router', ['app/page.tsx', 'pages/_app.tsx']], + ['a page with a custom page extension', ['app/page.tsx', 'pages/index.page.mdx']], + ])('returns false when an `app` dir project also has %s', (_description, files) => { + expect(hasOnlyAppRouterPages(makeProject(files))).toBe(false); + }); + + it.each([ + ['no `app` dir and no `pages` dir', ['package.json']], + ['no `app` dir but a `pages` dir', ['pages/index.tsx']], + ['no `app` dir and only API routes', ['pages/api/hello.ts']], + ])('returns false when the router type cannot be determined: %s', (_description, files) => { + expect(hasOnlyAppRouterPages(makeProject(files))).toBe(false); + }); + + it('returns false for a directory that does not exist', () => { + expect(hasOnlyAppRouterPages(path.join(os.tmpdir(), `sentry-does-not-exist-${Date.now()}`))).toBe(false); + }); +}); diff --git a/packages/nextjs/test/performance/nextRoutingInstrumentation.test.ts b/packages/nextjs/test/performance/nextRoutingInstrumentation.test.ts new file mode 100644 index 000000000000..4b6b117e2117 --- /dev/null +++ b/packages/nextjs/test/performance/nextRoutingInstrumentation.test.ts @@ -0,0 +1,102 @@ +import type { Client } from '@sentry/core'; +import { WINDOW } from '@sentry/react'; +import { JSDOM } from 'jsdom'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + appRouterInstrumentNavigation, + appRouterInstrumentPageLoad, +} from '../../src/client/routing/appRouterRoutingInstrumentation'; +import { + nextRouterInstrumentNavigation, + nextRouterInstrumentPageLoad, +} from '../../src/client/routing/nextRoutingInstrumentation'; +import { pagesRouterInstrumentNavigation } from '../../src/client/routing/pagesRouterNavigationInstrumentation'; +import { pagesRouterInstrumentPageLoad } from '../../src/client/routing/pagesRouterRoutingInstrumentation'; + +vi.mock('../../src/client/routing/appRouterRoutingInstrumentation', () => ({ + appRouterInstrumentNavigation: vi.fn(), + appRouterInstrumentPageLoad: vi.fn(), +})); +vi.mock('../../src/client/routing/pagesRouterNavigationInstrumentation', () => ({ + pagesRouterInstrumentNavigation: vi.fn(), +})); +vi.mock('../../src/client/routing/pagesRouterRoutingInstrumentation', () => ({ + pagesRouterInstrumentPageLoad: vi.fn(), +})); + +const client = {} as Client; +const originalDocument = WINDOW.document; +const originalHasPagesRouter = process.env._sentryHasPagesRouter; + +function setUpPage(router: 'app' | 'pages'): void { + const dom = new JSDOM( + router === 'pages' + ? '' + : '', + ); + Object.defineProperty(WINDOW, 'document', { value: dom.window.document, writable: true }); +} + +describe('nextRoutingInstrumentation', () => { + beforeEach(() => { + delete process.env._sentryHasPagesRouter; + }); + + afterEach(() => { + Object.defineProperty(WINDOW, 'document', { value: originalDocument, writable: true }); + if (originalHasPagesRouter === undefined) { + delete process.env._sentryHasPagesRouter; + } else { + process.env._sentryHasPagesRouter = originalHasPagesRouter; + } + vi.clearAllMocks(); + }); + + it('instruments the App Router when there is no __NEXT_DATA__ tag', () => { + setUpPage('app'); + + nextRouterInstrumentPageLoad(client); + nextRouterInstrumentNavigation(client); + + expect(appRouterInstrumentPageLoad).toHaveBeenCalledWith(client); + expect(appRouterInstrumentNavigation).toHaveBeenCalledWith(client); + expect(pagesRouterInstrumentPageLoad).not.toHaveBeenCalled(); + expect(pagesRouterInstrumentNavigation).not.toHaveBeenCalled(); + }); + + it('instruments the Pages Router when there is a __NEXT_DATA__ tag', () => { + setUpPage('pages'); + + nextRouterInstrumentPageLoad(client); + nextRouterInstrumentNavigation(client); + + expect(pagesRouterInstrumentPageLoad).toHaveBeenCalledWith(client); + expect(pagesRouterInstrumentNavigation).toHaveBeenCalledWith(client); + expect(appRouterInstrumentPageLoad).not.toHaveBeenCalled(); + expect(appRouterInstrumentNavigation).not.toHaveBeenCalled(); + }); + + describe('when the build declared the project has no Pages Router pages', () => { + beforeEach(() => { + process.env._sentryHasPagesRouter = 'false'; + }); + + it('skips Pages Router navigation instrumentation, so bundlers can drop it and `next/router`', () => { + setUpPage('pages'); + + nextRouterInstrumentNavigation(client); + + expect(pagesRouterInstrumentNavigation).not.toHaveBeenCalled(); + expect(appRouterInstrumentNavigation).not.toHaveBeenCalled(); + }); + + // App Router builds still serve `404.html`/`500.html` through the Pages Router. + it('still instruments Pages Router pageloads', () => { + setUpPage('pages'); + + nextRouterInstrumentPageLoad(client); + + expect(pagesRouterInstrumentPageLoad).toHaveBeenCalledWith(client); + }); + }); +}); diff --git a/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts index 356d7d3db092..55e794dda04e 100644 --- a/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts +++ b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts @@ -4,10 +4,8 @@ import { JSDOM } from 'jsdom'; import type { NEXT_DATA } from 'next/dist/shared/lib/utils'; import Router from 'next/router'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { - pagesRouterInstrumentNavigation, - pagesRouterInstrumentPageLoad, -} from '../../src/client/routing/pagesRouterRoutingInstrumentation'; +import { pagesRouterInstrumentPageLoad } from '../../src/client/routing/pagesRouterRoutingInstrumentation'; +import { pagesRouterInstrumentNavigation } from '../../src/client/routing/pagesRouterNavigationInstrumentation'; import { URL_TEMPLATE } from '@sentry/conventions/attributes'; const globalObject = WINDOW as typeof WINDOW & {