fix(nextjs): Lazy-load the Pages Router module so App Router apps do not bundle the Pages Router runtime

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

@@ -15,18 +15,41 @@ import {
   WINDOW,
 } from '@sentry/react';
 import type { NEXT_DATA } from 'next/dist/shared/lib/utils';
-import RouterImport from 'next/router';
+import type 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;
+type NextRouter = typeof RouterImport;
+
+/**
+ * Loads the Pages Router singleton (what `next/router` exports) on demand.
+ *
+ * It must not be imported statically: it is the whole Pages Router client runtime (the `Router` class,
+ * `path-to-regexp`, the route loader, ...), `next` does not declare `sideEffects`, and whether an app uses
+ * the Pages Router is only known at runtime (see `nextRoutingInstrumentation.ts`). A static import therefore
+ * lands the entire Pages Router in the client bundle of every app - App Router apps included, which never
+ * reach this code - and no bundler can tree-shake it away, with or without `__SENTRY_TRACING__`. Behind
+ * `import()` the module stays out of the initial graph.
+ *
+ * `next/dist/client/router` rather than the public `next/router` entry on purpose: that entry is a one-line
+ * CJS shim re-exporting this module, and a shim that is not in the app's initial chunks becomes a tiny extra
+ * chunk request on every Pages Router pageload (151 bytes on Turbopack, 99 on webpack when measured). The
+ * module itself is already part of the Pages Router runtime, so importing it directly adds no request and
+ * resolves on the next microtask. The type still comes from `next/router`; it is the same object.
+ */
+function loadNextRouter(): Promise<NextRouter> {
+  return import('next/dist/client/router').then(routerModule => {
+    // next/router v10 is CJS
+    //
+    // For ESM/CJS interoperability 'reasons', depending on how this file is loaded, Router might be the
+    // namespace itself, sit on its default export, or on the default export's default export.
+    const namespace = routerModule as unknown as { default?: NextRouter };
+    const candidate = (namespace.default ?? namespace) as NextRouter;
+    return candidate.events ? candidate : (candidate as unknown as { default: NextRouter }).default;
+  });
+}
 
 const globalObject = WINDOW;
 
@@ -144,39 +167,48 @@ export function pagesRouterInstrumentPageLoad(client: Client): void {
  *
  * Leverages the SingletonRouter from the `next/router` to
  * generate pageload/navigation transactions and parameterize
- * transaction names.
+ * transaction names. The router is loaded on demand (see `loadNextRouter`), so the
+ * `routeChangeStart` listener is registered once that import has settled.
  */
 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';
-    }
+  void loadNextRouter()
+    .then(Router => {
+      Router.events.on('routeChangeStart', (navigationTarget: string) => {
+        const strippedNavigationTarget = stripUrlQueryAndFragment(navigationTarget);
+        const matchedRoute = getNextRouteFromPathname(strippedNavigationTarget);
 
-    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) },
-    );
-  });
+        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) },
+        );
+      });
+    })
+    .catch((error: unknown) => {
+      DEBUG_BUILD &&
+        debug.warn('Could not load `next/router`, Pages Router navigations will not be instrumented:', error);
+    });
 }
 
 function getNextRouteFromPathname(pathname: string): string | undefined {

packages/nextjs/test/clientEntryBundlerGraph.test.ts

@@ -0,0 +1,40 @@
+import { spawnSync } from 'node:child_process';
+import { resolve } from 'node:path';
+import { describe, expect, it } from 'vitest';
+
+/**
+ * Importing the SDK client entry must not load `next/router`. That module is the whole Pages Router client
+ * runtime, and a static import of it lands in the client bundle of every app - App Router apps included,
+ * which never reach the Pages Router branch of the routing instrumentation. Bundlers cannot remove it
+ * (`next` declares no `sideEffects`, and the app/pages decision is made at runtime), so the durable guard
+ * is that the entry's module graph does not contain it: `pagesRouterRoutingInstrumentation` imports the
+ * router on demand instead. Runs in a child process for a clean module cache and real Node resolution,
+ * like `serverEntryBundlerGraph.test.ts`.
+ */
+describe('built CJS client entry', () => {
+  const clientEntry = resolve(__dirname, '../build/cjs/client/index.js');
+
+  it('does not load `next/router` at import time', () => {
+    const script = `
+      require(${JSON.stringify(clientEntry)});
+      const toPosix = modulePath => modulePath.split(require('path').sep).join('/');
+      const loaded = Object.keys(require.cache).map(toPosix);
+      // Control: the Pages Router instrumentation itself must be in the graph, or an empty list proves nothing.
+      if (!loaded.some(modulePath => modulePath.endsWith('/client/routing/pagesRouterRoutingInstrumentation.js'))) {
+        console.error('Control failed: the Pages Router routing instrumentation was not loaded at all');
+        process.exit(2);
+      }
+      const routerModules = loaded.filter(
+        modulePath => modulePath.endsWith('/next/router.js') || modulePath.includes('/next/dist/client/router'),
+      );
+      if (routerModules.length > 0) {
+        console.error('next/router loaded at import time:\\n' + routerModules.join('\\n'));
+        process.exit(1);
+      }
+    `;
+
+    // On failure, stderr carries the leaked module list, the failed control, or the import crash itself.
+    const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf8' });
+    expect(result.status, result.stderr).toBe(0);
+  });
+});

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

@@ -2,7 +2,8 @@ import type { Client } from '@sentry/core';
 import { WINDOW } from '@sentry/react';
 import { JSDOM } from 'jsdom';
 import type { NEXT_DATA } from 'next/dist/shared/lib/utils';
-import Router from 'next/router';
+// The instrumentation imports the module behind the `next/router` shim on demand, so that is what is mocked.
+import Router from 'next/dist/client/router';
 import { afterEach, describe, expect, it, vi } from 'vitest';
 import {
   pagesRouterInstrumentNavigation,
@@ -21,17 +22,17 @@ const originalBuildManifestRoutes = globalObject.__BUILD_MANIFEST?.sortedPages;
 
 let eventHandlers: { [eventName: string]: Set<(...args: any[]) => void> } = {};
 
-vi.mock('next/router', () => {
+vi.mock('next/dist/client/router', () => {
   return {
     default: {
       events: {
-        on(type: string, handler: (...args: any[]) => void) {
+        on: vi.fn((type: string, handler: (...args: any[]) => void) => {
           if (!eventHandlers[type]) {
             eventHandlers[type] = new Set();
           }
 
           eventHandlers[type]!.add(handler);
-        },
+        }),
         off: vi.fn((type: string, handler: (...args: any[]) => void) => {
           if (eventHandlers[type]) {
             eventHandlers[type]!.delete(handler);
@@ -300,7 +301,7 @@ describe('pagesRouterInstrumentNavigation', () => {
     ['/e/f/g', '/e/[f]/[g]/[[...h]]', 'route'],
   ])(
     'should create a parameterized transaction on route change (%s)',
-    (targetLocation, expectedTransactionName, expectedTransactionSource) => {
+    async (targetLocation, expectedTransactionName, expectedTransactionSource) => {
       setUpNextPage({
         url: 'https://example.com/home',
         route: '/home',
@@ -325,6 +326,8 @@ describe('pagesRouterInstrumentNavigation', () => {
       } as unknown as Client;
 
       pagesRouterInstrumentNavigation(client);
+      // The router is imported on demand; the listener exists once that import has settled.
+      await vi.dynamicImportSettled();
 
       Router.events.emit('routeChangeStart', targetLocation);
 
@@ -352,4 +355,31 @@ describe('pagesRouterInstrumentNavigation', () => {
       });
     },
   );
+
+  it('registers the route change listener only once the on-demand router import has settled', async () => {
+    setUpNextPage({
+      url: 'https://example.com/home',
+      route: '/home',
+      hasNextData: true,
+      navigatableRoutes: ['/home'],
+    });
+
+    const client = {
+      emit: vi.fn(),
+      getOptions: () => ({}),
+    } as unknown as Client;
+
+    // The pageload instrumentation reads `__NEXT_DATA__` and the build manifest only - it never needs the router.
+    pagesRouterInstrumentPageLoad(client);
+    expect(Router.events.on).not.toHaveBeenCalled();
+
+    // The navigation instrumentation imports the router on demand: nothing is registered synchronously ...
+    pagesRouterInstrumentNavigation(client);
+    expect(Router.events.on).not.toHaveBeenCalled();
+
+    // ... and exactly one listener once the import has settled.
+    await vi.dynamicImportSettled();
+    expect(Router.events.on).toHaveBeenCalledTimes(1);
+    expect(Router.events.on).toHaveBeenCalledWith('routeChangeStart', expect.any(Function));
+  });
 });