[pull] canary from vercel:canary - #1347
Merged
Merged
Conversation
### What? Adds export-name mangling to Turbopack, behind a new experimental option `experimental.turbopackMangleExportNames` (default `false`). It is independent of minification: `--no-mangling` is a minifier flag and does not affect it. When enabled, each ECMAScript module's *used* export names — including `default` and `__esModule` — are replaced by short keys in the emitted output, both where the module registers its exports and where every consumer reads them. Modules whose export names could be observed by user code keep their original names, decided per module. This is a reland of #89060 (on top of the already-merged #89406), originally written by Matt Mastracci, who is credited as a co-author on the commit. Stacked on top: #97676 flips the default to `true` on canary releases, so Next.js's own CI exercises the feature broadly before it is considered for stable. ### Why? Bundle size. A module's export keys exist only to link modules together: the producing module emits `{ someVeryLongExportName: … }` and every consumer reads `ns["someVeryLongExportName"]`. Both sides are generated by us, so as long as producer and consumer agree — and the name isn't observable from user code — the key can be a single character. Long export names are extremely common in real dependency graphs (icon sets, utility packages, barrel files), and each one is paid for once in the module that defines it and once per importing module. ### How? **Ported, not rebased.** `canary` is ~2500 commits past the original stack's base, and the files it touched were independently rewritten in the meantime (export-analysis refactor #92781, CJS analysis for scope hoisting #95826, the `module_fragments` subsystem #95978). A probe rebase produced 16 conflicting files on the first commit alone, so the original branches were used as a reference implementation — for intent, the identifier alphabet, and test coverage — and the feature was rebuilt on today's infrastructure. #89561 from the original stack (erasing the Next.js wrapper module types) is deliberately **not** part of this change; it turned out to be unnecessary, because those modules already declare whole-module export usage and therefore back off on their own. **The name table** (`references/esm/mangle/table.rs`) hashes each name into a table of all valid JS identifiers of the smallest length that fits the name set — 15 exports get single-character keys — and resolves collisions by open addressing. Hashing rather than assigning `a`, `b`, `c`, … is what keeps names stable: an unrelated edit elsewhere in the module doesn't renumber every other export, and a collision only perturbs its own cluster. Assignment happens in two passes: every name that is *already* a valid identifier at the chosen length keeps itself and reserves its bucket first, and only then is anything hashed — so an export called `a` keeps `a`, and nothing else can be assigned it. Both passes iterate in sorted order, so the mapping depends only on the set of names. A module with exactly **one** mangleable export is special-cased to a fixed key, `f`, rather than a hashed one. `f` is the most common character in JS keywords (`if`, `for`, `function`), and every single-export module in the graph then emits the same `.f` / `.f()` byte sequences, which gzip's back-references pick up across the whole bundle — a bigger win than hashing, at the cost of that one key changing when a second export is added. A fixed list, `RESERVED_KEYS`, is withheld from every table for two different reasons: JS reserved words (`if`, `in`, `do`, `for`, `let`, `new`, `try`, `var`) are legal as quoted property keys but a minifier will not fold `ns["if"]` into the shorter `ns.if`, so handing one out costs bytes instead of saving them; and `__esModule` is withheld because the runtime's `esm()` helper defines that property on every module's exports object regardless of what the module itself exports, so an assigned key landing on it would collide. (`default` needs no such protection — once it is mangled like any other export, nothing else emits a property under that literal name.) **One source of truth for the mapping.** `mangled_export_names(module, chunking_context)` is a turbo-task that both the producing side (`EsmExports::code_generation`) and the consuming side (`ReferencedAssetIdent::Module`, the single place a cross-module export access is materialized) ask for the *target* module's map. Neither side computes a table of its own, so they cannot disagree, and the task derives export usage from the chunking context itself rather than accepting it as an argument, so a caller can't supply usage from the wrong graph. Re-export chains need no special handling, because the consumer side already resolves through re-exports to the module that produces the binding. The mangling decision itself lives on `EsmExports` as a `mangle_export_names: bool` field, rather than a separate trait method every module type has to override. A module that derives its exports from another one (a facade, a locals module, a part, a rename) inherits the flag with the data, which removed seven hand-written delegations and the possibility of a new wrapper type forgetting one. **A mangling decision must not cross module identities.** A few module types hand out *another* module's exports value as their own (the WASM loader module, the module-fragments side-effects wrapper, the client-reference proxy). If that borrowed value carried a real mangling decision, the producing and consuming sides would key their lookups on two different modules and could compute two different keys for the same export — this actually broke every WASM- and `@vercel/og`-based test once the default-on layer exercised it in CI. `EcmascriptExports::borrowed()` is the one place this is handled: it always returns an unmangled view, and every such pass-through site uses it. **Back-off is per module**, built on the export-usage information that landed after the original PR (`BindingUsageInfo` / `ModuleExportUsageInfo`) rather than the original's locals/facade-split heuristic. A module keeps its names when its usage is `All` (a namespace import that couldn't be lowered, a computed property access, an unresolvable `export *`, or a chunk-group entry — which covers client references and the Next.js wrapper modules), when it is read through a namespace value at all, when its exports are dynamic or not statically known ESM, or when names aren't being mangled in this build. `__webpack_exports_info__` gains `canMangle` and `mangledName` per export, which is how a running test can observe the mapping; with the option off it emits exactly what it emitted before. ### Testing - `turbo-tasks-hash`/table unit tests: encode/decode round-trip, degenerate-name rejection, table sizing, the single-export fixed key (including its own reservation), reserved-word withholding (including a reserved bucket-count test that stays in sync with the reserved list), the preserved-name pass running before any hashing, uniqueness under heavy collision, order independence, wrap-around probing, and same-tier stability. - 13 `turbopack-tests` execution fixtures under `tests/execution/turbopack/exports/mangle-*`, several ported from the original PR and from webpack's `test/configCases/mangle`: named imports, re-export chains and default exports (including one literally named `__esModule`), escaping namespaces (`Object.keys`, `delete ns.missing`, `export * as`, CJS interop), destructuring, prototype-shadowing names (`toString`, `$1`, `__1`), a 60-export two-character table, dynamic `import()` with `webpackExports` / `turbopackExports`, a CommonJS consumer of an ESM module, dynamic re-exports, scope hoisting on and off, and a control with the option off. - 3 committed snapshot fixtures under `tests/snapshot/mangle-exports`, so the emitted keys, the back-off, and the fixed single-export key are visible in review. - One fixture under `__skipped__`, which the harness asserts *fails*, recording the namespace-materialization gap below. - Full suite: 546 unit + 272 execution + 125 snapshot tests pass, with no snapshot churn across the several refactors this PR went through in review. - Verified against real builds: targeted app-dir, worker, WASM, and `@vercel/og`-based e2e suites pass with the option forced on (the failures that remain are external-network tests that fail identically with it off), and a small two-page app shrank by 0.76% of total emitted JS / 0.53% gzipped. ### Known limitations, each intentional - **A module read through `import * as ns` is never mangled**, even when every read is statically tracked, because the analysis doesn't yet distinguish a lowered named read from a materialized namespace object. Namespace imports are common, so this leaves real wins on the table; unlocking it is the highest-value follow-up. - **Escaping namespaces back off entirely.** Webpack instead keeps mangling and materializes a namespace object keyed by the original names. That is the `__skipped__` fixture: implementing it turns the suite red until the fixture is moved out. - **CommonJS export mangling is out of scope.** The producing side is reachable, but the consuming accesses live in user source and nothing rewrites them today; it needs its own design pass. Closes PACK-435 <!-- NEXT_JS_LLM --> Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com> <!-- fleet ecdfa248-cd54-41ac-b4a2-c9d49e2a67ee --> --------- 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: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>
…7676) Stacked on #97672 — enable export mangling by default on canary releases. ### What? Two defaults change: - **Next.js builds.** `experimental.turbopackMangleExportNames` is pinned to `false` for stable releases and left *unset* on canary, where Turbopack then defaults it from the build mode: on for production builds, off in development. Explicitly setting the option wins in either direction, so setting it to `true` in development is honoured. - **Turbopack's own execution test suite.** `TestOptions::mangle_export_names` defaults to `true`, so all 279 fixtures exercise mangling instead of only the handful that opt in. ### Why? The feature is verified by turbopack's own fixtures, targeted e2e suites, and a couple of bundle-size measurements — a narrow slice of what Next.js's test suites actually cover. Defaulting it on for canary puts every production-mode e2e and integration test through the mangled code path for real users of the canary channel, without committing stable users to it yet. Turning it on for stable is a separate, later decision that can stack on top of this once canary has soaked it. Broad exposure has already earned its keep several times over. Turning mangling on in CI and in the fixture suite surfaced bugs that no hand-picked suite had found, including a module-fragments helper handing out another module's exports value (#97672's `EcmascriptExports::borrowed()`), a client-reference proxy memoized per wrapper instance rather than per content, and a code-elimination bug for export-less modules (fixed in the follow-up PR). ### How? `defaultConfig` cannot see the build mode, so it only expresses the stable/canary split (`isStableBuild() ? false : undefined`) and `NextConfig::turbopack_mangle_export_names(mode)` supplies the mode-dependent default on the Turbopack side. This is deliberately *not* the shape of the neighbouring `turbopackSharedRuntime: !isStableBuild()`: mangling should not apply in development, and hard-forcing `false` there in Rust — as an earlier revision of this PR did — would silently ignore a user who asked for it explicitly. Mangling does **not** depend on minification. An earlier version of this PR gated it on `minify(mode)`, which was wrong and has been removed; `--no-mangling` is a minifier flag and does not affect export mangling. Verified with a real `next build` A/B on a small app: the option left unset mangles (959,174 B total emitted JS, 293,407 B gzipped) while explicitly setting it to `false` does not (966,519 B / 294,963 B gzipped), confirming both the default and the override. All 279 execution fixtures pass with the suite default flipped, with no fixture needing an opt-out. Note this layer does not change the `turbopack-emit-collect` snapshots on its own: making a module without re-exports split additionally requires the split trigger added in the follow-up PR, so those snapshots move there. This is the layer to revert if canary turns up problems specific to running with the option on by default; #97672 remains useful as an opt-in feature either way. <!-- NEXT_JS_LLM --> Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com> <!-- fleet ecdfa248-cd54-41ac-b4a2-c9d49e2a67ee --> --------- 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: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>
### What? Adds debug-only OS realpath validation to successful `DiskFileSystem` file and directory reads. When a successfully canonicalized path differs from the supplied path, the read returns a normal task error naming both paths. Fixes pattern/glob traversal and NFT tracing so physical filesystem access uses resolved paths while logical paths remain available for user-visible specifiers and complete symlink-chain recovery. ### Why? Reading through an unresolved symlink parent gives the same filesystem object multiple path identities. That can make Turbo Tasks dependency tracking and invalidation inconsistent and can produce invalid deployment ZIPs when NFT output contains files below unresolved links. The checks return errors rather than asserting because paths can disagree temporarily under eventual consistency. Propagating a task error avoids panicking a worker thread while still exposing invalid callers during development. ### How? The validation lives directly in `DiskFileSystem::read` and `DiskFileSystem::raw_read_dir`. It calls the OS canonicalization API inline instead of the Turbo Tasks realpath task, keeping the diagnostic out of the task dependency graph. The guard runs only after the OS read succeeds, so missing/non-directory probes preserve their existing behavior. `read_matches` resolves each physical directory immediately before enumeration while retaining logical `PatternMatch` paths. `read_glob` and `track_glob` now resolve their initial directory before enumeration. Symlinks discovered later through wildcard segments are also traversed through resolved targets. `ReadGlobResult` deliberately retains logical paths rooted at the supplied base, allowing consumers to call `realpath_with_links` and recover the complete symlink chain. Consumers follow that contract explicitly: - NFT includes expand each logical match with `realpath_with_links`, emit resolved files and every traversed symlink, skip resolved directory targets, and deterministically deduplicate/sort output. - `import.meta.glob` uses recursive logical keys as the source of user-visible requests, while module resolution follows and tracks symlinks. - the hash-glob example resolves returned logical paths before reading. Webpack-loader context dependencies are covered for both `path/to/symlink/inner/path/*` and `path/to/*/inner/path/*`. The loader fixture performs its directory read with Node `fs`, reports the directory using `addContextDependency`, and Turbopack tracks the resolved target. ### Verification - `cargo fmt -p turbo-tasks-fs -p turbopack-ecmascript -p next-api -- --check` - `cargo clippy -p turbo-tasks-fs -p turbopack-ecmascript -p next-api --all-targets` - `cargo test -p turbo-tasks-fs` (128 passed) - `cargo test -p next-api` (7 passed) - `cargo check -p turbo-tasks-fs --examples` - `import.meta.glob` symlink execution fixture (1 passed) - Nine targeted node-file-trace CI cases with `release-with-assertions` (9 passed) - `pnpm build-all` - `webpack-loader-fs` Turbopack dev e2e (1 passed) - `build-trace-extra-entries-turbo` Turbopack production e2e (1 passed) - twoslash Turbopack production, normal mode (4 passed) - twoslash Turbopack production, cache-components mode (4 passed) - `bench/heavy-npm-deps` Turbopack development smoke test (HTTP 200) ### Notes The disk guard is cross-platform, while its symlink-parent regression test is Unix-only, matching neighbouring symlink tests. On Windows, OS canonicalization can also normalize casing and 8.3 short names; a debug read using a non-canonical spelling will therefore return the same diagnostic error. <!-- NEXT_JS_LLM --> <!-- fleet 1d32e12c-f4ec-4f22-862a-c85f0005805c --> --------- 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>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )