Skip to content

Commit 27026df

Browse files
h-a-n-ah-a-n-a
committed
fix(dev/lazy): avoid module reinitialization in lazy compilation patches (#9179)
## Summary When a module appears in two lazy-compilation chunks served concurrently (e.g. two `compileLazyEntry` requests racing for `shared.js`), the browser runs the module body twice. Non-idempotent module code blows up — concretely, `Object.defineProperty(this, "key", { configurable: false })` on the second run throws `Cannot redefine property`. This PR makes module initializers opt into runtime-side deduping: - `createEsmInitializer` / `createCjsInitializer` take a new `dedup` flag. When truthy and the stable id is already in `runtime.modules`, the factory is skipped (or, for CJS, the registered exports are reused). - The Rust emitter (`HmrAstFinalizer`) gains a `dedup_module_initializer` field. `compile_lazy_entry` sets it to `true`; the two HMR-patch render paths set it to `false` so HMR keeps re-running the factory and replacing exports — that's the whole point of a patch. - The stable module id is now passed into the factory as `__rolldown_module_id__`, so `registerModule` and `createModuleHotContext` reference it by identifier instead of duplicating the string literal. - The lazy proxy template deletes its own `runtime.modules` entry before triggering the real chunk, so the dedup gate on the real module's id starts from a clean slate. The dedup gate is a workaround. The proper fix is a runtime API for module disposal that HMR can call before re-execution; see the TODO on `HmrAstFinalizer::dedup_module_initializer`. ## Test plan - [x] `cargo test -p rolldown --test integration` (1690 passed, 70 ignored) - [x] `cargo clippy -p rolldown --tests --no-deps` clean
1 parent 0689e35 commit 27026df

21 files changed

Lines changed: 313 additions & 219 deletions

File tree

‎crates/rolldown/src/hmr/hmr_ast_finalizer.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,19 @@ pub struct HmrAstFinalizer<'me, 'ast> {
3232
pub affected_module_idx_to_init_fn_name: &'me FxHashMap<ModuleIdx, String>,
3333
pub use_pife_for_module_wrappers: bool,
3434

35+
/// Whether the runtime should short-circuit re-execution of the wrapped module body
36+
/// when its stable id is already registered.
37+
///
38+
/// `true` for lazy-compilation chunks: when a module appears in two lazy bundles served
39+
/// concurrently, we want only the first one to actually execute the body.
40+
///
41+
/// `false` for HMR patches: the patch's whole point is to re-execute the body and
42+
/// replace the registered exports, so deduping would silently drop the update.
43+
///
44+
/// Note: This works only as a **workaround**. In the future, HMR runtime should
45+
/// provide a runtime API to trigger module disposal and re-execution. @hana
46+
pub dedup_module_initializer: bool,
47+
3548
// Each module has a unique index, which is used to generate something that needs to be unique.
3649
pub unique_index: usize,
3750

‎crates/rolldown/src/hmr/hmr_stage.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,9 @@ impl<'a, Fs: FileSystem + Clone + 'static> HmrStage<'a, Fs> {
467467
exports: oxc::allocator::Vec::new_in(fields.allocator),
468468
affected_module_idx_to_init_fn_name: &module_idx_to_init_fn_name,
469469
use_pife_for_module_wrappers,
470+
// Lazy chunk: opt the runtime into deduping the module body so two
471+
// concurrent lazy bundles for the same module don't double-execute it.
472+
dedup_module_initializer: true,
470473
dependencies: FxIndexSet::default(),
471474
imports: FxHashSet::default(),
472475
generated_static_import_infos: FxHashMap::default(),
@@ -704,6 +707,7 @@ impl<'a, Fs: FileSystem + Clone + 'static> HmrStage<'a, Fs> {
704707
exports: oxc::allocator::Vec::new_in(fields.allocator),
705708
affected_module_idx_to_init_fn_name: &module_idx_to_init_fn_name,
706709
use_pife_for_module_wrappers,
710+
dedup_module_initializer: false,
707711
dependencies: FxIndexSet::default(),
708712
imports: FxHashSet::default(),
709713
generated_static_import_infos: FxHashMap::default(),
@@ -895,6 +899,7 @@ impl<'a, Fs: FileSystem + Clone + 'static> HmrStage<'a, Fs> {
895899
exports: oxc::allocator::Vec::new_in(fields.allocator),
896900
affected_module_idx_to_init_fn_name: &module_idx_to_init_fn_name,
897901
use_pife_for_module_wrappers,
902+
dedup_module_initializer: false,
898903
dependencies: FxIndexSet::default(),
899904
imports: FxHashSet::default(),
900905
generated_static_import_infos: FxHashMap::default(),

‎crates/rolldown/src/hmr/impl_traverse_for_hmr_ast_finalizer.rs‎

Lines changed: 89 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ use rolldown_ecmascript::{
99
CJS_ROLLDOWN_EXPORTS_REF_IDENT, CJS_ROLLDOWN_MODULE_REF_IDENT,
1010
};
1111

12-
use crate::hmr::{hmr_ast_finalizer::HmrAstFinalizer, utils::HmrAstBuilder};
12+
use crate::hmr::{
13+
hmr_ast_finalizer::HmrAstFinalizer,
14+
utils::{HmrAstBuilder, MODULE_ID_PARAM_FOR_HMR},
15+
};
1316

1417
impl<'ast> Traverse<'ast, ()> for HmrAstFinalizer<'_, 'ast> {
1518
fn enter_program(
@@ -63,46 +66,63 @@ impl<'ast> Traverse<'ast, ()> for HmrAstFinalizer<'_, 'ast> {
6366

6467
let init_fn_name = &self.affected_module_idx_to_init_fn_name[&self.module.idx];
6568

66-
let mut params = self.snippet.builder.formal_parameters(
69+
// The runtime wrappers (createEsmInitializer / createCjsInitializer) call the body
70+
// with the module's stable id as an extra argument, so it's available inside the body
71+
// as `__rolldown_module_id__`. This lets registerModule / createModuleHotContext reference
72+
// the id by identifier instead of duplicating the string literal.
73+
let module_id_param = self.snippet.builder.formal_parameter(
74+
SPAN,
75+
self.builder.vec(),
76+
self.snippet.builder.binding_pattern_binding_identifier(SPAN, MODULE_ID_PARAM_FOR_HMR),
77+
NONE,
78+
NONE,
79+
false,
80+
None,
81+
false,
82+
false,
83+
);
84+
let params = self.snippet.builder.formal_parameters(
6785
SPAN,
6886
ast::FormalParameterKind::Signature,
69-
self.snippet.builder.vec_with_capacity(2),
87+
{
88+
if self.module.exports_kind.is_commonjs() {
89+
self.snippet.builder.vec_from_array([
90+
self.snippet.builder.formal_parameter(
91+
SPAN,
92+
self.builder.vec(),
93+
self
94+
.snippet
95+
.builder
96+
.binding_pattern_binding_identifier(SPAN, CJS_ROLLDOWN_EXPORTS_REF_IDENT),
97+
NONE,
98+
NONE,
99+
false,
100+
None,
101+
false,
102+
false,
103+
),
104+
self.snippet.builder.formal_parameter(
105+
SPAN,
106+
self.builder.vec(),
107+
self
108+
.snippet
109+
.builder
110+
.binding_pattern_binding_identifier(SPAN, CJS_ROLLDOWN_MODULE_REF_IDENT),
111+
NONE,
112+
NONE,
113+
false,
114+
None,
115+
false,
116+
false,
117+
),
118+
module_id_param,
119+
])
120+
} else {
121+
self.snippet.builder.vec1(module_id_param)
122+
}
123+
},
70124
NONE,
71125
);
72-
if self.module.exports_kind.is_commonjs() {
73-
params.items.push(
74-
self.snippet.builder.formal_parameter(
75-
SPAN,
76-
self.builder.vec(),
77-
self
78-
.snippet
79-
.builder
80-
.binding_pattern_binding_identifier(SPAN, CJS_ROLLDOWN_EXPORTS_REF_IDENT),
81-
NONE,
82-
NONE,
83-
false,
84-
None,
85-
false,
86-
false,
87-
),
88-
);
89-
params.items.push(
90-
self.snippet.builder.formal_parameter(
91-
SPAN,
92-
self.builder.vec(),
93-
self
94-
.snippet
95-
.builder
96-
.binding_pattern_binding_identifier(SPAN, CJS_ROLLDOWN_MODULE_REF_IDENT),
97-
NONE,
98-
NONE,
99-
false,
100-
None,
101-
false,
102-
false,
103-
),
104-
);
105-
}
106126
// function () { [user code] }
107127
let mut user_code_wrapper = self.snippet.builder.alloc_function(
108128
SPAN,
@@ -124,31 +144,41 @@ impl<'ast> Traverse<'ast, ()> for HmrAstFinalizer<'_, 'ast> {
124144
// mark the callback as PIFE because the callback is executed when this chunk is loaded
125145
user_code_wrapper.pife = self.use_pife_for_module_wrappers;
126146

127-
let initializer_call = if self.module.exports_kind.is_commonjs() {
128-
// __rolldown__runtime.createCjsInitializer((function (exports, module) { [user code] }))
129-
self.snippet.builder.alloc_call_expression(
147+
// Initializer call arguments: always (stable_id, factory). For lazy-compilation
148+
// chunks we append a truthy dedup flag so the runtime short-circuits re-execution
149+
// when another lazy blob has already registered this module. HMR patches omit the
150+
// flag so the runtime always re-executes the body to publish new exports.
151+
let mut initializer_args = self.snippet.builder.vec_with_capacity(3);
152+
initializer_args.push(ast::Argument::StringLiteral(self.snippet.builder.alloc_string_literal(
153+
SPAN,
154+
self.snippet.builder.str(&self.module.stable_id),
155+
None,
156+
)));
157+
initializer_args
158+
.push(ast::Argument::from(ast::Expression::FunctionExpression(user_code_wrapper)));
159+
if self.dedup_module_initializer {
160+
initializer_args.push(ast::Argument::from(self.snippet.builder.expression_numeric_literal(
130161
SPAN,
131-
self.snippet.id_ref_expr("__rolldown_runtime__.createCjsInitializer", SPAN),
132-
NONE,
133-
self
134-
.snippet
135-
.builder
136-
.vec1(ast::Argument::from(ast::Expression::FunctionExpression(user_code_wrapper))),
137-
false,
138-
)
162+
1.0,
163+
None,
164+
ast::NumberBase::Decimal,
165+
)));
166+
}
167+
168+
let initializer_callee = if self.module.exports_kind.is_commonjs() {
169+
// __rolldown__runtime.createCjsInitializer(stable_id, (function (exports, module) { [user code] })[, 1])
170+
"__rolldown_runtime__.createCjsInitializer"
139171
} else {
140-
// __rolldown__runtime.createEsmInitializer((function () { [user code] }))
141-
self.snippet.builder.alloc_call_expression(
142-
SPAN,
143-
self.snippet.id_ref_expr("__rolldown_runtime__.createEsmInitializer", SPAN),
144-
NONE,
145-
self
146-
.snippet
147-
.builder
148-
.vec1(ast::Argument::from(ast::Expression::FunctionExpression(user_code_wrapper))),
149-
false,
150-
)
172+
// __rolldown__runtime.createEsmInitializer(stable_id, (function () { [user code] })[, 1])
173+
"__rolldown_runtime__.createEsmInitializer"
151174
};
175+
let initializer_call = self.snippet.builder.alloc_call_expression(
176+
SPAN,
177+
self.snippet.id_ref_expr(initializer_callee, SPAN),
178+
NONE,
179+
initializer_args,
180+
false,
181+
);
152182

153183
// var init_foo = __rolldown__runtime.createEsmInitializer((function () { [user code] }))
154184
let var_decl = self.snippet.builder.alloc_variable_declaration(

‎crates/rolldown/src/hmr/utils.rs‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use rolldown_ecmascript::{CJS_MODULE_REF, CJS_ROLLDOWN_MODULE_REF};
88
use crate::{hmr::hmr_ast_finalizer::HmrAstFinalizer, module_finalizers::ScopeHoistingFinalizer};
99

1010
pub static MODULE_EXPORTS_NAME_FOR_ESM: &str = "__rolldown_exports__";
11+
pub static MODULE_ID_PARAM_FOR_HMR: &str = "__rolldown_module_id__";
1112

1213
pub trait HmrAstBuilder<'any, 'ast> {
1314
fn builder(&self) -> &oxc::ast::AstBuilder<'ast>;
@@ -22,6 +23,20 @@ pub trait HmrAstBuilder<'any, 'ast> {
2223

2324
fn cjs_module_name() -> &'static str;
2425

26+
/// How to refer to the current module id at the emission site.
27+
///
28+
/// The HMR/lazy path wraps each module body in `createEsmInitializer(id, function () { … })`,
29+
/// so inside the body the id is available as an identifier (`__rolldown_module_id__`) passed in
30+
/// by the runtime. The main-bundle path has no such wrapper, so it still needs to emit the
31+
/// stable id as a string literal.
32+
fn module_id_argument(&self) -> ast::Argument<'ast> {
33+
ast::Argument::StringLiteral(self.builder().alloc_string_literal(
34+
SPAN,
35+
self.builder().str(&self.module().stable_id),
36+
None,
37+
))
38+
}
39+
2540
/// `__rolldown_runtime__.registerModule(moduleId, module)`
2641
fn create_register_module_stmt(&self) -> ast::Statement<'ast> {
2742
let module_exports = match self.module().exports_kind {
@@ -63,15 +78,9 @@ pub trait HmrAstBuilder<'any, 'ast> {
6378
};
6479

6580
// ...(moduleId, module)
66-
// Use stable module ID for consistent lookup in runtime
67-
let arguments = self.builder().vec_from_array([
68-
ast::Argument::StringLiteral(self.builder().alloc_string_literal(
69-
SPAN,
70-
self.builder().str(&self.module().stable_id),
71-
None,
72-
)),
73-
module_exports,
74-
]);
81+
// moduleId is either `__rolldown_module_id__` (HMR/lazy path) or the stable-id
82+
// string literal (main-bundle path).
83+
let arguments = self.builder().vec_from_array([self.module_id_argument(), module_exports]);
7584

7685
// __rolldown_runtime__.registerModule(moduleId, module)
7786
let register_call = self.builder().alloc_call_expression(
@@ -119,13 +128,7 @@ pub trait HmrAstBuilder<'any, 'ast> {
119128
),
120129
),
121130
NONE,
122-
self.builder().vec1(ast::Argument::StringLiteral(
123-
self.builder().alloc_string_literal(
124-
SPAN,
125-
self.builder().str(&self.module().stable_id),
126-
None,
127-
),
128-
)),
131+
self.builder().vec1(self.module_id_argument()),
129132
false,
130133
),
131134
)),
@@ -158,6 +161,16 @@ impl<'any, 'ast> HmrAstBuilder<'any, 'ast> for HmrAstFinalizer<'any, 'ast> {
158161
fn cjs_module_name() -> &'static str {
159162
CJS_ROLLDOWN_MODULE_REF
160163
}
164+
165+
/// HMR/lazy path: each module body is wrapped in
166+
/// `createEsmInitializer(id, function (__rolldown_module_id__) { … })`
167+
/// (or `createCjsInitializer(id, function (exports, module, __rolldown_module_id__) { … })`),
168+
/// so the id is in lexical scope as a parameter.
169+
fn module_id_argument(&self) -> ast::Argument<'ast> {
170+
ast::Argument::Identifier(
171+
self.builder().alloc_identifier_reference(SPAN, MODULE_ID_PARAM_FOR_HMR),
172+
)
173+
}
161174
}
162175

163176
impl<'any, 'ast> HmrAstBuilder<'any, 'ast> for ScopeHoistingFinalizer<'any, 'ast> {

‎crates/rolldown/tests/rolldown/topics/hmr/change_accept/artifacts.snap‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ main_hot.accept("parent.js", () => {});
5757
```js
5858
//#region parent.js
5959
import * as import_node_assert_00 from "node:assert";
60-
var init_parent_0 = __rolldown_runtime__.createEsmInitializer((function() {
60+
var init_parent_0 = __rolldown_runtime__.createEsmInitializer("parent.js", (function(__rolldown_module_id__) {
6161
try {
6262
var __rolldown_exports__ = __rolldown_runtime__.__exportAll({});
63-
__rolldown_runtime__.registerModule("parent.js", { exports: __rolldown_exports__ });
64-
const hot_parent = __rolldown_runtime__.createModuleHotContext("parent.js");
63+
__rolldown_runtime__.registerModule(__rolldown_module_id__, { exports: __rolldown_exports__ });
64+
const hot_parent = __rolldown_runtime__.createModuleHotContext(__rolldown_module_id__);
6565
var import_child_01 = __rolldown_runtime__.loadExports("child.js");
6666
hot_parent.accept("child.js", () => {
6767
globalThis.newAcceptWasCalled = true;
@@ -92,11 +92,11 @@ __rolldown_runtime__.applyUpdates([['main.js', 'parent.js']]);
9292

9393
```js
9494
//#region child.js
95-
var init_child_0 = __rolldown_runtime__.createEsmInitializer((function() {
95+
var init_child_0 = __rolldown_runtime__.createEsmInitializer("child.js", (function(__rolldown_module_id__) {
9696
try {
9797
var __rolldown_exports__ = __rolldown_runtime__.__exportAll({ foo: () => foo });
98-
__rolldown_runtime__.registerModule("child.js", { exports: __rolldown_exports__ });
99-
const hot_child = __rolldown_runtime__.createModuleHotContext("child.js");
98+
__rolldown_runtime__.registerModule(__rolldown_module_id__, { exports: __rolldown_exports__ });
99+
const hot_child = __rolldown_runtime__.createModuleHotContext(__rolldown_module_id__);
100100
const foo = 1;
101101
} finally {}
102102
}));

‎crates/rolldown/tests/rolldown/topics/hmr/cjs_exports_lhs_rewrite/artifacts.snap‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@ __rolldown_runtime__.registerModule("main.js", { exports: main_exports });
3434

3535
```js
3636
//#region a.js
37-
var require_a_0 = __rolldown_runtime__.createCjsInitializer((function(__rolldown_exports__, __rolldown_module__) {
37+
var require_a_0 = __rolldown_runtime__.createCjsInitializer("a.js", (function(__rolldown_exports__, __rolldown_module__, __rolldown_module_id__) {
3838
try {
39-
__rolldown_runtime__.registerModule("a.js", __rolldown_module__);
40-
const hot_a = __rolldown_runtime__.createModuleHotContext("a.js");
39+
__rolldown_runtime__.registerModule(__rolldown_module_id__, __rolldown_module__);
40+
const hot_a = __rolldown_runtime__.createModuleHotContext(__rolldown_module_id__);
4141
__rolldown_exports__ = __rolldown_module__.exports = {};
4242
__rolldown_exports__.value = "v2";
4343
__rolldown_module__.exports.other = "o2";

0 commit comments

Comments
 (0)