diff --git a/src/ast/e.rs b/src/ast/e.rs index 32c61c186f30..58c48ab87170 100644 --- a/src/ast/e.rs +++ b/src/ast/e.rs @@ -2401,7 +2401,7 @@ pub type UnwrappedRequireIndexOptional = pub struct RequireString { pub import_record_index: u32, - /// Set when `unwrap_commonjs_to_esm` turned this `require()` into an import. + /// Set when unwrapping `const x = require()` into an import; consumed by `visit_decls`. pub unwrapped_id: UnwrappedRequireIndexOptional, } impl Default for RequireString { diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index bc47073ee17d..cebaff68072f 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -2309,25 +2309,16 @@ impl<'a> LinkerContext<'a> { pub(crate) fn require_or_import_meta_for_source( &mut self, source_index: crate::IndexInt, - was_unwrapped_require: bool, ) -> js_printer::RequireOrImportMeta { let flags = self.graph.meta.items_flags()[source_index as usize]; js_printer::RequireOrImportMeta { - exports_ref: if flags.wrap == WrapKind::Esm - || (was_unwrapped_require - && self.graph.ast.items_flags()[source_index as usize] - .contains(AstFlags::FORCE_CJS_TO_ESM)) - { + exports_ref: if flags.wrap == WrapKind::Esm { self.graph.ast.items_exports_ref()[source_index as usize] } else { Ref::NONE }, is_wrapper_async: flags.is_async_or_has_async_dependency, wrapper_ref: self.graph.ast.items_wrapper_ref()[source_index as usize], - - was_unwrapped_require: was_unwrapped_require - && self.graph.ast.items_flags()[source_index as usize] - .contains(AstFlags::FORCE_CJS_TO_ESM), } } @@ -2562,12 +2553,8 @@ impl<'a> LinkerContext<'a> { /// can call back into `LinkerContext::require_or_import_meta_for_source`. impl<'a> js_printer::RequireOrImportMetaSource for LinkerContext<'a> { #[inline] - fn require_or_import_meta_for_source( - &mut self, - id: u32, - was_unwrapped_require: bool, - ) -> js_printer::RequireOrImportMeta { - LinkerContext::require_or_import_meta_for_source(self, id, was_unwrapped_require) + fn require_or_import_meta_for_source(&mut self, id: u32) -> js_printer::RequireOrImportMeta { + LinkerContext::require_or_import_meta_for_source(self, id) } } diff --git a/src/js_parser/parser.rs b/src/js_parser/parser.rs index 8be190092052..fc931b27a67a 100644 --- a/src/js_parser/parser.rs +++ b/src/js_parser/parser.rs @@ -1011,8 +1011,7 @@ pub struct ExprIn { /// tests. pub(crate) assign_target: js_ast::AssignTarget, - /// Currently this is only used when unwrapping a call to `require()` - /// with `__toESM()`. + /// Identifier-binding initializer; only used to unwrap `const x = require()` into an import. pub(crate) is_immediately_assigned_to_decl: bool, pub(crate) property_access_for_method_call_maybe_should_replace_with_undefined: bool, diff --git a/src/js_parser/visit/mod.rs b/src/js_parser/visit/mod.rs index 685d5e1ef7a5..546da723dad2 100644 --- a/src/js_parser/visit/mod.rs +++ b/src/js_parser/visit/mod.rs @@ -320,7 +320,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O self.visit_expr_in_out( &mut val, ExprIn { - is_immediately_assigned_to_decl: true, + // Only the identifier unwrap below consumes the marker this requests. + is_immediately_assigned_to_decl: matches!( + decl.binding.data, + BData::BIdentifier(_) + ), ..Default::default() }, ); diff --git a/src/js_printer/lib.rs b/src/js_printer/lib.rs index a465407cf3f4..23f37c821ea6 100644 --- a/src/js_printer/lib.rs +++ b/src/js_printer/lib.rs @@ -1194,11 +1194,7 @@ pub struct Options<'a> { } impl<'a> Options<'a> { - pub(crate) fn require_or_import_meta_for_source( - &self, - id: u32, - was_unwrapped_require: bool, - ) -> RequireOrImportMeta { + pub(crate) fn require_or_import_meta_for_source(&self, id: u32) -> RequireOrImportMeta { if self .require_or_import_meta_for_source_callback .ctx @@ -1206,8 +1202,7 @@ impl<'a> Options<'a> { { return RequireOrImportMeta::default(); } - self.require_or_import_meta_for_source_callback - .call(id, was_unwrapped_require) + self.require_or_import_meta_for_source_callback.call(id) } } @@ -1281,7 +1276,6 @@ pub struct RequireOrImportMeta { pub wrapper_ref: Ref, pub exports_ref: Ref, pub is_wrapper_async: bool, - pub was_unwrapped_require: bool, } // Clone/Copy: bitwise OK — `ctx` is a non-owning opaque backref the caller @@ -1289,12 +1283,12 @@ pub struct RequireOrImportMeta { #[derive(Clone, Copy)] pub struct RequireOrImportMetaCallback { pub(crate) ctx: Option>, - pub(crate) callback: fn(*mut (), u32, bool) -> RequireOrImportMeta, + pub(crate) callback: fn(*mut (), u32) -> RequireOrImportMeta, } impl Default for RequireOrImportMetaCallback { fn default() -> Self { - fn noop(_: *mut (), _: u32, _: bool) -> RequireOrImportMeta { + fn noop(_: *mut (), _: u32) -> RequireOrImportMeta { RequireOrImportMeta::default() } Self { @@ -1307,28 +1301,20 @@ impl Default for RequireOrImportMetaCallback { /// PORTING.md §Dispatch — manual vtable. The erased thunk is monomorphized /// over `T: RequireOrImportMetaSource`, so `callback` stays a captureless `fn`. pub trait RequireOrImportMetaSource { - fn require_or_import_meta_for_source( - &mut self, - id: u32, - was_unwrapped_require: bool, - ) -> RequireOrImportMeta; + fn require_or_import_meta_for_source(&mut self, id: u32) -> RequireOrImportMeta; } impl RequireOrImportMetaCallback { - pub(crate) fn call(&self, id: u32, was_unwrapped_require: bool) -> RequireOrImportMeta { - (self.callback)(self.ctx.unwrap().as_ptr(), id, was_unwrapped_require) + pub(crate) fn call(&self, id: u32) -> RequireOrImportMeta { + (self.callback)(self.ctx.unwrap().as_ptr(), id) } pub fn init(ctx: &mut T) -> Self { - fn thunk( - p: *mut (), - id: u32, - was_unwrapped_require: bool, - ) -> RequireOrImportMeta { + fn thunk(p: *mut (), id: u32) -> RequireOrImportMeta { // SAFETY: `p` was constructed from `&mut T` in `init` below; caller guarantees // `ctx` outlives this `RequireOrImportMetaCallback`, so the cast-back // deref is valid and exclusive. - unsafe { (*p.cast::()).require_or_import_meta_for_source(id, was_unwrapped_require) } + unsafe { (*p.cast::()).require_or_import_meta_for_source(id) } } Self { // Type-erased to `*mut ()` and cast back to `*mut T` inside the thunk before dereference. @@ -1822,7 +1808,6 @@ pub(crate) mod __gated_printer { match statement { None => self.print_require_or_import_expr( import.import_record_index, - false, &[], Expr::EMPTY, Level::Lowest, @@ -1843,7 +1828,6 @@ pub(crate) mod __gated_printer { self.print_equals(); self.print_require_or_import_expr( import.import_record_index, - false, &[], Expr::EMPTY, Level::Lowest, @@ -1893,7 +1877,6 @@ pub(crate) mod __gated_printer { match statement { None => self.print_require_or_import_expr( import.import_record_index, - false, &[], Expr::EMPTY, Level::Lowest, @@ -2455,7 +2438,6 @@ pub(crate) mod __gated_printer { pub(crate) fn print_require_or_import_expr( &mut self, import_record_index: u32, - was_unwrapped_require: bool, leading_interior_comments: &[G::Comment], import_options: Expr, level_: Level, @@ -2504,10 +2486,9 @@ pub(crate) mod __gated_printer { } if record.source_index.is_valid() { - let mut meta = self.options.require_or_import_meta_for_source( - record.source_index.get(), - was_unwrapped_require, - ); + let mut meta = self + .options + .require_or_import_meta_for_source(record.source_index.get()); // Don't need the namespace object if the result is unused anyway if flags.contains(ExprFlag::ExprResultIsUnused) { @@ -2534,7 +2515,6 @@ pub(crate) mod __gated_printer { // Internal "require()" or "import()" let has_side_effects = meta.wrapper_ref.is_valid() || meta.exports_ref.is_valid() - || meta.was_unwrapped_require || self.options.input_files_for_dev_server.is_some(); if record.kind == ImportKind::Dynamic { self.print_space_before_identifier(); @@ -2568,7 +2548,7 @@ pub(crate) mod __gated_printer { let path = &input_files[record.source_index.get() as usize].path; self.print_string_literal_utf8(path.pretty, false); self.print(b")"); - } else if !meta.was_unwrapped_require { + } else { // Call the wrapper if meta.wrapper_ref.is_valid() { self.print_space_before_identifier(); @@ -2596,10 +2576,6 @@ pub(crate) mod __gated_printer { self.print(b")"); } } - } else { - if !meta.exports_ref.is_empty() { - self.print_symbol(meta.exports_ref); - } } if wrap_with_to_esm { @@ -3313,9 +3289,10 @@ pub(crate) mod __gated_printer { } } ExprData::ERequireString(e) => { + // An unwrapped require() is consumed by the parser's visit_decls, never printed. + debug_assert!(e.unwrapped_id.is_none()); self.print_require_or_import_expr( e.import_record_index, - e.unwrapped_id.is_some(), &[], Expr::EMPTY, level, @@ -3383,7 +3360,6 @@ pub(crate) mod __gated_printer { } else { self.print_require_or_import_expr( e.import_record_index, - false, &[], // e.leading_interior_comments, e.options, level, diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index 2ab2c8769c22..b1254fbdb933 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -284,15 +284,70 @@ describe("bundler", () => { stdout: "react\nreact\nreact\nreact\nundefined\nreact\nreact\nreact\nreact\nreact\nreact\n1 react\nreact\nreact", }, }); - // A require() of an unwrapped package that initializes a destructuring - // declaration is kept as a require expression that remembers it was - // unwrapped, and prints as the namespace object. One inside try/catch is + // The required packages assign module.exports, so they stay wrapped in __commonJS. + // A destructuring declaration has to read from the import the require() became, + // not from the wrapped module's own `exports` binding. + itBundled("cjs2esm/UnwrappedModuleRequireDestructured", { + files: { + "/entry.js": /* js */ ` + const { react } = require("react"); + console.log(react); + + let { react: renamed, missing = "fallback" } = require("react"); + console.log(renamed, missing); + + var { react: { length } } = require("react"); + console.log(length); + + const before = require("react"), + { react: between } = require("react"), + after = require("react"); + console.log(before.react, between, after.react); + + function inFunction() { + const { react } = require("react"); + return react; + } + console.log(inFunction()); + + const [first, second] = require("scheduler"); + console.log(first, second); + `, + ...fakeReactNodeModules, + "/node_modules/scheduler/index.js": /* js */ ` + module.exports = ["first", "second"]; + `, + "/node_modules/scheduler/package.json": /* json */ ` + { + "name": "scheduler", + "version": "1.0.0", + "main": "index.js" + } + `, + }, + onAfterBundle: api => { + const code = api.readFile("out.js"); + expect(code).toContain("var require_react = __commonJS("); + expect(code).toContain("var require_scheduler = __commonJS("); + expect(code).toMatch(/\{ react: between \} = \w+;/); + expect(code).toMatch(/\[first, second\] = \w+;/); + }, + run: { + stdout: "react\nreact fallback\n5\nreact react react\nreact\nfirst second", + }, + }); + // Against a package that does convert to ESM, a destructuring declaration + // reads from the generated namespace object. A require() inside try/catch is // never unwrapped and prints as an ordinary require. itBundled("cjs2esm/UnwrappedModuleRequireDestructuredAndInTry", { files: { "/entry.js": /* js */ ` - const { react: named } = require("react"); - console.log(named); + const { react: named, version = "none" } = require("react"); + console.log(named, version); + + const whole = require("react"), + { react: again } = require("react"); + console.log(whole.react, again); let inTry = "missing"; try { @@ -313,11 +368,12 @@ describe("bundler", () => { }, onAfterBundle: api => { const code = api.readFile("out.js"); - expect(code).toMatch(/\{ react: named \} = \(?exports_react\)?;/); + expect(code).toMatch(/\{ react: named, version = "none" \} = \(?exports_react\)?;/); + expect(code).toMatch(/\{ react: again \} = \(?exports_react\)?;/); expect(code).toContain("__toCommonJS(exports_react)).react"); }, run: { - stdout: "react\nreact", + stdout: "react none\nreact react\nreact", }, }); itBundled("cjs2esm/ReactSpecificUnwrapping", {