fix(code-splitting): avoid the dynamic-entry facade when the same-chunk collapse carries the trigger#10433
Open
hyfdev wants to merge 3 commits into
Open
fix(code-splitting): avoid the dynamic-entry facade when the same-chunk collapse carries the trigger#10433hyfdev wants to merge 3 commits into
hyfdev wants to merge 3 commits into
Conversation
…nk collapse carries the trigger When a dynamic entry's eliminated facade chunk has only same-chunk dynamic importers, restore_order_wrap_entry_facades no longer resurrects it: the merged-entry rewrite carries the entry trigger inline through the shared esm_init_target view (Promise.resolve().then(() => (init_x(), ns))), matching the non-strict chunk-merging output. A cross-chunk dynamic importer still forces the restore, because the facade runs the trigger synchronously within its module evaluation rather than in a .then microtask after the host chunk settles (pinned by m4_dynamic_facade_race). Emitted-chunk facades are always restored so emitFile reference ids keep resolving to a real file.
✅ Deploy Preview for rolldown-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Drop the incidental static import, chunkOptimization flag, and includeDependenciesRecursively/preserveEntrySignatures pair: the group alone co-locates target.js with entry a and produces the eliminated phantom facade. Without the static import the on-demand cell also order-wraps the target, so both cells now exercise the restore boundary instead of wrap-all only. Verified the snapshot still pins the boundary: neutralizing the cross-chunk guard flips the fixture's snapshot (facade dropped) while its runtime assertions alone would not catch it — the microtask race itself stays pinned by m4_dynamic_facade_race.
…mic-facade-collapse # Conflicts: # crates/rolldown/src/module_finalizers/mod.rs # crates/rolldown/tests/rolldown/function/experimental/strict_execution_order/manual_group_phantom_dynamic/artifacts.snap # crates/rolldown/tests/rolldown/function/experimental/strict_execution_order/mixed_static_dynamic_same_target/artifacts.snap # crates/rolldown/tests/rolldown/issues/9463/artifacts.snap # crates/rolldown/tests/rolldown/issues/9463_plain_group/artifacts.snap # crates/rolldown/tests/rolldown/optimization/chunk_merging/dynamic_entry_merged_in_user_defined_entry/artifacts.snap
Merging this PR will not alter performance
Comparing Footnotes
|
19 tasks
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.
Under
strictExecutionOrder, dynamically importing a module that chunking already placed in the importer's own chunk still shipped a dedicated facade chunk, and theimport()stayed a real network fetch of that file. After this PR the import collapses inline and carries the module's deferred initialization with it, so the facade chunk disappears — matching what the same layout already produced without strict mode. A facade is still emitted whenever any other chunk dynamically imports the module.Problem
Second P1 item of #10294. Strict-order builds defer each module's execution behind an initializer, and a dynamically imported module needs that initializer called when the import resolves. When code splitting co-locates the import target with its importer — a
codeSplittinggroup matching both, orexperimental.chunkOptimizationmerging the dynamic entry into its host — the build resurrected the eliminated facade chunk to carry that call, even though nothing else would ever load it.issues/9463wrap-all ships ashared.jschunk containing only a noop initializer call plus a re-export; the same shape recurs with real, non-noop initializers inmanual_group_phantom_dynamic,mixed_static_dynamic_same_target, andchunk_merging/dynamic_entry_merged_in_user_defined_entry. Wrap-all hits every such entry; on-demand hits whichever targets the analysis wraps.Root cause: the restore pass (
restore_order_wrap_entry_facades, which un-tombstones facade chunks the chunk optimizer eliminated) ran unconditionally for every wrapped entry, because the finalizer's merged-entry rewrite (rewrite_dynamic_import_for_merged_entry, which turnsimport()of a merged dynamic entry into an inline namespace access) could not express an execution-order trigger — its non-interop arms asserted such entries never reach them. The interop arm already carries its wrapper's trigger through the collapse, and the cross-chunk links stage already publishes execution-order wrapper exports; the machinery was half-built, so the restore undid every merge the optimizer had proved.Blast radius: output shape only — one extra chunk and one extra request per affected dynamic entry; runtime behavior was already correct. Not affected: non-strict builds, single-chunk builds,
emitFile'd chunks, and any dynamic entry with a cross-chunk dynamic importer.Fix
Keep the facade eliminated exactly when every live dynamic importer sits in the chunk that hosts the entry's implementation, and let the collapse carry the trigger.
esm_init_targetview, which presents interop and execution-order wrappers uniformly, and emitsPromise.resolve().then(() => (init_x(), ns))for the same-chunk case. A noop initializer call gets the/* @__PURE__ */annotation, so the default dce-only minify drops it —issues/9463wrap-all becomes byte-identical to its on-demand cell. A newdebug_assert!(!target.tla_tainted)pins why the non-awaiting collapse is sound: facade elimination is globally disabled when any module is TLA-tainted..thentrigger would run a microtask after the host chunk settles — an observable reordering thatm4_dynamic_facade_raceandwrapped_dynamic_entry_keeps_facade_after_manual_chunk_mergepin (both stay green). Emitted-chunk facades are always restored soemitFilereference ids keep resolving to a real file. The importer scan is one pass over the module table shared by all restore candidates.Why not the ledger's narrower alternative — skip the facade only when the initializer is a noop: three of the four anchor fixtures have real initializers (
init_m6,init_dep_b,init_lib), so only trigger-carrying collapse removes their facades.Non-goals: the cross-chunk
esm_init_targetarm is dead-by-construction today (any cross-chunk importer forces the restore) and is kept for symmetry, not as a functional path; routing the interop arm through the shared init-call helper widens the PURE-when-noop annotation to merged interop entries in non-strict builds (no existing snapshot changes); the wrap-all-side unconditional entry split stays as tracked in #10294.Validation
strict_execution_order/cross_chunk_dynamic_importer_keeps_facadepins the boundary in minimal form (two entries, one target, one group): one same-chunk plus one cross-chunk dynamic importer keeps the restored facade with its synchronous trigger, in both wrap modes. Verified it bites: neutralizing the cross-chunk guard flips this fixture's snapshot, while its runtime assertions alone would not catch it — the microtask-race semantics stay pinned bym4_dynamic_facade_race._test.mjsassertions:issues/9463wrap-all loses theshared.jsfacade (byte-identical to on-demand),issues/9463_plain_grouplikewise;manual_group_phantom_dynamicdropsm6.jsin both wrap modes;mixed_static_dynamic_same_targetdropsdep-b.js;chunk_merging/dynamic_entry_merged_in_user_defined_entrydropslib.jsin the esm and cjs wrap-all cells and stops publishing the now-dead exports.cargo test -p rolldown --test integration: 1889 passed; the only failures are the pre-existing environment set (cjs_module_lexer_compat×3,npm_packages/util_deprecate,test262), verified byte-identical by rerun on unmodified main on the same machine.strictExecutionOrder/onDemandWrapping: follow-up ledger #10294, with byte-identical verdict signatures on both builds — zero failure signatures introduced or altered.Part of #10294.