Skip to content

fix: don't body-demand modules from simulated facade chunk includes#10351

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/10337-facade-replay-late-inclusion
Jul 21, 2026
Merged

fix: don't body-demand modules from simulated facade chunk includes#10351
graphite-app[bot] merged 1 commit into
mainfrom
fix/10337-facade-replay-late-inclusion

Conversation

@IWANABETHATGUY

Copy link
Copy Markdown
Member

Description

Fixes #10337.

Since 1.1.5, bundling a dynamically imported sideEffects: false package (three-mesh-bvh in the report) together with a codeSplitting group that matches the package crashes with:

Symbol "common_functions" in ".../three-mesh-bvh/src/webgl/glsl/common_functions.glsl.js" should belong to a chunk

Root cause

Regression from #10080. When a manual code-splitting group leaves a dynamic entry's facade chunk empty, optimize_facade_entry_chunks eliminates the facade and re-includes the entry's namespace object (SymbolIncludeReason::SimulatedFacadeChunk) so the namespace becomes an export of the merge-target chunk.

That include went through drain_body_demand_stmts, so it counted as body demand for the entry module and resurrected its deferred (gated) side-effect statements. In the reproduction, three-mesh-bvh's backwards-compatibility statement

export const shaderIntersectFunction = ` ... ${BVHShaderGLSL.common_functions} ... `;

comes back to life, and its member expression canonicalizes through the export * chain straight to the glsl leaf module. The leaf therefore becomes included after split_chunks already assigned every included module to a chunk, so it belongs to no chunk and compute_cross_chunk_links panics.

Fix

A simulated-facade include only materializes the eliminated facade's namespace object as a chunk export; it is not a semantic observation of the module, so it must not create body demand. handle_include_symbol now skips drain_body_demand_stmts for SimulatedFacadeChunk includes — mirroring the existing WorkItem::Module gate on is_simulated_facade_chunk a few lines below. This is safe because the namespace statement's symbol getters were already narrowed to link-time-used symbols before the replay, so the include cannot legitimately need anything that wasn't included at link time.

This also restores the invariant that chunk-layout choices don't change tree-shaking results: with the fix, the grouped build emits exactly what the ungrouped build emits (the dead ${common_functions} statement no longer appears).

Alternatives considered

Instead of (or in addition to) the semantic gate, the generate stage could detect modules newly included by the replay and assign them to the merge-target chunk. That converts the panic into working output, but it ships code the ungrouped build proves dead, and it papers over inclusion growing after chunk assignment instead of preventing it — so the semantic gate alone is preferred.

Test

crates/rolldown/tests/rolldown/issues/10337 mirrors the three-mesh-bvh shape: dynamic import() → an index module with export * as plus the deferred template-literal statement → an export * barrel → the glsl leaf, with treeshake.moduleSideEffects: false and a codeSplitting group. The entry only touches res.MeshBVH, so the leaf is reachable exclusively through the facade-elimination replay. The fixture panics on main and passes with this fix; it also executes the output under node and asserts the namespace member exists.

@netlify

netlify Bot commented Jul 19, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

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

@IWANABETHATGUY
IWANABETHATGUY marked this pull request as ready for review July 20, 2026 16:43
@IWANABETHATGUY
IWANABETHATGUY force-pushed the fix/10337-facade-replay-late-inclusion branch from c7a5768 to 2f97b88 Compare July 20, 2026 16:43
@codspeed-hq

codspeed-hq Bot commented Jul 20, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing fix/10337-facade-replay-late-inclusion (2f97b88) with main (ae2ec2e)

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@hyfdev hyfdev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The synthetic facade include now preserves the link-stage tree-shaking decision while still materializing the narrowed dynamic namespace.

— AI review by gpt-5

IWANABETHATGUY commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • Jul 21, 9:21 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 21, 9:21 AM UTC: IWANABETHATGUY added this pull request to the Graphite merge queue.
  • Jul 21, 9:26 AM UTC: The Graphite merge queue couldn't merge this PR because it was not satisfying all requirements (Failed CI: 'node-dev-server-test-ubuntu (20) / Node Dev Server Test').
  • Jul 21, 1:36 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 21, 1:41 PM UTC: hyfdev added this pull request to the Graphite merge queue.
  • Jul 21, 1:47 PM UTC: Merged by the Graphite merge queue.

graphite-app Bot pushed a commit that referenced this pull request Jul 21, 2026
…10351)

### Description

Fixes #10337.

Since 1.1.5, bundling a dynamically imported `sideEffects: false` package (three-mesh-bvh in the report) together with a `codeSplitting` group that matches the package crashes with:

```
Symbol "common_functions" in ".../three-mesh-bvh/src/webgl/glsl/common_functions.glsl.js" should belong to a chunk
```

### Root cause

Regression from #10080. When a manual code-splitting group leaves a dynamic entry's facade chunk empty, `optimize_facade_entry_chunks` eliminates the facade and re-includes the entry's namespace object (`SymbolIncludeReason::SimulatedFacadeChunk`) so the namespace becomes an export of the merge-target chunk.

That include went through `drain_body_demand_stmts`, so it counted as *body demand* for the entry module and resurrected its deferred (gated) side-effect statements. In the reproduction, three-mesh-bvh's backwards-compatibility statement

```js
export const shaderIntersectFunction = ` ... ${BVHShaderGLSL.common_functions} ... `;
```

comes back to life, and its member expression canonicalizes through the `export *` chain straight to the glsl leaf module. The leaf therefore becomes included **after** `split_chunks` already assigned every included module to a chunk, so it belongs to no chunk and `compute_cross_chunk_links` panics.

### Fix

A simulated-facade include only materializes the eliminated facade's namespace object as a chunk export; it is not a semantic observation of the module, so it must not create body demand. `handle_include_symbol` now skips `drain_body_demand_stmts` for `SimulatedFacadeChunk` includes — mirroring the existing `WorkItem::Module` gate on `is_simulated_facade_chunk` a few lines below. This is safe because the namespace statement's symbol getters were already narrowed to link-time-used symbols before the replay, so the include cannot legitimately need anything that wasn't included at link time.

This also restores the invariant that chunk-layout choices don't change tree-shaking results: with the fix, the grouped build emits exactly what the ungrouped build emits (the dead `${common_functions}` statement no longer appears).

### Alternatives considered

Instead of (or in addition to) the semantic gate, the generate stage could detect modules newly included by the replay and assign them to the merge-target chunk. That converts the panic into working output, but it ships code the ungrouped build proves dead, and it papers over inclusion growing after chunk assignment instead of preventing it — so the semantic gate alone is preferred.

### Test

`crates/rolldown/tests/rolldown/issues/10337` mirrors the three-mesh-bvh shape: dynamic `import()` → an index module with `export * as` plus the deferred template-literal statement → an `export *` barrel → the glsl leaf, with `treeshake.moduleSideEffects: false` and a `codeSplitting` group. The entry only touches `res.MeshBVH`, so the leaf is reachable exclusively through the facade-elimination replay. The fixture panics on `main` and passes with this fix; it also executes the output under node and asserts the namespace member exists.
@graphite-app
graphite-app Bot force-pushed the fix/10337-facade-replay-late-inclusion branch from 2f97b88 to 4d52fb7 Compare July 21, 2026 09:22
hyfdev pushed a commit that referenced this pull request Jul 21, 2026
…10351)

### Description

Fixes #10337.

Since 1.1.5, bundling a dynamically imported `sideEffects: false` package (three-mesh-bvh in the report) together with a `codeSplitting` group that matches the package crashes with:

```
Symbol "common_functions" in ".../three-mesh-bvh/src/webgl/glsl/common_functions.glsl.js" should belong to a chunk
```

### Root cause

Regression from #10080. When a manual code-splitting group leaves a dynamic entry's facade chunk empty, `optimize_facade_entry_chunks` eliminates the facade and re-includes the entry's namespace object (`SymbolIncludeReason::SimulatedFacadeChunk`) so the namespace becomes an export of the merge-target chunk.

That include went through `drain_body_demand_stmts`, so it counted as *body demand* for the entry module and resurrected its deferred (gated) side-effect statements. In the reproduction, three-mesh-bvh's backwards-compatibility statement

```js
export const shaderIntersectFunction = ` ... ${BVHShaderGLSL.common_functions} ... `;
```

comes back to life, and its member expression canonicalizes through the `export *` chain straight to the glsl leaf module. The leaf therefore becomes included **after** `split_chunks` already assigned every included module to a chunk, so it belongs to no chunk and `compute_cross_chunk_links` panics.

### Fix

A simulated-facade include only materializes the eliminated facade's namespace object as a chunk export; it is not a semantic observation of the module, so it must not create body demand. `handle_include_symbol` now skips `drain_body_demand_stmts` for `SimulatedFacadeChunk` includes — mirroring the existing `WorkItem::Module` gate on `is_simulated_facade_chunk` a few lines below. This is safe because the namespace statement's symbol getters were already narrowed to link-time-used symbols before the replay, so the include cannot legitimately need anything that wasn't included at link time.

This also restores the invariant that chunk-layout choices don't change tree-shaking results: with the fix, the grouped build emits exactly what the ungrouped build emits (the dead `${common_functions}` statement no longer appears).

### Alternatives considered

Instead of (or in addition to) the semantic gate, the generate stage could detect modules newly included by the replay and assign them to the merge-target chunk. That converts the panic into working output, but it ships code the ungrouped build proves dead, and it papers over inclusion growing after chunk assignment instead of preventing it — so the semantic gate alone is preferred.

### Test

`crates/rolldown/tests/rolldown/issues/10337` mirrors the three-mesh-bvh shape: dynamic `import()` → an index module with `export * as` plus the deferred template-literal statement → an `export *` barrel → the glsl leaf, with `treeshake.moduleSideEffects: false` and a `codeSplitting` group. The entry only touches `res.MeshBVH`, so the leaf is reachable exclusively through the facade-elimination replay. The fixture panics on `main` and passes with this fix; it also executes the output under node and asserts the namespace member exists.
@hyfdev
hyfdev force-pushed the fix/10337-facade-replay-late-inclusion branch from 4d52fb7 to eea5917 Compare July 21, 2026 13:36
…10351)

### Description

Fixes #10337.

Since 1.1.5, bundling a dynamically imported `sideEffects: false` package (three-mesh-bvh in the report) together with a `codeSplitting` group that matches the package crashes with:

```
Symbol "common_functions" in ".../three-mesh-bvh/src/webgl/glsl/common_functions.glsl.js" should belong to a chunk
```

### Root cause

Regression from #10080. When a manual code-splitting group leaves a dynamic entry's facade chunk empty, `optimize_facade_entry_chunks` eliminates the facade and re-includes the entry's namespace object (`SymbolIncludeReason::SimulatedFacadeChunk`) so the namespace becomes an export of the merge-target chunk.

That include went through `drain_body_demand_stmts`, so it counted as *body demand* for the entry module and resurrected its deferred (gated) side-effect statements. In the reproduction, three-mesh-bvh's backwards-compatibility statement

```js
export const shaderIntersectFunction = ` ... ${BVHShaderGLSL.common_functions} ... `;
```

comes back to life, and its member expression canonicalizes through the `export *` chain straight to the glsl leaf module. The leaf therefore becomes included **after** `split_chunks` already assigned every included module to a chunk, so it belongs to no chunk and `compute_cross_chunk_links` panics.

### Fix

A simulated-facade include only materializes the eliminated facade's namespace object as a chunk export; it is not a semantic observation of the module, so it must not create body demand. `handle_include_symbol` now skips `drain_body_demand_stmts` for `SimulatedFacadeChunk` includes — mirroring the existing `WorkItem::Module` gate on `is_simulated_facade_chunk` a few lines below. This is safe because the namespace statement's symbol getters were already narrowed to link-time-used symbols before the replay, so the include cannot legitimately need anything that wasn't included at link time.

This also restores the invariant that chunk-layout choices don't change tree-shaking results: with the fix, the grouped build emits exactly what the ungrouped build emits (the dead `${common_functions}` statement no longer appears).

### Alternatives considered

Instead of (or in addition to) the semantic gate, the generate stage could detect modules newly included by the replay and assign them to the merge-target chunk. That converts the panic into working output, but it ships code the ungrouped build proves dead, and it papers over inclusion growing after chunk assignment instead of preventing it — so the semantic gate alone is preferred.

### Test

`crates/rolldown/tests/rolldown/issues/10337` mirrors the three-mesh-bvh shape: dynamic `import()` → an index module with `export * as` plus the deferred template-literal statement → an `export *` barrel → the glsl leaf, with `treeshake.moduleSideEffects: false` and a `codeSplitting` group. The entry only touches `res.MeshBVH`, so the leaf is reachable exclusively through the facade-elimination replay. The fixture panics on `main` and passes with this fix; it also executes the output under node and asserts the namespace member exists.
@graphite-app
graphite-app Bot force-pushed the fix/10337-facade-replay-late-inclusion branch from eea5917 to 96b05d1 Compare July 21, 2026 13:43
@graphite-app
graphite-app Bot merged commit 96b05d1 into main Jul 21, 2026
34 checks passed
@graphite-app
graphite-app Bot deleted the fix/10337-facade-replay-late-inclusion branch July 21, 2026 13:47
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.

[Panic]: Symbol "..." should belong to a chunk in compute_cross_chunk_links with codeSplitting groups + dynamic import (regression in 1.1.5)

2 participants