Skip to content

fix(code-splitting): keep eager entries out of group chunks in cjs output#10401

Closed
hyfdev wants to merge 2 commits into
mainfrom
fix/cjs-output-entry-group-pin
Closed

fix(code-splitting): keep eager entries out of group chunks in cjs output#10401
hyfdev wants to merge 2 commits into
mainfrom
fix/cjs-output-entry-group-pin

Conversation

@hyfdev

@hyfdev hyfdev commented Jul 23, 2026

Copy link
Copy Markdown
Member

Closed by decision. The only affected configuration — a codeSplitting group capturing a user-defined entry module itself, in cjs output — is fuzzer-generated and has no known real-world use, and this fix would silently override the user's group directive for such entries. The analysis, executed red/green fixtures, and the correct long-term direction (wrap captured entries, keep entry facades free of co-hosted modules, cycle-safe CJS chunk layout) are preserved here for whenever a real-world report appears. Tracked in #10294.

Summary

A codeSplitting group could capture a user-defined entry module into a group chunk in cjs output while the entry's body still executes eagerly. The entry chunk requires the group chunk at its top, the group chunk requires the entry chunk back, and the entry's body then reads the entry chunk's require_*/init_* wrapper exports before that chunk has installed them:

  • Unwrapped CommonJS entry (the default in cjs output): TypeError: require(...).require_leaf is not a function, and the entry's module.exports lands on the group chunk instead of the entry file.
  • Wrapped CommonJS entry (also required, e.g. by a second entry): the facade runs module.exports = require_main() before its own wrapper exports exist, and the trailing re-export assignment then clobbers the entry's exports.
  • Unwrapped ESM entry outside strict execution order: its eager body hits the same cycle through __toESM(require_leaf()) and cross-chunk live-binding getters.

Manual group capture now pins such entries to their entry chunks in cjs output: CommonJS entries in every mode, other entries while unwrapped outside strict execution order. Their dependencies follow ordinary assignment alongside them. Three adjacent shapes deliberately keep their existing grouping behavior:

  • Dynamic-import entries stay capturable: their cjs lowering defers the chunk require into Promise.resolve().then(...), which runs only after the exporting chunk finished executing (pinned by the existing optimization/chunk_merging/dynamic_entry_merged_in_common_chunk* fixtures, whose output is unchanged).
  • Under strict execution order, non-CommonJS entries remain capturable: the post-chunking order lowering legalizes captured entries itself (pinned by the existing strict fixtures that capture ESM entries in cjs output).
  • ESM output is unaffected — hoisted live-bound import cycles tolerate the ordering (pinned by a fixture control cell).

Reduced from the order-fuzzer finding tracked in #10294.

Tests

All three fixtures execute their output under node and fail on current main:

  • code_splitting/entry_group_facade_wrapper_cycle — unwrapped CJS entry requiring a CJS leaf and an ESM leaf. On main 4930a00b the group chunk calls both require_leaf() and init_leaf_esm() through the partially initialized facade and throws; with this change the plain, strict wrap-all, and strict on-demand cells pass, and an esm-format control cell pins the already-correct ESM shape.
  • code_splitting/entry_group_required_cjs_entry — the wrapped-entry arm: a second entry requires the CJS entry, giving it a require_* wrapper; on main its captured facade throws the same way and loses the entry's exports.
  • code_splitting/entry_group_esm_entry_eager_body — the unwrapped-ESM-entry arm: an ESM entry importing a CJS leaf and an ESM leaf; on main its captured eager body throws through the same cycle.

A recursive-capture probe (includeDependenciesRecursively: true) collapses back to a single chunk with the pin and stays correct.

Validation

  • cargo test --workspace --exclude rolldown_binding — full Rust suite with no snapshot changes outside the new fixtures
  • cargo clippy --workspace --all-targets -- --deny warnings, cargo fmt

@netlify

netlify Bot commented Jul 23, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit d3c7fda
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a61b232d5944f00080c30b1

@hyfdev hyfdev closed this Jul 23, 2026
graphite-app Bot pushed a commit that referenced this pull request Jul 25, 2026
…tion order (#10405)

## Bug

Two mutually unreachable CommonJS entries, co-located by a `codeSplitting` group, in cjs output with `strictExecutionOrder: true`:

```js
// effect.js (entry A)
console.log('effect');
module.exports = { name: 'effect' };

// silent.js (entry B, no relation to A)
module.exports = { name: 'silent' };
```

```js
// rolldown.config
{
  input: { effect: './effect.js', silent: './silent.js' },
  output: {
    format: 'cjs',
    strictExecutionOrder: true,
    codeSplitting: { groups: [{ name: 'common' }] },
  },
}
```

Current main emits both entry bodies raw in the group chunk:

```js
// common.js
console.log('effect');               // entry A's top level
module.exports = { name: 'effect' };
module.exports = { name: 'silent' }; // clobbers the line above

// silent.js (entry facade)
require("./common.js");              // runs A's side effects; exports lost
```

`require('./silent.js')` executes entry A and returns `{}`. This build is byte-identical to the `strictExecutionOrder: false` output — strict silently stops engaging. v1.2.0 is correct:

```js
// common.js (v1.2.0)
var require_effect = __commonJSMin((exports, module) => { console.log('effect'); module.exports = { name: 'effect' }; });
var require_silent = __commonJSMin((exports, module) => { module.exports = { name: 'silent' }; });
Object.defineProperty(exports, "require_effect", { get: () => require_effect });
Object.defineProperty(exports, "require_silent", { get: () => require_silent });

// silent.js (v1.2.0 entry facade)
module.exports = require("./common.js").require_silent();
```

Unreleased regression, bisected with the execution-order fuzzer's directed control (`cross-entry-cjs-output-strict`):

| build | result |
| --- | --- |
| v1.2.0 (npm) | pass |
| `ce98ae798` (parent of the #10104 squash) | pass |
| `1d86ffe06` (#10104) | events-reordered |
| current `main` | events-reordered |

## Cause

A CommonJS module has exactly one deferral mechanism: its `__commonJS` wrapper. Two rules decide who gets one, and after #10104 a never-imported CommonJS module falls through both:

- **Interop wrapping (link stage)** wraps a CJS module only when something imports or requires it. A CJS *entry* in cjs output has no importer, keeps `WrapKind::None`, and renders raw.
- **Order wrapping (generate stage)** is ESM-only: `is_order_wrap_eligible` requires `ExportsKind::Esm | None`, and the lowering synthesizes only `init_*`/`__esm` wrappers.

Pre-#10104, `wrap_modules` closed this gap — under `strictExecutionOrder` every CommonJS module was forced to `WrapKind::Cjs`. #10104 deleted that override together with the ESM wrap-all logic it replaced, although its own stated non-goal was "Changing CJS or require-of-ESM interop output when no order wrapper is selected". This restores the CJS half only; the ESM half is genuinely owned by order analysis now.

## Why this must live at the link stage, not in order lowering

The natural question is why order lowering doesn't just handle the case itself — it runs post-chunking and can see the hazardous placement. Detection is not the problem; repair is:

- **For CommonJS, representation *is* deferral.** An ESM order wrapper is a side-effect trigger (`init_x()`); a CJS wrapper must materialize and return `module.exports`, with `module`/`exports` rebound to closure parameters, a `module.exports = require_x()` entry facade, and live getter exports. That is the interop `WrapKind::Cjs` pathway — order lowering has no CJS-shaped tool.
- **`wrap_kind()` is immutable after linking, by #10104's own design.** `OrderWrapState` holds all late state explicitly *without changing link-stage `WrapKind`*, and the module finalizer, `render_chunk_exports`, and `compute_cross_chunk_links` all dispatch on `wrap_kind()`. Wrapping CJS late would either mutate that sealed fact or thread a second wrap-kind channel through every consumer — duplicating existing interop machinery.
- **The gate — strict plus configured `codeSplitting` groups — is the semantically correct condition.** Without strict, a lone CJS entry rendering raw is the intended minimal output. Under strict, the hazard still needs adverse placement: the only mechanism that can move a never-imported CommonJS module out of its own entry chunk is a group capture (standard chunk assignment keeps an entry with its exclusive dependencies, and every chunk-optimizer merge path either requires the entry chunk to have been emptied by a capture first or degenerates to "loading the chunk *is* executing that entry"). Placement itself is invisible to the link stage, but the groups config is not. With groups configured, `strictExecutionOrder` is exactly the user's opt-in to "stay correct under any placement, pay the wrapper bytes" — the same reasoning as wrap-all deferring hazard-free ESM modules.

Without groups the raw body is load-bearing, not just byte-smaller: review surfaced that the wrapper's synthetic `module`/`exports` parameters break a standalone CJS entry's Node module contract — `exports: 'named'` output collapses to `{ default: getter }`, `module.filename` reads undefined, `module.parent` loses its value, and `require.main === module` turns false — all of which the raw form preserves. The first revision of this PR wrapped unconditionally under strict and regressed exactly that; the gate restores main's behavior for every no-groups config.

Cost: under strict with groups configured, a never-imported CJS entry is wrapped even when no group actually captures it — identical to every released version's output under groups. If that ever matters, the refinement is a generate-stage *render* downgrade backed by a placement proof (the same conservative-first pattern as the planned `init_*` hoisted-form narrowing), not a late wrap channel.

## Tests

`function/experimental/strict_execution_order/cjs_entry_group_isolation` — the shape above, wrap-all and on-demand cells. Snapshot pins the wrapped shape; `_test.mjs` executes the output: requiring the silent entry must run no foreign top level and both entries must keep their own `module.exports`. Red on main without the fix. No existing fixture combined CommonJS-source entries with cjs-format strict output, which is why #10104's snapshot audit could not see this.

`standalone_cjs_entry_named_exports` and `standalone_cjs_entry_node_module` (from review, thanks @IWANABETHATGUY) pin the no-groups arm in the same wrap-all and on-demand cells: a standalone CJS entry under strict must keep rendering raw, preserving its named-exports shape and Node module identity (`module.filename`, `module.parent`, `require.main === module`, both required and run as the CLI). Red on this PR's first revision, green on main and on the gated fix.

## Validation

- Full integration fixture suite and workspace unit tests pass with no snapshot changes outside the new fixture; clippy `-D warnings` and `cargo fmt --check` clean.
- Execution-order fuzzer current-target audit against this branch: `cross-entry-cjs-output-strict` flips to pass. The two `cjs-output-facade-cycle` strict controls (uninitialized-wrapper-export finding from the same fuzzer list) also flip to pass — a wrapped entry is reached through a live getter instead of a not-yet-installed export. That finding's `strictExecutionOrder: false` arm is out of scope here. `family-a-existing-seed` stays known-red (separate bug).
- A four-cell matrix ({cjs,esm} × {seo:true,false}) reproduces v1.2.0 behavior exactly: cjs+strict isolated with correct exports, esm cells clean, cjs+seo:false unchanged (#9998).
- After gating on groups: full suite green again with the `cjs_entry_group_isolation` snapshot byte-identical, so every groups-present config keeps the first revision's output; no-groups configs are byte-identical to main by construction (the force-wrap block is skipped entirely). The fuzzer results above all exercise group configs and are unaffected.

Tracked in #10294 (execution-order fuzzer findings, CJS-output strict cross-entry leak). Related: #10104 (introducing change), #9998 (the pre-existing seo:false arm of the same co-location root, unchanged here), #9997 (the original strict isolation guarantee), #10401 (closed chunking-side pin for the facade-cycle shape; this change is the "wrap captured entries" direction recorded there).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant