Skip to content

Commit c89f6e6

Browse files
chargomechargome
authored
fix(v10/nextjs): Keep the Pages Router runtime out of App Router client bundles (#24223)
Backport of: #24188 ## Differences to the original PR - `packages/nextjs/src/client/routing/pagesRouterNavigationInstrumentation.ts`: uses v10's `SEMANTIC_ATTRIBUTE_SENTRY_OP`/`SEMANTIC_ATTRIBUTE_SENTRY_SOURCE` and the plain route/URL span name; v10 has no `@sentry/conventions/op`, `SENTRY_SEGMENT_NAME_SOURCE`, or span-streaming name fallback. The `__BUILD_MANIFEST`-typed `globalObject` moved here with the route matching code. - `packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts`: keeps v10's pageload code (`browserPerformanceTimeOrigin` start time, v10 attribute constants); only the navigation half and the `next/router` import were removed. - `dev-packages/e2e-tests/test-applications/nextjs-16/tests/build-output.test.ts`: new file on v10 containing only the bundle assertion; the absolute-path-imports test from `develop` depends on a `@sentry-internal/test-utils` helper that does not exist on v10.
1 parent de25dcb commit c89f6e6

10 files changed

Lines changed: 417 additions & 122 deletions

File tree

‎dev-packages/e2e-tests/test-applications/nextjs-16/tests/build-output.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { expect, test } from '@playwright/test';
2+
import * as fs from 'fs';
3+
import * as path from 'path';
4+
import { isDevMode } from './isDevMode';
5+
6+
// This app has no `pages` directory, so `withSentryConfig` lets the bundler drop the Pages Router navigation
7+
// instrumentation and its `next/router` import (~80 KB raw).
8+
test('does not ship the Pages Router runtime in the App Router client bundle', () => {
9+
test.skip(isDevMode, 'Only production builds are tree-shaken');
10+
11+
const buildManifest = JSON.parse(fs.readFileSync(path.join(process.cwd(), '.next', 'build-manifest.json'), 'utf8'));
12+
const rootMainFiles: string[] = buildManifest.rootMainFiles;
13+
expect(rootMainFiles.length).toBeGreaterThan(0);
14+
15+
const appClientBundle = rootMainFiles
16+
.map(file => fs.readFileSync(path.join(process.cwd(), '.next', file), 'utf8'))
17+
.join('\n');
18+
19+
// Control: the Sentry client is in these files.
20+
expect(appClientBundle).toContain('auto.pageload.nextjs.pages_router_instrumentation');
21+
22+
expect(appClientBundle).not.toContain('auto.navigation.nextjs.pages_router_instrumentation');
23+
// Only Next.js' Pages Router runtime contains this event name.
24+
expect(appClientBundle).not.toContain('beforeHistoryChange');
25+
});

‎packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import type { Client } from '@sentry/core';
22
import { WINDOW } from '@sentry/react';
33
import { appRouterInstrumentNavigation, appRouterInstrumentPageLoad } from './appRouterRoutingInstrumentation';
4-
import { pagesRouterInstrumentNavigation, pagesRouterInstrumentPageLoad } from './pagesRouterRoutingInstrumentation';
4+
import { pagesRouterInstrumentNavigation } from './pagesRouterNavigationInstrumentation';
5+
import { pagesRouterInstrumentPageLoad } from './pagesRouterRoutingInstrumentation';
56

67
/**
78
* Instruments the Next.js Client Router for page loads.
@@ -22,7 +23,10 @@ export function nextRouterInstrumentNavigation(client: Client): void {
2223
const isAppRouter = !WINDOW.document.getElementById('__NEXT_DATA__');
2324
if (isAppRouter) {
2425
appRouterInstrumentNavigation(client);
25-
} else {
26+
} else if (process.env._sentryHasPagesRouter !== 'false') {
27+
// `withSentryConfig` inlines `'false'` for App Router-only projects, so bundlers drop this module and its
28+
// `next/router` import (the whole Pages Router runtime). Pageload stays: App Router builds still serve
29+
// `404.html`/`500.html` through the Pages Router, and it does not need `next/router`.
2630
pagesRouterInstrumentNavigation(client);
2731
}
2832
}

‎packages/nextjs/src/client/routing/pagesRouterNavigationInstrumentation.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import type { Client, TransactionSource } from '@sentry/core';
2+
import {
3+
SEMANTIC_ATTRIBUTE_SENTRY_OP,
4+
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
5+
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
6+
stripUrlQueryAndFragment,
7+
} from '@sentry/core';
8+
import { getAbsoluteUrl, startBrowserTracingNavigationSpan, WINDOW } from '@sentry/react';
9+
import RouterImport from 'next/router';
10+
import { URL_TEMPLATE } from '@sentry/conventions/attributes';
11+
12+
// next/router v10 is CJS
13+
//
14+
// For ESM/CJS interoperability 'reasons', depending on how this file is loaded, Router might be on the default export
15+
const Router: typeof RouterImport = RouterImport.events
16+
? RouterImport
17+
: (RouterImport as unknown as { default: typeof RouterImport }).default;
18+
19+
const globalObject = WINDOW as typeof WINDOW & {
20+
__BUILD_MANIFEST?: {
21+
sortedPages?: string[];
22+
};
23+
};
24+
25+
/**
26+
* Instruments the Next.js pages router for navigation.
27+
* Only supported for client side routing. Works for Next >= 10.
28+
*
29+
* Leverages the SingletonRouter from the `next/router` to
30+
* generate pageload/navigation transactions and parameterize
31+
* transaction names.
32+
*/
33+
export function pagesRouterInstrumentNavigation(client: Client): void {
34+
Router.events.on('routeChangeStart', (navigationTarget: string) => {
35+
const strippedNavigationTarget = stripUrlQueryAndFragment(navigationTarget);
36+
const matchedRoute = getNextRouteFromPathname(strippedNavigationTarget);
37+
38+
let newLocation: string;
39+
let spanSource: TransactionSource;
40+
41+
if (matchedRoute) {
42+
newLocation = matchedRoute;
43+
spanSource = 'route';
44+
} else {
45+
newLocation = strippedNavigationTarget;
46+
spanSource = 'url';
47+
}
48+
49+
startBrowserTracingNavigationSpan(
50+
client,
51+
{
52+
name: newLocation,
53+
attributes: {
54+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
55+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.nextjs.pages_router_instrumentation',
56+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: spanSource,
57+
...(spanSource === 'route' && { [URL_TEMPLATE]: newLocation }),
58+
},
59+
},
60+
{ url: getAbsoluteUrl(navigationTarget) },
61+
);
62+
});
63+
}
64+
65+
function getNextRouteFromPathname(pathname: string): string | undefined {
66+
const pageRoutes = globalObject.__BUILD_MANIFEST?.sortedPages;
67+
68+
// Page route should in 99.999% of the cases be defined by now but just to be sure we make a check here
69+
if (!pageRoutes) {
70+
return;
71+
}
72+
73+
return pageRoutes.find(route => {
74+
const routeRegExp = convertNextRouteToRegExp(route);
75+
return pathname.match(routeRegExp);
76+
});
77+
}
78+
79+
/**
80+
* Converts a Next.js style route to a regular expression that matches on pathnames (no query params or URL fragments).
81+
*
82+
* In general this involves replacing any instances of square brackets in a route with a wildcard:
83+
* e.g. "/users/[id]/info" becomes /\/users\/([^/]+?)\/info/
84+
*
85+
* Some additional edgecases need to be considered:
86+
* - All routes have an optional slash at the end, meaning users can navigate to "/users/[id]/info" or
87+
* "/users/[id]/info/" - both will be resolved to "/users/[id]/info".
88+
* - Non-optional "catchall"s at the end of a route must be considered when matching (e.g. "/users/[...params]").
89+
* - Optional "catchall"s at the end of a route must be considered when matching (e.g. "/users/[[...params]]").
90+
*
91+
* @param route A Next.js style route as it is found in `global.__BUILD_MANIFEST.sortedPages`
92+
*/
93+
function convertNextRouteToRegExp(route: string): RegExp {
94+
// We can assume a route is at least "/".
95+
const routeParts = route.split('/');
96+
97+
let optionalCatchallWildcardRegex = '';
98+
if (routeParts[routeParts.length - 1]?.match(/^\[\[\.\.\..+\]\]$/)) {
99+
// If last route part has pattern "[[...xyz]]" we pop the latest route part to get rid of the required trailing
100+
// slash that would come before it if we didn't pop it.
101+
routeParts.pop();
102+
optionalCatchallWildcardRegex = '(?:/(.+?))?';
103+
}
104+
105+
const rejoinedRouteParts = routeParts
106+
.map(
107+
routePart =>
108+
routePart
109+
.replace(/^\[\.\.\..+\]$/, '(.+?)') // Replace catch all wildcard with regex wildcard
110+
.replace(/^\[.*\]$/, '([^/]+?)'), // Replace route wildcards with lazy regex wildcards
111+
)
112+
.join('/');
113+
114+
// oxlint-disable-next-line sdk/no-regexp-constructor -- routeParts are from the build manifest, so no raw user input
115+
return new RegExp(
116+
`^${rejoinedRouteParts}${optionalCatchallWildcardRegex}(?:/)?$`, // optional slash at the end
117+
);
118+
}

‎packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts‎

Lines changed: 3 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,19 @@
1-
import type { Client, TransactionSource } from '@sentry/core';
1+
import type { Client } from '@sentry/core';
22
import {
33
browserPerformanceTimeOrigin,
44
debug,
55
parseBaggageHeader,
66
SEMANTIC_ATTRIBUTE_SENTRY_OP,
77
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
88
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
9-
stripUrlQueryAndFragment,
109
} from '@sentry/core';
11-
import {
12-
getAbsoluteUrl,
13-
startBrowserTracingNavigationSpan,
14-
startBrowserTracingPageLoadSpan,
15-
WINDOW,
16-
} from '@sentry/react';
10+
import { startBrowserTracingPageLoadSpan, WINDOW } from '@sentry/react';
1711
import type { NEXT_DATA } from 'next/dist/shared/lib/utils';
18-
import RouterImport from 'next/router';
1912
import type { ParsedUrlQuery } from 'querystring';
2013
import { DEBUG_BUILD } from '../../common/debug-build';
2114
import { URL_TEMPLATE } from '@sentry/conventions/attributes';
2215

23-
// next/router v10 is CJS
24-
//
25-
// For ESM/CJS interoperability 'reasons', depending on how this file is loaded, Router might be on the default export
26-
const Router: typeof RouterImport = RouterImport.events
27-
? RouterImport
28-
: (RouterImport as unknown as { default: typeof RouterImport }).default;
29-
30-
const globalObject = WINDOW as typeof WINDOW & {
31-
__BUILD_MANIFEST?: {
32-
sortedPages?: string[];
33-
};
34-
};
16+
const globalObject = WINDOW;
3517

3618
/**
3719
* Describes data located in the __NEXT_DATA__ script tag. This tag is present on every page of a Next.js app.
@@ -142,98 +124,3 @@ export function pagesRouterInstrumentPageLoad(client: Client): void {
142124
{ sentryTrace, baggage },
143125
);
144126
}
145-
146-
/**
147-
* Instruments the Next.js pages router for navigation.
148-
* Only supported for client side routing. Works for Next >= 10.
149-
*
150-
* Leverages the SingletonRouter from the `next/router` to
151-
* generate pageload/navigation transactions and parameterize
152-
* transaction names.
153-
*/
154-
export function pagesRouterInstrumentNavigation(client: Client): void {
155-
Router.events.on('routeChangeStart', (navigationTarget: string) => {
156-
const strippedNavigationTarget = stripUrlQueryAndFragment(navigationTarget);
157-
const matchedRoute = getNextRouteFromPathname(strippedNavigationTarget);
158-
159-
let newLocation: string;
160-
let spanSource: TransactionSource;
161-
162-
if (matchedRoute) {
163-
newLocation = matchedRoute;
164-
spanSource = 'route';
165-
} else {
166-
newLocation = strippedNavigationTarget;
167-
spanSource = 'url';
168-
}
169-
170-
startBrowserTracingNavigationSpan(
171-
client,
172-
{
173-
name: newLocation,
174-
attributes: {
175-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
176-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.nextjs.pages_router_instrumentation',
177-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: spanSource,
178-
...(spanSource === 'route' && { [URL_TEMPLATE]: newLocation }),
179-
},
180-
},
181-
{ url: getAbsoluteUrl(navigationTarget) },
182-
);
183-
});
184-
}
185-
186-
function getNextRouteFromPathname(pathname: string): string | undefined {
187-
const pageRoutes = globalObject.__BUILD_MANIFEST?.sortedPages;
188-
189-
// Page route should in 99.999% of the cases be defined by now but just to be sure we make a check here
190-
if (!pageRoutes) {
191-
return;
192-
}
193-
194-
return pageRoutes.find(route => {
195-
const routeRegExp = convertNextRouteToRegExp(route);
196-
return pathname.match(routeRegExp);
197-
});
198-
}
199-
200-
/**
201-
* Converts a Next.js style route to a regular expression that matches on pathnames (no query params or URL fragments).
202-
*
203-
* In general this involves replacing any instances of square brackets in a route with a wildcard:
204-
* e.g. "/users/[id]/info" becomes /\/users\/([^/]+?)\/info/
205-
*
206-
* Some additional edgecases need to be considered:
207-
* - All routes have an optional slash at the end, meaning users can navigate to "/users/[id]/info" or
208-
* "/users/[id]/info/" - both will be resolved to "/users/[id]/info".
209-
* - Non-optional "catchall"s at the end of a route must be considered when matching (e.g. "/users/[...params]").
210-
* - Optional "catchall"s at the end of a route must be considered when matching (e.g. "/users/[[...params]]").
211-
*
212-
* @param route A Next.js style route as it is found in `global.__BUILD_MANIFEST.sortedPages`
213-
*/
214-
function convertNextRouteToRegExp(route: string): RegExp {
215-
// We can assume a route is at least "/".
216-
const routeParts = route.split('/');
217-
218-
let optionalCatchallWildcardRegex = '';
219-
if (routeParts[routeParts.length - 1]?.match(/^\[\[\.\.\..+\]\]$/)) {
220-
// If last route part has pattern "[[...xyz]]" we pop the latest route part to get rid of the required trailing
221-
// slash that would come before it if we didn't pop it.
222-
routeParts.pop();
223-
optionalCatchallWildcardRegex = '(?:/(.+?))?';
224-
}
225-
226-
const rejoinedRouteParts = routeParts
227-
.map(
228-
routePart =>
229-
routePart
230-
.replace(/^\[\.\.\..+\]$/, '(.+?)') // Replace catch all wildcard with regex wildcard
231-
.replace(/^\[.*\]$/, '([^/]+?)'), // Replace route wildcards with lazy regex wildcards
232-
)
233-
.join('/');
234-
235-
// oxlint-disable-next-line sdk/no-regexp-constructor -- routeParts are from the build manifest, so no raw user input
236-
return new RegExp(
237-
`^${rejoinedRouteParts}${optionalCatchallWildcardRegex}(?:/)?$`, // optional slash at the end
238-
);
239-
}

‎packages/nextjs/src/config/withSentryConfig/buildTime.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as childProcess from 'child_process';
22
import * as fs from 'fs';
33
import * as path from 'path';
44
import type { NextConfigObject, SentryBuildOptions } from '../types';
5+
import { hasOnlyAppRouterPages } from './pagesRouterDetection';
56

67
/**
78
* Adds Sentry-related build-time variables to `nextConfig.env`.
@@ -11,11 +12,13 @@ import type { NextConfigObject, SentryBuildOptions } from '../types';
1112
* @param userNextConfig - The user's Next.js config object
1213
* @param userSentryOptions - The Sentry build options passed to `withSentryConfig`
1314
* @param releaseName - The resolved release name, if any
15+
* @param projectDir - The Next.js project root
1416
*/
1517
export function setUpBuildTimeVariables(
1618
userNextConfig: NextConfigObject,
1719
userSentryOptions: SentryBuildOptions,
1820
releaseName: string | undefined,
21+
projectDir: string = process.cwd(),
1922
): void {
2023
const assetPrefix = userNextConfig.assetPrefix || userNextConfig.basePath || '';
2124
const basePath = userNextConfig.basePath ?? '';
@@ -71,6 +74,11 @@ export function setUpBuildTimeVariables(
7174
buildTimeVariables._sentryRelease = releaseName;
7275
}
7376

77+
// See `client/routing/nextRoutingInstrumentation.ts`.
78+
if (hasOnlyAppRouterPages(projectDir)) {
79+
buildTimeVariables._sentryHasPagesRouter = 'false';
80+
}
81+
7482
if (typeof userNextConfig.env === 'object') {
7583
userNextConfig.env = { ...buildTimeVariables, ...userNextConfig.env };
7684
} else if (userNextConfig.env === undefined) {

‎packages/nextjs/src/config/withSentryConfig/pagesRouterDetection.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import * as fs from 'fs';
2+
import * as path from 'path';
3+
4+
/**
5+
* Whether the project has an `app` directory and no page files outside `pages/api`.
6+
*
7+
* A false positive would silently drop Pages Router navigation spans, so any file outside `pages/api` counts as a
8+
* page (covers custom `pageExtensions` and `_app`/`_document`), and no `app` directory means `false`.
9+
*/
10+
export function hasOnlyAppRouterPages(projectDir: string): boolean {
11+
const hasAppDir = ['app', path.join('src', 'app')].some(dir => isDirectory(path.join(projectDir, dir)));
12+
if (!hasAppDir) {
13+
return false;
14+
}
15+
16+
return ['pages', path.join('src', 'pages')].every(dir => !containsNonApiPages(path.join(projectDir, dir)));
17+
}
18+
19+
function containsNonApiPages(pagesDir: string): boolean {
20+
if (!isDirectory(pagesDir)) {
21+
return false;
22+
}
23+
24+
return fs.readdirSync(pagesDir).some(entry => entry !== 'api');
25+
}
26+
27+
function isDirectory(dir: string): boolean {
28+
try {
29+
return fs.statSync(dir).isDirectory();
30+
} catch {
31+
return false;
32+
}
33+
}

0 commit comments

Comments
 (0)