Repository navigation
[lts-active] Turbopack: backport Pages Router dev module graph chaining (#98823, #99910, #100091) - #100008
Conversation
…8823) ### What? Chains the Pages Router **development** client module graph so that `/_app` and the page being chunked live in the same `ModuleGraph`. This is PR 1 of a six-PR stack, based on `canary` (#98733, #99686 and #99749 are already merged): 1. **This PR** — Pages Router dev client graph chaining 2. #99910 — regression test for Pages Router `_app` availability (#99789) 3. #98734 — use graph-derived merged groups 4. #99750 — chunking takes a `ChunkGroupKey` 5. #98685 — chunk-group availability bitmaps 6. #99050 — keyed chunk-group index lookups ### Why? In development `Project::per_page_module_graph()` is true, so every page endpoint builds its own client `ModuleGraph`, each with its own `ChunkGroupInfo` and its own index space starting at `0`. `/_app` is always loaded before the page in the browser, so the page reuses `/_app`'s availability when chunking. Today that availability is a set of concrete modules, which is graph-independent, so the split graphs go unnoticed. That stops being true as soon as availability is expressed as chunk-group indices (#98685): `/_app` would record index `0` in *its* graph, and the page would resolve that same `0` against *its own* graph — where it means the page's own entry group. Every module of the group being chunked then looks already-available and is dropped, producing runtime errors like: ``` Module [hmr-entry]/hmr-entry.js { ENTRY => "[project]/pages/_error" } was instantiated as a runtime entry of chunk …, but the module factory is not available. ``` ### How? - `PageEndpoint::client_module_graph` builds a graph *chain* in per-page mode: `/_app`'s entry group first, then the page's, with `VisitedModules` threaded between them — the same layout-segment approach `ssr_module_graph` already uses for document → app → page. - Whole-app mode (production) is unchanged and still goes through `module_graph_for_modules`. - `/_app` itself is not chained onto anything. - Extracts a small `client_entry_modules` helper, since the entry module list is now needed in two places. Availability is still carried as the set of modules `/_app` emitted. Making the two endpoints share one graph also fixes #99789: with separate graphs, batching in the page graph could produce a node for a module shared with `/_app` whose edges point at page-only modules, and those modules were skipped as already available ("module factory is not available" on hydration). #99910 adds the regression test. Lining up the graph identities is also what #98685 builds on. ### Verification - `turbopack-core` — 160 passed - `next-core` — 84 passed - Turbopack snapshots — 125 passed - `404-page-router` and `server-navigation-error` development suites - Affected Cargo checks and Rust formatting <!-- NEXT_JS_LLM --> <!-- fleet 4f67c6f7-bbe2-454b-9a17-822a60ef8f7e --> Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com> (cherry picked from commit 6dd4b27) Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
…99910) ### What? Regression coverage for #99789, stacked on #98823. This is PR 2 of a six-PR stack, based on `canary`: 1. #98823 — Pages Router dev client graph chaining 2. **This PR** — regression test for Pages Router `_app` availability (#99789) 3. #98734 — use graph-derived merged groups 4. #99750 — chunking takes a `ChunkGroupKey` 5. #98685 — chunk-group availability bitmaps 6. #99050 — keyed chunk-group index lookups - **New e2e suite** `test/e2e/pages-app-availability-boundary`: a Pages Router app with the import shape from #99789: - `_app` imports `c-shared` and dynamically imports a module that also imports it. - `pages/index` imports `a-page-only`, `b-boundary`, `c-shared`, `d-boundary`, `e-page`, and dynamically imports a module that imports `b`/`d`, which makes `b` and `d` batching boundaries. The test asserts that the page hydrates (a marker set in `useEffect`), that every imported value renders, and that no "module factory is not available" error is logged. - **`app-document-import-order`**: the "loads modules shared by _app and the page only once" assertion added in #97664 was skipped in all of dev. It now runs everywhere and is gated `// @gate !(turbopack && dev)` (expected failure). ### Why? In Turbopack dev, #97664 seeds each page's client chunk group with `/_app`'s availability. But each page had its own module graph, so availability items (`ModuleOrBatch` identities) were compared across graphs. In the page graph, batching can produce a `ModuleOrBatch::Module(c-shared)` node whose edges point at earlier siblings (`a`, `b`). Because `/_app` already has `Module(c-shared)`, the traversal excluded that subtree, and `a`/`b` were never chunked. Hydration then fails with `Module [project]/lib/a-page-only.js … was instantiated … but the module factory is not available`. No test covered this, because the #97664 test was skipped in dev. #98823 chains `/_app` and the page into one dev client module graph, which fixes the missing modules. It does **not** yet dedupe the `app-document-import-order` fixture in Turbopack dev. The shared module is still emitted in both the `_app` and the page chunk, because the page's batches in the combined graph carry different chunk-group bitmaps than `_app`'s own graph, so they don't match `_app`'s availability items. Hence the gate. It stays valid through #98734 and #99750; #98685 (higher in this stack) dedupes the fixture in Turbopack dev, turning it into a "stale gate", and removes it. ### How was it verified? Local JS build of this branch plus the preview native binaries of #98823's head (`8dd759a`) and its base (`26fd598`) in `packages/next-swc/native`. The two commits differ only in Rust. | Suite | Mode | #98823 base | #98823 head | |---|---|---|---| | `pages-app-availability-boundary` | Turbopack dev | ❌ module factory not available | ✅ | | `pages-app-availability-boundary` | Turbopack start, webpack dev/start | | ✅ | | `app-document-import-order` | Turbopack dev | | ✅ (gated: failed as expected) | | `app-document-import-order` | webpack dev, Turbopack/webpack start | | ✅ | After restacking onto canary, with native builds of each layer, in Turbopack dev: - This PR's tree: `pages-app-availability-boundary` ✅, `app-document-import-order` ✅ (gated: failed as expected). - #98734 and #99750: gated assertion still fails as expected. - #98685 and #99050: gate stale, so #98685 removes it; ungated, both suites ✅. Fixes #99789 (together with #98823) <!-- NEXT_JS_LLM --> <!-- fleet a7614af3-e186-4297-beaf-7e55eaddc0da --> <!-- fleet 4f67c6f7-bbe2-454b-9a17-822a60ef8f7e --> Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com> Co-authored-by: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> (cherry picked from commit 1e1f9e6) Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
Tests PassedCommit: e656ebe |
… of 404ing (#100011) Backports [Fix google tag manager e2e test, assume GTM 200s instead of 404ing (#99896)](#99896) to `releases/lts/active`. This is a test-only change, cherry-picked cleanly from `canary` with `-x`. The GTM script now returns 200 for the fake tag id (it used to 404) and pushes its own `dataLayer` events. That makes `@next/third-parties basic usage › renders GTM` fail on every LTS CI run, for example on #100008. The tests now wait for GTM/GA to initialize and assert only on the events they care about. ## Verification - Baseline with the LTS test files: `renders GTM` fails in both `test/e2e/third-parties/index.test.ts` and `test/e2e/app-dir/third-parties/basic.test.ts` (`dataLayer.length` 3 vs 1), matching CI. - With this change, both suites pass 8/8 in Turbopack dev and start and in webpack dev and start, against the real GTM script. <!-- NEXT_JS_LLM --> <!-- fleet 652922cb-dc34-493d-9be5-c241a5164cad --> Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Benjamin Woodruff <benjamin.woodruff@vercel.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
|
@sokra does this fix #99789 (comment) as well? |
I can't really say without more info... |
@sokra our app is running into a similar issue and I opened an issue with it, thanks! https://github.com/dcheng1290/next-loadable-404 |
For what it's worth I can't repro their claim locally |
|
@dcheng1290 Thanks for the repro. I ran it on macOS arm64 (Node 24.21.0, React 19.2.0), with a fresh install and a fresh
So it starts in canary.7, not canary.8. canary.7 is the first release with #98823 (the fix this PR backports), so that change is the likely cause. I did not bisect inside canary.7 to prove it. @sokra this may matter for the LTS backport too. Written by an agent (Claude Code, claude-opus-5-5). |
…ility (#100091) ## Summary Fixes a Turbopack dev regression from #98823 in the Pages Router. When `pages/_app` renders a `next/dynamic` component, a page's SSR HTML can preload a chunk that 404s. This was reported in #99789, with a repro at [dcheng1290/next-loadable-404](https://github.com/dcheng1290/next-loadable-404). The component still renders, but the browser logs a failed request. This PR also makes Turbopack production preload `_app`'s `next/dynamic` chunks on every page, matching dev and webpack. **Why:** #98823 chains the `/_app` and page client module graphs in development. `next/dynamic` imports are collected from every layer of that graph, so each page's `react-loadable-manifest` now also lists `/_app`'s dynamic imports, but chunked with the **page's** availability. At runtime those imports are loaded by `/_app`'s own async loader, which is chunked with `/_app`'s graph and availability. The preloaded chunk names therefore don't match the chunks that are emitted and requested. In production, `/_app`'s dynamic imports were only listed in `server/pages/_app/react-loadable-manifest.json`, which page renders don't read, so they were never preloaded. **How:** a Pages-only helper in `crates/next-api/src/pages.rs` (`page_next_dynamic_chunks`), used for every page except `/_app`: - it chunks `/_app`'s dynamic imports with the `/_app` endpoint's client module graph and client availability, which is exactly what `/_app` itself lists; - it chunks the page's own imports as before, with `/_app`'s filtered out (in per-page/dev mode they are reachable from the page's chained graph); - it merges both into the page's manifest. The generic collector in `module_graph.rs` is untouched, and so is App Router. This should be backported to `releases/lts/active` together with #98823 (backport PR #100008). ## Verification - New e2e suite `test/e2e/pages-app-next-dynamic-preload`, based on the repro. It checks that: - every `/_next/static` asset in the SSR HTML of `/` and `/other` returns 200; - the `_app` dynamic component renders; - both pages preload the same non-empty set of `_app` dynamic chunks. - The suite fails without the fix in Turbopack dev (a 404 asset). With the fix it passes in Turbopack dev and start and in webpack dev and start, using a locally built native binary. - Existing suites pass in Turbopack dev and start: `next-dynamic`, `next-dynamic-css`, `next-dynamic-lazy-compilation`, `pages-app-availability-boundary`, `app-document-import-order` (#99910), `app-dir/dynamic` and `app-dir/dynamic-import`. - Manual check, using the repro with a headless browser in dev and in production: - there are no failed requests or console errors on either page; - production manifests list `_app`'s entry with the chunk `_app` itself uses. - `cargo clippy -p next-api` and `cargo fmt --check` pass. <!-- NEXT_JS_LLM --> <!-- fleet 652922cb-dc34-493d-9be5-c241a5164cad --> Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
…ility (#100091) ## Summary Fixes a Turbopack dev regression from #98823 in the Pages Router. When `pages/_app` renders a `next/dynamic` component, a page's SSR HTML can preload a chunk that 404s. This was reported in #99789, with a repro at [dcheng1290/next-loadable-404](https://github.com/dcheng1290/next-loadable-404). The component still renders, but the browser logs a failed request. This PR also makes Turbopack production preload `_app`'s `next/dynamic` chunks on every page, matching dev and webpack. **Why:** #98823 chains the `/_app` and page client module graphs in development. `next/dynamic` imports are collected from every layer of that graph, so each page's `react-loadable-manifest` now also lists `/_app`'s dynamic imports, but chunked with the **page's** availability. At runtime those imports are loaded by `/_app`'s own async loader, which is chunked with `/_app`'s graph and availability. The preloaded chunk names therefore don't match the chunks that are emitted and requested. In production, `/_app`'s dynamic imports were only listed in `server/pages/_app/react-loadable-manifest.json`, which page renders don't read, so they were never preloaded. **How:** a Pages-only helper in `crates/next-api/src/pages.rs` (`page_next_dynamic_chunks`), used for every page except `/_app`: - it chunks `/_app`'s dynamic imports with the `/_app` endpoint's client module graph and client availability, which is exactly what `/_app` itself lists; - it chunks the page's own imports as before, with `/_app`'s filtered out (in per-page/dev mode they are reachable from the page's chained graph); - it merges both into the page's manifest. The generic collector in `module_graph.rs` is untouched, and so is App Router. This should be backported to `releases/lts/active` together with #98823 (backport PR #100008). ## Verification - New e2e suite `test/e2e/pages-app-next-dynamic-preload`, based on the repro. It checks that: - every `/_next/static` asset in the SSR HTML of `/` and `/other` returns 200; - the `_app` dynamic component renders; - both pages preload the same non-empty set of `_app` dynamic chunks. - The suite fails without the fix in Turbopack dev (a 404 asset). With the fix it passes in Turbopack dev and start and in webpack dev and start, using a locally built native binary. - Existing suites pass in Turbopack dev and start: `next-dynamic`, `next-dynamic-css`, `next-dynamic-lazy-compilation`, `pages-app-availability-boundary`, `app-document-import-order` (#99910), `app-dir/dynamic` and `app-dir/dynamic-import`. - Manual check, using the repro with a headless browser in dev and in production: - there are no failed requests or console errors on either page; - production manifests list `_app`'s entry with the chunk `_app` itself uses. - `cargo clippy -p next-api` and `cargo fmt --check` pass. <!-- NEXT_JS_LLM --> <!-- fleet 652922cb-dc34-493d-9be5-c241a5164cad --> Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com> (cherry picked from commit 5eec1d9) Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
Summary
Backports these canary commits to
releases/lts/active:6dd4b27bd8a53d3b9799a52e7d589763e74fb9da_appavailability (#99789).1e1f9e691109dd422dd04aa42d93e52aafdfc6b85eec1d9d981d4208bda51394be3c79701cc8c0d6_appnext/dynamicpreload 404s introduced by #98823, and preload those imports in production too.The third commit must ship with #98823: chaining the page and
_appgraphs made pages preload_app's dynamic imports with the page's availability, unlike_app's own async loader. #100091 chunks those imports with_app's own graph and availability, then merges them with the page's imports. The generic collector and App Router are untouched. The regression was reproduced with dcheng1290/next-loadable-404.All three backports retain cherry-pick provenance. #100091 had an import-only conflict in
pages.rsbecause LTS lacks an unrelatedsmallvecimport; the resolution adds onlyFxHashSet. The added/removed lines and test files match upstream. Existing branch history is preserved; the merged GTM test fix (#100011) remains included.Fixes #99789 on the LTS line.
Verification
With a native binary rebuilt from this LTS backport branch:
pages-app-next-dynamic-preload: 5/5 pass in Turbopack dev and build/start. The suite checks every referenced static asset returns 200, the dynamic component renders, and both pages preload the same non-empty chunk set.pages-app-availability-boundaryandapp-document-import-order: 3 pass, 1 existing skip in Turbopack dev. The upstream gated deduplication assertion remains unchanged.pnpm build-all --env-mode=loose,cargo check -p next-api,cargo fmt -p next-api --check, scoped Prettier and ESLint pass. The initial full bootstrap ran out of sandbox disk during native compilation; after clearing disposable artifacts and passing the Cargo environment through Turbo, the full build passed.Earlier verification, before adding #100091, also covered the #99910 suites in Turbopack start and webpack dev. The new LTS backport has not been tested locally in deploy mode, on other platforms, or with full workspace type-checking; CI will provide that coverage.