fix: don't body-demand modules from simulated facade chunk includes#10351
Merged
Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
IWANABETHATGUY
marked this pull request as ready for review
July 20, 2026 16:43
IWANABETHATGUY
force-pushed
the
fix/10337-facade-replay-late-inclusion
branch
from
July 20, 2026 16:43
c7a5768 to
2f97b88
Compare
Merging this PR will not alter performance
Comparing Footnotes
|
hyfdev
approved these changes
Jul 21, 2026
hyfdev
left a comment
Member
There was a problem hiding this comment.
The synthetic facade include now preserves the link-stage tree-shaking decision while still materializing the narrowed dynamic namespace.
— AI review by gpt-5
Member
Author
Merge activity
|
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
Bot
force-pushed
the
fix/10337-facade-replay-late-inclusion
branch
from
July 21, 2026 09:22
2f97b88 to
4d52fb7
Compare
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
force-pushed
the
fix/10337-facade-replay-late-inclusion
branch
from
July 21, 2026 13:36
4d52fb7 to
eea5917
Compare
…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
Bot
force-pushed
the
fix/10337-facade-replay-late-inclusion
branch
from
July 21, 2026 13:43
eea5917 to
96b05d1
Compare
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Fixes #10337.
Since 1.1.5, bundling a dynamically imported
sideEffects: falsepackage (three-mesh-bvh in the report) together with acodeSplittinggroup that matches the package crashes with:Root cause
Regression from #10080. When a manual code-splitting group leaves a dynamic entry's facade chunk empty,
optimize_facade_entry_chunkseliminates 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 statementcomes back to life, and its member expression canonicalizes through the
export *chain straight to the glsl leaf module. The leaf therefore becomes included aftersplit_chunksalready assigned every included module to a chunk, so it belongs to no chunk andcompute_cross_chunk_linkspanics.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_symbolnow skipsdrain_body_demand_stmtsforSimulatedFacadeChunkincludes — mirroring the existingWorkItem::Modulegate onis_simulated_facade_chunka 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/10337mirrors the three-mesh-bvh shape: dynamicimport()→ an index module withexport * asplus the deferred template-literal statement → anexport *barrel → the glsl leaf, withtreeshake.moduleSideEffects: falseand acodeSplittinggroup. The entry only touchesres.MeshBVH, so the leaf is reachable exclusively through the facade-elimination replay. The fixture panics onmainand passes with this fix; it also executes the output under node and asserts the namespace member exists.