Skip to content

Commit 66fa442

Browse files
fix(nextjs): Lazy-load the Pages Router module so App Router apps do not bundle the Pages Router runtime
`client/routing/pagesRouterRoutingInstrumentation.ts` imported `next/router` statically and resolved its CJS/ESM interop at module scope. That module is the whole Pages Router client runtime (the `Router` class, path-to-regexp, the route loader, script.js, ...), it is reached statically from the client entry, and whether an app uses the Pages Router is only known at runtime - so every App Router app shipped ~87 KB raw / ~36 KB gzip of code it can never execute, and no bundler could remove it (`next` declares no `sideEffects`). The router is now imported on demand, only from `pagesRouterInstrumentNavigation`, the single place that needs it; the pageload instrumentation is unchanged and still synchronous. The import targets `next/dist/client/router` rather than the `next/router` shim: the shim is not part of a Pages Router app's initial chunks and became a tiny extra chunk request on every pageload (151 bytes on Turbopack, 99 on webpack), while the module itself is already loaded by the framework runtime, so importing it directly adds no request. Tests: the navigation tests await `vi.dynamicImportSettled()`; a new test pins that the listener is registered exactly once and only after the import settles, and that the pageload path never touches the router; `test/clientEntryBundlerGraph.test.ts` requires the built CJS client entry in a child process and fails if `next/router` or `next/dist/client/router` is in the module cache (with a positive control on the instrumentation module itself). Measured on an App Router app (Next 16.3.3, Turbopack): Sentry client chunk 168.4 KB -> 82 KB raw; the Pages Router runtime lands in an async chunk the app never requests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 81f8ade commit 66fa442

3 files changed

Lines changed: 144 additions & 42 deletions

File tree

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

Lines changed: 69 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,41 @@ import {
1515
WINDOW,
1616
} from '@sentry/react';
1717
import type { NEXT_DATA } from 'next/dist/shared/lib/utils';
18-
import RouterImport from 'next/router';
18+
import type RouterImport from 'next/router';
1919
import type { ParsedUrlQuery } from 'querystring';
2020
import { DEBUG_BUILD } from '../../common/debug-build';
2121
import { SENTRY_OP, SENTRY_SEGMENT_NAME_SOURCE, URL_TEMPLATE } from '@sentry/conventions/attributes';
2222
import { NAVIGATION, PAGELOAD } from '@sentry/conventions/op';
2323

24-
// next/router v10 is CJS
25-
//
26-
// For ESM/CJS interoperability 'reasons', depending on how this file is loaded, Router might be on the default export
27-
const Router: typeof RouterImport = RouterImport.events
28-
? RouterImport
29-
: (RouterImport as unknown as { default: typeof RouterImport }).default;
24+
type NextRouter = typeof RouterImport;
25+
26+
/**
27+
* Loads the Pages Router singleton (what `next/router` exports) on demand.
28+
*
29+
* It must not be imported statically: it is the whole Pages Router client runtime (the `Router` class,
30+
* `path-to-regexp`, the route loader, ...), `next` does not declare `sideEffects`, and whether an app uses
31+
* the Pages Router is only known at runtime (see `nextRoutingInstrumentation.ts`). A static import therefore
32+
* lands the entire Pages Router in the client bundle of every app - App Router apps included, which never
33+
* reach this code - and no bundler can tree-shake it away, with or without `__SENTRY_TRACING__`. Behind
34+
* `import()` the module stays out of the initial graph.
35+
*
36+
* `next/dist/client/router` rather than the public `next/router` entry on purpose: that entry is a one-line
37+
* CJS shim re-exporting this module, and a shim that is not in the app's initial chunks becomes a tiny extra
38+
* chunk request on every Pages Router pageload (151 bytes on Turbopack, 99 on webpack when measured). The
39+
* module itself is already part of the Pages Router runtime, so importing it directly adds no request and
40+
* resolves on the next microtask. The type still comes from `next/router`; it is the same object.
41+
*/
42+
function loadNextRouter(): Promise<NextRouter> {
43+
return import('next/dist/client/router').then(routerModule => {
44+
// next/router v10 is CJS
45+
//
46+
// For ESM/CJS interoperability 'reasons', depending on how this file is loaded, Router might be the
47+
// namespace itself, sit on its default export, or on the default export's default export.
48+
const namespace = routerModule as unknown as { default?: NextRouter };
49+
const candidate = (namespace.default ?? namespace) as NextRouter;
50+
return candidate.events ? candidate : (candidate as unknown as { default: NextRouter }).default;
51+
});
52+
}
3053

3154
const globalObject = WINDOW;
3255

@@ -144,39 +167,48 @@ export function pagesRouterInstrumentPageLoad(client: Client): void {
144167
*
145168
* Leverages the SingletonRouter from the `next/router` to
146169
* generate pageload/navigation transactions and parameterize
147-
* transaction names.
170+
* transaction names. The router is loaded on demand (see `loadNextRouter`), so the
171+
* `routeChangeStart` listener is registered once that import has settled.
148172
*/
149173
export function pagesRouterInstrumentNavigation(client: Client): void {
150-
Router.events.on('routeChangeStart', (navigationTarget: string) => {
151-
const strippedNavigationTarget = stripUrlQueryAndFragment(navigationTarget);
152-
const matchedRoute = getNextRouteFromPathname(strippedNavigationTarget);
153-
154-
let newLocation: string;
155-
let spanSource: TransactionSource;
156-
157-
if (matchedRoute) {
158-
newLocation = matchedRoute;
159-
spanSource = 'route';
160-
} else {
161-
newLocation = strippedNavigationTarget;
162-
spanSource = 'url';
163-
}
174+
void loadNextRouter()
175+
.then(Router => {
176+
Router.events.on('routeChangeStart', (navigationTarget: string) => {
177+
const strippedNavigationTarget = stripUrlQueryAndFragment(navigationTarget);
178+
const matchedRoute = getNextRouteFromPathname(strippedNavigationTarget);
164179

165-
startBrowserTracingNavigationSpan(
166-
client,
167-
{
168-
// With span streaming, span names have to be low cardinality, so we can't fall back to the URL.
169-
name: spanSource === 'route' || !hasSpanStreamingEnabled(client) ? newLocation : NAVIGATION_SPAN_NAME_FALLBACK,
170-
attributes: {
171-
[SENTRY_OP]: NAVIGATION,
172-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.nextjs.pages_router_instrumentation',
173-
[SENTRY_SEGMENT_NAME_SOURCE]: spanSource,
174-
...(spanSource === 'route' && { [URL_TEMPLATE]: newLocation }),
175-
},
176-
},
177-
{ url: getAbsoluteUrl(navigationTarget) },
178-
);
179-
});
180+
let newLocation: string;
181+
let spanSource: TransactionSource;
182+
183+
if (matchedRoute) {
184+
newLocation = matchedRoute;
185+
spanSource = 'route';
186+
} else {
187+
newLocation = strippedNavigationTarget;
188+
spanSource = 'url';
189+
}
190+
191+
startBrowserTracingNavigationSpan(
192+
client,
193+
{
194+
// With span streaming, span names have to be low cardinality, so we can't fall back to the URL.
195+
name:
196+
spanSource === 'route' || !hasSpanStreamingEnabled(client) ? newLocation : NAVIGATION_SPAN_NAME_FALLBACK,
197+
attributes: {
198+
[SENTRY_OP]: NAVIGATION,
199+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.nextjs.pages_router_instrumentation',
200+
[SENTRY_SEGMENT_NAME_SOURCE]: spanSource,
201+
...(spanSource === 'route' && { [URL_TEMPLATE]: newLocation }),
202+
},
203+
},
204+
{ url: getAbsoluteUrl(navigationTarget) },
205+
);
206+
});
207+
})
208+
.catch((error: unknown) => {
209+
DEBUG_BUILD &&
210+
debug.warn('Could not load `next/router`, Pages Router navigations will not be instrumented:', error);
211+
});
180212
}
181213

182214
function getNextRouteFromPathname(pathname: string): string | undefined {
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { spawnSync } from 'node:child_process';
2+
import { resolve } from 'node:path';
3+
import { describe, expect, it } from 'vitest';
4+
5+
/**
6+
* Importing the SDK client entry must not load `next/router`. That module is the whole Pages Router client
7+
* runtime, and a static import of it lands in the client bundle of every app - App Router apps included,
8+
* which never reach the Pages Router branch of the routing instrumentation. Bundlers cannot remove it
9+
* (`next` declares no `sideEffects`, and the app/pages decision is made at runtime), so the durable guard
10+
* is that the entry's module graph does not contain it: `pagesRouterRoutingInstrumentation` imports the
11+
* router on demand instead. Runs in a child process for a clean module cache and real Node resolution,
12+
* like `serverEntryBundlerGraph.test.ts`.
13+
*/
14+
describe('built CJS client entry', () => {
15+
const clientEntry = resolve(__dirname, '../build/cjs/client/index.js');
16+
17+
it('does not load `next/router` at import time', () => {
18+
const script = `
19+
require(${JSON.stringify(clientEntry)});
20+
const toPosix = modulePath => modulePath.split(require('path').sep).join('/');
21+
const loaded = Object.keys(require.cache).map(toPosix);
22+
// Control: the Pages Router instrumentation itself must be in the graph, or an empty list proves nothing.
23+
if (!loaded.some(modulePath => modulePath.endsWith('/client/routing/pagesRouterRoutingInstrumentation.js'))) {
24+
console.error('Control failed: the Pages Router routing instrumentation was not loaded at all');
25+
process.exit(2);
26+
}
27+
const routerModules = loaded.filter(
28+
modulePath => modulePath.endsWith('/next/router.js') || modulePath.includes('/next/dist/client/router'),
29+
);
30+
if (routerModules.length > 0) {
31+
console.error('next/router loaded at import time:\\n' + routerModules.join('\\n'));
32+
process.exit(1);
33+
}
34+
`;
35+
36+
// On failure, stderr carries the leaked module list, the failed control, or the import crash itself.
37+
const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf8' });
38+
expect(result.status, result.stderr).toBe(0);
39+
});
40+
});

‎packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ import type { Client } from '@sentry/core';
22
import { WINDOW } from '@sentry/react';
33
import { JSDOM } from 'jsdom';
44
import type { NEXT_DATA } from 'next/dist/shared/lib/utils';
5-
import Router from 'next/router';
5+
// The instrumentation imports the module behind the `next/router` shim on demand, so that is what is mocked.
6+
import Router from 'next/dist/client/router';
67
import { afterEach, describe, expect, it, vi } from 'vitest';
78
import {
89
pagesRouterInstrumentNavigation,
@@ -21,17 +22,17 @@ const originalBuildManifestRoutes = globalObject.__BUILD_MANIFEST?.sortedPages;
2122

2223
let eventHandlers: { [eventName: string]: Set<(...args: any[]) => void> } = {};
2324

24-
vi.mock('next/router', () => {
25+
vi.mock('next/dist/client/router', () => {
2526
return {
2627
default: {
2728
events: {
28-
on(type: string, handler: (...args: any[]) => void) {
29+
on: vi.fn((type: string, handler: (...args: any[]) => void) => {
2930
if (!eventHandlers[type]) {
3031
eventHandlers[type] = new Set();
3132
}
3233

3334
eventHandlers[type]!.add(handler);
34-
},
35+
}),
3536
off: vi.fn((type: string, handler: (...args: any[]) => void) => {
3637
if (eventHandlers[type]) {
3738
eventHandlers[type]!.delete(handler);
@@ -300,7 +301,7 @@ describe('pagesRouterInstrumentNavigation', () => {
300301
['/e/f/g', '/e/[f]/[g]/[[...h]]', 'route'],
301302
])(
302303
'should create a parameterized transaction on route change (%s)',
303-
(targetLocation, expectedTransactionName, expectedTransactionSource) => {
304+
async (targetLocation, expectedTransactionName, expectedTransactionSource) => {
304305
setUpNextPage({
305306
url: 'https://example.com/home',
306307
route: '/home',
@@ -325,6 +326,8 @@ describe('pagesRouterInstrumentNavigation', () => {
325326
} as unknown as Client;
326327

327328
pagesRouterInstrumentNavigation(client);
329+
// The router is imported on demand; the listener exists once that import has settled.
330+
await vi.dynamicImportSettled();
328331

329332
Router.events.emit('routeChangeStart', targetLocation);
330333

@@ -352,4 +355,31 @@ describe('pagesRouterInstrumentNavigation', () => {
352355
});
353356
},
354357
);
358+
359+
it('registers the route change listener only once the on-demand router import has settled', async () => {
360+
setUpNextPage({
361+
url: 'https://example.com/home',
362+
route: '/home',
363+
hasNextData: true,
364+
navigatableRoutes: ['/home'],
365+
});
366+
367+
const client = {
368+
emit: vi.fn(),
369+
getOptions: () => ({}),
370+
} as unknown as Client;
371+
372+
// The pageload instrumentation reads `__NEXT_DATA__` and the build manifest only - it never needs the router.
373+
pagesRouterInstrumentPageLoad(client);
374+
expect(Router.events.on).not.toHaveBeenCalled();
375+
376+
// The navigation instrumentation imports the router on demand: nothing is registered synchronously ...
377+
pagesRouterInstrumentNavigation(client);
378+
expect(Router.events.on).not.toHaveBeenCalled();
379+
380+
// ... and exactly one listener once the import has settled.
381+
await vi.dynamicImportSettled();
382+
expect(Router.events.on).toHaveBeenCalledTimes(1);
383+
expect(Router.events.on).toHaveBeenCalledWith('routeChangeStart', expect.any(Function));
384+
});
355385
});

0 commit comments

Comments
 (0)