diff --git a/crates/perry/src/commands/compile/cjs_wrap/mod.rs b/crates/perry/src/commands/compile/cjs_wrap/mod.rs index f1129ef466..f8be17bb8e 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/mod.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/mod.rs @@ -226,6 +226,40 @@ module.exports = inner; assert!(wrapped.contains("const _cjs = (function()")); } + #[test] + fn wrap_module_and_exports_are_reassignable_vars() { + // #3527: a CJS body may rebind `module`/`exports` (iconv-lite's + // `for (...) { var module = modules[i]; mergeModules(exports, module); }`). + // The wrapper must expose them as reassignable `var`s — not a `const` + // the body would silently fail to rebind — while reading the real + // exports back from a stable, body-untouchable `__cjs_module`. + let src = "exports.foo = 42;"; + let wrapped = wrap_commonjs(src, &PathBuf::from("/tmp/test.js")); + assert!( + wrapped.contains("const __cjs_module = { exports: {} };"), + "expected stable __cjs_module, got:\n{}", + wrapped + ); + assert!( + wrapped.contains("var module = __cjs_module;"), + "expected reassignable `var module`, got:\n{}", + wrapped + ); + assert!( + wrapped.contains("var exports = __cjs_module.exports;"), + "expected reassignable `var exports`, got:\n{}", + wrapped + ); + assert!( + wrapped.contains("return __cjs_module.exports;"), + "export must be read from the stable ref, got:\n{}", + wrapped + ); + // The body must NOT re-collide with a `const module`/`const exports`. + assert!(!wrapped.contains("const module = ")); + assert!(!wrapped.contains("const exports = ")); + } + #[test] fn wrap_hoists_require_as_import() { // Issue #665 (third pass): when the CJS source has a unique alias diff --git a/crates/perry/src/commands/compile/cjs_wrap/wrap.rs b/crates/perry/src/commands/compile/cjs_wrap/wrap.rs index 6d8c5bc84e..d1c9d78ee4 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/wrap.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/wrap.rs @@ -335,8 +335,18 @@ pub(in crate::commands::compile) fn wrap_commonjs(source: &str, source_path: &Pa {import_aliases} {hoisted_class_block} const _cjs = (function() {{ - const module = {{ exports: {{}} }}; - const exports = module.exports; + // #3527: `module`/`exports` are reassignable `var`s (mirroring Node, where + // they are wrapper-function parameters), so CJS bodies that do + // `var module = X` / `module = X` / `exports = X` — e.g. iconv-lite's + // `for (...) {{ var module = modules[i]; mergeModules(exports, module); }}` + // — rebind the local instead of colliding with a `const`. The stable + // `__cjs_module` is what the module actually exports, read back at the end; + // a body reassigning its local `module` can't clobber it (Node holds the + // real module ref the same way), so named/default-export resolution stays + // correct regardless of what the body does to its `module` local. + const __cjs_module = {{ exports: {{}} }}; + var module = __cjs_module; + var exports = __cjs_module.exports; function require(specifier) {{ {require_cases} throw new Error('require() is not supported: ' + specifier); @@ -344,7 +354,7 @@ const _cjs = (function() {{ {body_for_iife} - return module.exports; + return __cjs_module.exports; }})(); {default_export_decl}