fix: skip init-call transfer for circular wrapper deps#10270
fix: skip init-call transfer for circular wrapper deps#10270marvinhagemeister wants to merge 2 commits into
Conversation
Transferring an init_*() call to an earlier unwrapped module makes it run eagerly at top level. If the wrapper body calls init_*() for a module declared later in the output, that variable is still undefined (TypeError: init_* is not a function). This occurs with circular imports (a.js <-> b.js): moving init_b() to an earlier position runs it before var init_a is assigned. Skip the transfer when the wrapped module depends on any WrapKind::Esm module with higher exec order than the transfer target. The init call then stays inside the importer lazy wrapper closure where it is safe. Fixes rolldown#10265
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
|
|
Sweet, that's perfect! FWIW: I was able to reproduce it even in a larger codebase without |
Then I guess you probably did not fix the original issues in the large codebase, but thanks for reporting the bug with |
## Summary - Add the circular ESM initialization graph reported in #10265, whose legacy init-call transfer failure is addressed by #10270. - Exercise both strict modes: `strictExecutionOrder: true` by itself (wrap-all), and strict execution order with `experimental.onDemandWrapping: true`. - Execute the generated entry in both cells and assert that the circular `init_a` / `init_b` path completes without calling an uninitialized wrapper. ## What this proves The failure addressed by #10270 comes from the legacy `ensure_lazy_module_initialization_order()` transfer moving an `init_*()` call before a later wrapper assignment. With #10104's new strict implementation, that function is still called but returns before running the transfer logic; the wrap-all or on-demand execution-order analysis owns scheduling instead. The exact circular graph executes successfully in both cells, demonstrating that this failure mode is absent from both new strict paths. Current `main` at `03e1e3422` also passes both strict configurations through the old implementation, so this is path-specific regression coverage rather than a `main`-fails / parent-passes claim. ## Scope This conclusion is limited to strict execution order, in wrap-all and on-demand modes. Default and non-strict output still use the legacy transfer logic and are outside this test. Refs #10265 and #10270. ## Validation - `CARGO_TARGET_DIR=/tmp/codex-rolldown-pr10104-target just t-run crates/rolldown/tests/rolldown/issues/10265/_config.json` (twice; snapshot-stable) - Both strict cells also pass on `main` at `03e1e3422`, with different generated layouts - `just lint-node` - `just lint-repo` - `git diff --check`
|
Thanks for your effort. Ive added a test case under the new impl #10104 to ensure it's covered. New impl would be shipped this Wendesaday's release. |
3 participants
When a wrapped ESM module imports another wrapped module that appears later in the output (circular imports), the init-call transfer logic in
ensure_lazy_module_initialization_ordercan move aninit_*()call to an earlier unwrapped module's top-level position. The call then runs before the later wrapper'svar init_* = __esmMin(...)is assigned, causingTypeError: init_* is not a function.This was reported in #10265 with
onDemandWrapping, but the underlying issue can trigger whenever unwrapped modules coexist with circular wrapped dependencies.Fixes #10265