fix(nextjs): Lazy-load the Pages Router module so App Router apps do not bundle the Pages Router runtime - #24033
Open
AlbertoMihai98 wants to merge 1 commit into
Conversation
…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>
AlbertoMihai98
requested review from
mydea and
s1gr1d
and removed request for
a team
September 3, 2026 21:58
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).Closes #24032
What
client/routing/pagesRouterRoutingInstrumentation.tsimportednext/routerstatically and resolved the CJS/ESM interop at module scope.next/routeris the whole Pages Router client runtime (Routerclass,path-to-regexp, route loader,script.js, ...), it is reached statically from the client entry (client/index.ts->browserTracingIntegration->nextRoutingInstrumentation-> this file), and the app/pages decision is made at runtime - so every App Router app shipped ~87 KB raw / ~36 KB gzip of Pages Router code it can never execute, and no bundler could remove it (nextdeclares nosideEffects; with or without__SENTRY_TRACING__).This PR:
import type RouterImport from 'next/router'for the types and loads the module on demand (loadNextRouter()), only frompagesRouterInstrumentNavigation- the single place that needs it (Router.events.on('routeChangeStart', ...)). The pageload instrumentation is unchanged: it reads__NEXT_DATA__and__BUILD_MANIFESTonly, so the pageload span is still started synchronously inafterAllSetup..default->.default.default), and logs aDEBUG_BUILDwarning if the import ever rejects.pagesRouterInstrumentation.test.ts(await vi.dynamicImportSettled()afterpagesRouterInstrumentNavigation, same idiom assveltekit/test/client/browserTracingIntegration.test.ts) and adds a test that the listener is registered exactly once, only after the import has settled, and that the pageload path never touches the router.test/clientEntryBundlerGraph.test.ts, mirroringserverEntryBundlerGraph.test.ts: a child process requires the built CJS client entry and fails ifnext/routerornext/dist/client/routeris inrequire.cache(with a positive control that the Pages Router instrumentation itself was loaded). Reintroducing the static import makes it fail with the two module paths listed; I ran that mutation before opening this.The size-limit entry for
@sentry/nextjs (client)cannot see this class of regression because it hasignore: ['next/router', 'next/constants']; the graph test above is the gate instead.Measurements
Same app, same instrument at both ends (
next build --experimental-analyze+ a chunk-composition script), patching the installed build of the published SDK with exactly this change.App Router app (Next 16.3.3, Turbopack,
__SENTRY_TRACING__compiled tofalse)next(Pages Router runtime)@sentry/nextjs10.72.0 as publishedPages Router app (minimal:
/,/aboutvia<Link>,/redirectdoingrouter.replace('/about')from a mount effect;tracesSampleRate: 1,beforeSendTransactionrecords every transaction onwindow.__txns)Built with
@sentry/nextjs10.73.0 as published, then with this change applied to the published ESM file (the fork's own build targets v11 exports and cannot be installed against the 10.x registry graph); served withnext startand driven with Chrome DevTools./pageload/pageload<Link>navigation spanrouter.replace()spanimport('next/router')(public shim)t.exports=o.r(26990)requested on every pageloadimport('next/dist/client/router')(this PR)The shim (
next/router.js=module.exports = require('./dist/client/router')) is not part of a Pages Router app's initial chunks unless the app itself importsnext/routeron that page, so both bundlers turnimport('next/router')into a real, tiny async chunk; the module it re-exports is already loaded by the Pages Router runtime, soimport('next/dist/client/router')costs nothing at runtime and settles on the next microtask. The three span rows carry the exact Pages Router origins (auto.pageload.nextjs.pages_router_instrumentation/auto.navigation.nextjs.pages_router_instrumentation), read from abeforeSendTransactionhook.Notes for reviewers
window.next.router.events(droppingnext/routeraltogether) was considered and rejected: Next marksinstance.eventsas "never documented ... remove the following major version", and the instance only exists aftercreateRouter().nextRoutingInstrumentation.ts) was rejected too: it would make the pageload span asynchronous, andafterAllSetuprelies on it existing beforebrowserTracingIntegrationInstance.afterAllSetup(client)continues.v10? The chain and the cost are identical there (10.73.0).Gates run locally on the branch:
yarn build,yarn test:unit(58 files / 997 tests),yarn lint,yarn lint:types,yarn circularDepCheck,yarn lint:es-compatibility,oxfmt --checkon the touched files - all green.🤖 Generated with Claude Code