Skip to content

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
mainfrom
fix/strict-order-dynamic-facade-collapse
Open

fix(code-splitting): avoid the dynamic-entry facade when the same-chunk collapse carries the trigger#10433
hyfdev wants to merge 3 commits into
mainfrom
fix/strict-order-dynamic-facade-collapse

Conversation

@hyfdev

@hyfdev hyfdev commented Jul 24, 2026

Copy link
Copy Markdown
Member

Under strictExecutionOrder, dynamically importing a module that chunking already placed in the importer's own chunk still shipped a dedicated facade chunk, and the import() 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.

// before — the importer fetches a file whose whole job is the trigger:
import("./lib.js").then((mod) => ...)
// dist/lib.js:
//   import { n as a, r as init_lib } from "./entry2.js";
//   init_lib();
//   export { a };

// after — dist/lib.js is gone; the collapse carries the trigger:
Promise.resolve().then(() => (init_lib(), lib_exports)).then((mod) => ...)

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 codeSplitting group matching both, or experimental.chunkOptimization merging 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/9463 wrap-all ships a shared.js chunk containing only a noop initializer call plus a re-export; the same shape recurs with real, non-noop initializers in manual_group_phantom_dynamic, mixed_static_dynamic_same_target, and chunk_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 turns import() 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.

  • The merged-entry rewrite now routes both arms through the shared esm_init_target view, which presents interop and execution-order wrappers uniformly, and emits Promise.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/9463 wrap-all becomes byte-identical to its on-demand cell. A new debug_assert!(!target.tla_tainted) pins why the non-awaiting collapse is sound: facade elimination is globally disabled when any module is TLA-tainted.
  • The restore still runs when any cross-chunk dynamic importer exists: a facade runs the trigger synchronously within its own module evaluation, while a .then trigger would run a microtask after the host chunk settles — an observable reordering that m4_dynamic_facade_race and wrapped_dynamic_entry_keeps_facade_after_manual_chunk_merge pin (both stay green). Emitted-chunk facades are always restored so emitFile reference ids keep resolving to a real file. The importer scan is one pass over the module table shared by all restore candidates.
  • The plan-applied debug assertion now accepts an entry whose entry chunk is another module's entry chunk containing it (dynamic entry merged into a user-defined entry), a state the unconditional restore previously made unreachable.

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_target arm 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

  • New fixture strict_execution_order/cross_chunk_dynamic_importer_keeps_facade pins 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 by m4_dynamic_facade_race.
  • Acceptance and anchors, green with runtime _test.mjs assertions: issues/9463 wrap-all loses the shared.js facade (byte-identical to on-demand), issues/9463_plain_group likewise; manual_group_phantom_dynamic drops m6.js in both wrap modes; mixed_static_dynamic_same_target drops dep-b.js; chunk_merging/dynamic_entry_merged_in_user_defined_entry drops lib.js in the esm and cjs wrap-all cells and stops publishing the now-dead exports.
  • Full 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.
  • Execution-order fuzzer differential, 80 mixed cases per cell (seeds 400001–400080) in on-demand and wrap-all against this build and an unmodified-main build: identical results in all four cells (78/80); the two failing seeds are the known packaged pure-definer-behind-barrel residue tracked in strictExecutionOrder / onDemandWrapping: follow-up ledger #10294, with byte-identical verdict signatures on both builds — zero failure signatures introduced or altered.
  • Not exercised by a fixture: a TLA-tainted entry reaching the collapse. It is unreachable today (facade elimination is globally disabled when any module is TLA-tainted) and the new debug assertion pins that coupling.

Part of #10294.

…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.
@netlify

netlify Bot commented Jul 24, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs ready!

Name Link
🔨 Latest commit 14f6c01
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a632f5800222a0008634cca
😎 Deploy Preview https://deploy-preview-10433--rolldown-rs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

hyfdev added 2 commits July 24, 2026 12:59
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
@hyfdev
hyfdev marked this pull request as ready for review July 25, 2026 03:27
@hyfdev
hyfdev requested a review from IWANABETHATGUY as a code owner July 25, 2026 03:27
Copilot AI review requested due to automatic review settings July 25, 2026 03:27
@hyfdev
hyfdev requested a review from shulaoda as a code owner July 25, 2026 03:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codspeed-hq

codspeed-hq Bot commented Jul 25, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing fix/strict-order-dynamic-facade-collapse (14f6c01) with main (7e13de4)2

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.

  2. No successful run was found on main (4fbb0c9) during the generation of this report, so 7e13de4 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

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.

2 participants