Turbopack: mangle exports behind an escaping namespace via the facade - #97770

The previous attempt at this ("remove the ident-string skip-list, it was
already unnecessary") was wrong — it only happened not to reproduce in the
narrow set of scenarios tested locally at the time; CI's broader fixture
matrix (an edge-runtime layout, a page using import.meta.url, a client
component used as a route template) still hit the same
"Duplicate module idents in graph" crash for `__nextjs-internal-proxy.mjs`.
That attempt has been reverted, and this commit replaces it with an actual
fix.

Root cause, found by reproducing the exact crash and reading the generated
proxy module's own construction: `EcmascriptClientReferenceModule::proxy_module`
built the "module path" string embedded in its generated source —
`registerClientReference(fn, MODULE_PATH, name)`, and the text of the thrown
error messages — from `self.server_ident.to_string()`. That ident carries
whichever *part* the specific resolution asked for (`AssetIdent::to_string`
renders `<module evaluation>` etc. for it — see `ident.rs`), and turbopack
resolves the plain named-export form and the always-present module-evaluation
form of the same import as two separate edges, both landing on this same
wrapper type for the same underlying client component. Two different embedded
strings meant two different `VirtualSource` values at the exact same file
path, so two distinct raw modules got analyzed, split into a facade and a
locals submodule each, and both locals submodules rendered to the same ident
string — the collision this crashed on. The wasm loader (the skip-list's other
entry) doesn't have this problem: its generated source only reflects the wasm
module's own exports/imports, never a rendered ident.

The part suffix in that embedded string was never meaningful to a reader
anyway — it's a debug/error-message label, not something that needs to
distinguish which internal resolution form produced it. Stripping
`AssetIdent::parts` before rendering it removes the divergence at its source:
both forms now embed the identical string, so `proxy_module` memoizes to the
same result regardless of which part asked for it, and the facade/locals
split introduced by this stack no longer has two distinct raw modules to
collide over.

Verified against every previously-crashing suite in this stack's history for
this bug, with the skip-list removed and this fix in place: the exact
crashing fixtures (app-basepath-custom-server, app-edge,
non-root-project-monorepo) all green, the app-dir mega-suite
(test/e2e/app-dir/app/index.test.ts, which contains the template.js scenario)
builds and runs cleanly with zero "Duplicate module idents"/
"TurbopackInternalError" occurrences (its 11 remaining failures are
external-URL navigation timeouts, unrelated — no outbound network in this
sandbox), and the full turbopack-tests suite (545 unit + 272 execution + 125
snapshot).

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

…odule

Tried swapping the stripped-server_ident approach for self.client_module.ident()
(provably canonical regardless of which edge reached this wrapper) instead of
hand-stripping .parts. It broke client component resolution entirely:

  Could not find the module "...global-error.js [app-client] (ecmascript)#default"
  in the React Client Manifest.

The React Client Manifest keys its entries by this exact string elsewhere, so a
differently-shaped ident here is a genuine regression, not a cosmetic change --
confirmed by reverting it, which restored the previously-passing suites
(action-forward-loop, app-basepath, catch-error-react-compiler, dynamic-import,
19/20, the one remaining failure being the pre-existing external-network test).

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

The `.parts`-stripping fix from the previous commit was necessary but not
sufficient — CI kept hitting the identical "Duplicate module idents" crash on
different fixtures afterward. The reason took a wrong turn to find: it does
not reproduce with a `cargo build-native-release` binary at all, only with
`build-native-release-with-assertions` — the profile CI's build-native job
actually uses (`debug-assertions = true`, `overflow-checks = true`, no LTO,
64 codegen units, per `[profile.release-with-assertions]` in the root
Cargo.toml). Rebuilding with that profile turned this from an unreproducible,
CI-only crash into a reliable same-command local one.

With that in hand: instrumenting confirmed the previous commit's fix does what
it says — the plain named-export form and the module-evaluation form of the
same import now generate byte-identical proxy source content. The crash
persisted anyway, which meant the real bug was never about content parity in
the first place.

`proxy_module` was `#[turbo_tasks::function] async fn proxy_module(&self)` —
a method whose memoization key is `self`, i.e. the whole
`EcmascriptClientReferenceModule` cell. Two different cells (one for the plain
form, one for the module-evaluation form of the same client reference) each
call it independently; nothing about calling the same *content* from two
different `self`s makes turbo-tasks treat the two calls as the same call. Each
one builds its own `VirtualSource` and processes it into its own analyzed
module, at the same file path, regardless of whether their generated content
happens to match.

Moved the whole body into a standalone `proxy_module_for(server_asset_context,
canonical_server_ident, client_module)`, keyed on exactly the three
value-equal inputs both forms share (the earlier fix already made the ident
argument value-equal by stripping `.parts` from it). Turbo-tasks' task
memoization is genuinely argument-value-based — this is the same mechanism
`EcmascriptModuleLocalsModule::new(module)` already relies on to dedupe by
`module` alone elsewhere in this stack — so calling this from two different
`EcmascriptClientReferenceModule` cells now correctly interns to one shared
result, and the facade/locals split introduced by this stack has only one raw
module to ever see for a given client reference, not two.

Verified with the release-with-assertions profile, matching CI: the exact
crashing fixture (0/3 runs crash, was crashing on every attempt before this),
the five other suites CI has caught this on across three rounds
(action-forward-loop, app-basepath, catch-error-react-compiler, dynamic-import,
app-edge, non-root-project-monorepo — 36 tests total, only the pre-existing
external-network failure and a testing-methodology mistake on my part
(app-edge doesn't support cache-components, unrelated to this fix) remain),
and the full turbopack-tests suite (545 unit + 272 execution + 125 snapshot).

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

canary gained this suite (turbopack-emit-collect) since the stack's last
rebase, asserting the literal set of module idents Turbopack's own
__turbopack_modules__ registers for various routes. Its fixture module
src/collect-result.js has one plain named export and no re-exports, so once
export mangling defaults on (this branch), it now splits into a facade (kept
at the plain ident, used by importers) plus a <locals> submodule (the
mangled internals) -- exactly the intended behavior, just newly visible to a
snapshot written before mangling existed.

Updated via the test's own -u flag (pnpm test-start-turbo
test/e2e/turbopack-emit-collect/index.test.ts -u) rather than hand-editing,
so the new snapshot text is exactly what the harness produces. Confirmed it
touched precisely the 7 sub-tests CI reported and left the other 2 (/api,
/proxy) alone, matching CI's failure list exactly. Verified 9/9 green after,
and that prettier considers the result unchanged.

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

Enabling mangling by default in the execution suite (previous commit) exposed a
code-elimination bug in this layer's split trigger.

`split_locals_and_reexports` returned true whenever `mangle_export_names` was
set, *including for a module with no exports at all*. A test `index.js` -- only
`it(...)` calls, nothing exported -- therefore split into a facade plus a locals
module, and `EcmascriptModuleFacadeModule::side_effects()` unconditionally
reports its module evaluation as side-effect free. The module's real top-level
side effects became invisible to tree shaking and the entire body was
eliminated, leaving `__turbopack_context__.s([])`. Affected test files silently
ran zero tests.

Splitting a module with zero export names is pointless by construction -- there
is nothing to mangle -- so it is now skipped. This fixes, with no fixture
changes at all: webpack/scope-hoisting/order-without-side-effects,
webpack/scope-hoisting/side-effects-11990{,-star}, and
turbopack/tree-shaking/ignore-side-effect-free-module. The doc comments on
`split_locals_and_reexports` and on the facade's `side_effects()` now record the
invariant that connects the two.

The mangle-exports snapshots are regenerated: their export-less entry modules no
longer emit a pointless `<locals>` module. Verified at module-key level that this
is the only change -- `lone.js` etc. still split, and the single-export `.f` key
is still emitted.

Two fixtures need deliberate accommodation, both commented in place:

- `turbopack/exports/live` asserted a plain `{ value, writable: false }`
  descriptor for a non-mutated export. Its module has exports, so it does split,
  and reading across a facade boundary always goes through a getter
  (`ReferencedAssetIdent::Module` cannot know the target's liveness). Values are
  unchanged; the fixture now asserts the getter shape *and* that the values are
  still correct.
- `turbopack/scope-hoisting/pure-comments` looks a module factory up by ident
  suffix, which the split changes. It opts out of mangling; the fixture is about
  `/*#__PURE__*/` retention.

`turbopack/async-modules/cycle-2` also opts out, but for a KNOWN BUG rather than
a property of the fixture: an import cycle of split modules containing an async
module deadlocks while building the module graph. `follow_reexports` steps
facade -> <locals> and stalls in the locals module's `get_exports()`, which
awaits the original module's `analyze()`, already in flight further up the same
cycle (~0.5% CPU, flat RSS, no Node process -- a turbo-tasks await cycle, not a
loop; `compute_async_module_info_single` is never reached). That bug is
pre-existing on canary and needs nothing but a re-export, an import cycle and a
top-level `await` -- there is a standalone reproduction on branch
`fleet/turbopack-reexport-cycle-deadlock-repro`. Mangling only makes it far
easier to reach, by splitting every module with exports rather than only
re-exporting ones. The opt-out and its `index.js` comment both say to remove it
once that is fixed.

Verified: 546 unit + 274 execution + 125 snapshot green, run to completion;
clippy and fmt clean.

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

Every fixture that opts out of mangling should say why in the fixture itself, so
a reader hitting the `options.json` does not have to guess whether it is papering
over a bug. `exports/live` and `async-modules/cycle-2` already did; this one did
not.

The reason here is benign: the test finds the module factory to inspect by
matching an ident suffix, and splitting a module into facade + `<locals>` changes
that ident. The behaviour under test -- `/*#__PURE__*/` retention -- has nothing
to do with export names.

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

…e-2 opt-out

`exports/live` asserted property descriptors on an `import * as` namespace
object. Once a module's exports are mangled it is split into a facade plus a
`<locals>` module, and the namespace those assertions see is the facade's. A
facade re-exposes every name through a getter, because it forwards a binding it
does not own and cannot know the binding is never reassigned. The assertions
therefore observed the facade rather than the value-vs-getter decision they
exist to check, and could no longer fail.

Reach the locals module instead: find its id in `__turbopack_modules__`, load
its namespace with `__turbopack_import__`, and resolve mangled keys through
`__webpack_exports_info__` evaluated in the module that owns the exports. The
non-mutated bindings are asserted to be plain values and `g` to be a getter, in
the same namespace, so the test discriminates rather than just checking that
descriptors exist. `g` is also read through the facade first: used-export
tracking is per name, and an export this fixture never mentions is dropped from
the locals module altogether.

Also removes the `mangleExportNames: false` opt-out from `async-modules/cycle-2`
and the explanation it needed. That workaround existed for the re-export cycle
deadlock, which is fixed on canary in #97933, so the fixture now passes with
mangling enabled and is byte-identical to canary again.

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

These fixtures' emitted chunk filenames are content hashed, so they move
whenever the base changes the hashing or the chunking around them. Regenerated
with the harness rather than edited by hand.

The behaviour they assert is unchanged: single-export still emits the fixed
`s(["f",0,...])` key and its consumer still reads `.f` off the `<locals>`
module. Only the filenames and the entry's reference to them move.

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

The previous explanation blamed the facade reporting its own module evaluation
as side-effect free. That was wrong: the facade also builds an
`ExportUsage::evaluation()` reference on the locals module, which is what keeps
the original module's side effects reachable.

The real chain, confirmed by removing the guard and varying the input:

- an export-less facade builds no `ExportUsage::named` references, so its
  evaluation reference is the only edge to the locals module;
- `BindingUsageInfo` drops an evaluation edge whose target is side-effect free;
- the locals module reports whatever the original module reports, and a
  `"sideEffects": false` package declaration makes that true even when the body
  does have side effects;
- unsplit, the module is a chunk group entry and evaluates regardless; split,
  its one prunable edge goes away and the body with it.

Removing the guard fails `tree-shaking/ignore-side-effect-free-module`,
`scope-hoisting/order-without-side-effects` and both `side-effects-11990`
fixtures with "No tests were run". Deleting `"sideEffects": false` from
`order-without-side-effects` makes it pass with the guard still removed, which
isolates the cause.

Also moves the facade's `side_effects` note out of a doc comment into the
function body. No behaviour change.

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

…mangling

The fixture located `a.ts`'s factory with an exact `endsWith(... (ecmascript))`
match, which misses once the module is split into a facade plus a `<locals>`
module, so it disabled mangling to keep the ident stable. Nothing it tests
involves export names, so the opt-out was working around the lookup rather than
a real interaction.

Match with `includes` instead and drop `mangleExportNames` from its options.
Only one module id matches either way -- the import is of a local binding, so it
resolves to the locals module and the facade is never chunked -- and the fixture
now passes both with mangling on and with it explicitly off.

`turbopack/exports/mangle-off` is now the only fixture that opts out, which is
the deliberate control for the feature.

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

Seed `part_references` with the locals module's evaluation reference before
the export loop rather than building a separate `vec!` and chaining the two,
which drops a temporary and an `into_iter().chain(...).collect()`. The
evaluation reference stays first, so reference order -- and therefore
evaluation order -- is unchanged; the committed snapshots are byte-identical.

Cut three comments back to what their sites actually need. The facade's
`side_effects` no longer restates the evaluation-reference rationale, which
belongs where that reference is built. The unused-reference check in
`EcmascriptModulePartReference::code_generation` says what it does and why in
two lines. `split_locals_and_reexports` keeps the two reasons a module is
split and why an export-less one is not, without walking through the pruning
mechanics of a decision made in another crate.

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

Rebasing onto `codex/turbopack-strict-chunk-factories` changes how module
factories are written into a chunk: strict factories are grouped so the
per-factory `"use strict"` directive is emitted once for the group rather than
once per factory. That shifts the emitted text of any snapshot containing a
strict factory, including the facade module this PR adds for a module whose
namespace escapes.

Regenerated with the test harness; no assertion or fixture input changed.

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

The facade a mangled module splits into is the shape the compact re-export
registration exists for: every one of its exports forwards a binding out of the
locals module, and the only other reference is that module's evaluation. Its
emitted body goes from

    var t = e.i("…/enums.js <locals>");
    e.s(["ENUM_A", ()=>t.Q, "ENUM_B", ()=>t.A, "default", ()=>t.U, "exportsInfo", ()=>t.D]);

to

    e.S(["…/enums.js <locals>", "ENUM_A,Q,ENUM_B,A,default,U,exportsInfo,D"]);

Three things were needed.

A facade is synthetic: it has no source, so `with_consumed_parse_result` hands
its code generation a default `EvalContext` with an empty `ImportMap`, which
classifies as `Normal` and could never reach the compact path. The registration
mode is therefore an option on `EcmascriptModuleContentOptions`, which the
facade sets to `Reexport` and the other synthetic wrappers set to `Normal`;
ordinary modules leave it unset and keep deriving it from their own imports.

That empty `ImportMap` also means an export's source position cannot be looked
up, and positions are what order the groups. Rather than give up, a missing
position now restricts the result to a single group -- with only one group there
is no order to get wrong, which is exactly the facade's case.

A facade's imports are `EcmascriptModulePartReference`s, so those needed the
same suppression as `EsmAssetReference`. Both check the namespace variables the
registration reports as redundant.

Also excludes async modules, found by the async-module and wasm fixtures
failing: their imports are promises that the async-module wrapper awaits and
assigns back over the namespace variables, whereas the compact registration
reads the namespace when it runs. The signal is the chunk item's
`async_module_info`, not the module's `AsyncModule` -- a facade always has the
latter, so using it disabled the very case this change is for.

Fixes the `type_complexity` lint on `CodeModuleIdsAndPaths` while here; it fails
`clippy -D warnings` on the branch this is stacked on and so blocks CI for
everything above it.

Co-authored-by: Fleet Dev Agent <fleet@vercel.com>
Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

A synthetic facade has no ImportMap of its own, so its export groups have no
source positions. That does not require abandoning the compact form:
namespace-object heads instantiate nothing, so their order is irrelevant.

Use namespace heads when several such groups are present. When there is only
one group, there is no relative order to get wrong, so let the registration use
a module-id head and subsume the facade's import entirely. This is the common
case and produces the requested form:

    e.S(["…<locals>", "ENUM_A,Q,ENUM_B,A,default,U"])

instead of retaining `var t=e.i(id)` and passing `t` as the head.

Co-authored-by: Fleet Dev Agent <fleet@vercel.com>
Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

A circuit breaker's export registration is emitted before its imports so a
cyclic importer can see the getters while this module is still evaluating. The
compact registration performs the imports itself and so cannot reproduce that
ordering, which makes it unsafe for those modules.

`is_circuit_breaker` alone cannot express this: when no export-usage analysis
has run, the conservative fallback reports every module as one, and excluding
all of them disables the feature entirely. Record whether the usage is actually
known, and exclude only modules that a real cycle was found for.

Also keys the facade's part-reference suppression on the namespace's syntax
context, matching the change below this one.

Co-authored-by: Fleet Dev Agent <fleet@vercel.com>
Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>

Co-authored-by: Fleet Dev Agent <fleet@vercel.com>
Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>