Default interop missing when importing a CommonJS module

packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid-dynamic/a.js

@@ -0,0 +1,5 @@
+import b from './b';
+export const foo = b.foo; // <-- missing default interop
+export const bar = (() => require('./b').foo)();
+
+output = foo + bar;

packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid-dynamic/b.js

@@ -0,0 +1 @@
+module.exports = {foo: 1};

packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid/a.js

@@ -0,0 +1,5 @@
+import b from './b';
+const foo = b.foo;
+const bar = require('./b').foo;
+
+output = foo + bar;

packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid/b.js

@@ -0,0 +1 @@
+module.exports = {foo: 1};

packages/core/integration-tests/test/scope-hoisting.js

@@ -4359,6 +4359,30 @@ describe('scope hoisting', function () {
       assert.deepEqual(output.default, obj);
     });
 
+    it('should add a default interop for a CJS module used in a hybrid module', async function () {
+      let b = await bundle(
+        path.join(
+          __dirname,
+          '/integration/scope-hoisting/commonjs/interop-commonjs-hybrid/a.js',
+        ),
+      );
+
+      let output = await run(b);
+      assert.equal(output, 2);
+    });
+
+    it('should add a default interop for a CJS module used non-statically in a hybrid module', async function () {
+      let b = await bundle(
+        path.join(
+          __dirname,
+          '/integration/scope-hoisting/commonjs/interop-commonjs-hybrid-dynamic/a.js',
+        ),
+      );
+
+      let output = await run(b);
+      assert.equal(output, 2);
+    });
+
     it('should not insert default interop for wrapped CJS modules', async function () {
       let b = await bundle(
         path.join(

packages/transformers/js/core/src/hoist.rs

@@ -51,6 +51,7 @@ struct ImportedSymbol {
   local: JsWord,
   imported: JsWord,
   loc: SourceLocation,
+  kind: ImportKind,
 }
 
 struct Hoist<'a> {
@@ -208,6 +209,7 @@ impl<'a> Fold for Hoist<'a> {
                         local: exported,
                         imported: match_export_name(&named.orig).0,
                         loc: SourceLocation::from(&self.collect.source_map, named.span),
+                        kind: ImportKind::Import,
                       });
                     }
                     ExportSpecifier::Default(default) => {
@@ -216,6 +218,7 @@ impl<'a> Fold for Hoist<'a> {
                         local: default.exported.sym,
                         imported: js_word!("default"),
                         loc: SourceLocation::from(&self.collect.source_map, default.exported.span),
+                        kind: ImportKind::Import,
                       });
                     }
                     ExportSpecifier::Namespace(namespace) => {
@@ -224,6 +227,7 @@ impl<'a> Fold for Hoist<'a> {
                         local: match_export_name(&namespace.name).0,
                         imported: "*".into(),
                         loc: SourceLocation::from(&self.collect.source_map, namespace.span),
+                        kind: ImportKind::Import,
                       });
                     }
                   }
@@ -237,14 +241,18 @@ impl<'a> Fold for Hoist<'a> {
                       None => match_export_name(&named.orig).0,
                     };
                     if let Some(Import {
-                      source, specifier, ..
+                      source,
+                      specifier,
+                      kind,
+                      ..
                     }) = self.collect.imports.get(&id)
                     {
                       self.re_exports.push(ImportedSymbol {
                         source: source.clone(),
                         local: exported,
                         imported: specifier.clone(),
                         loc: SourceLocation::from(&self.collect.source_map, named.span),
+                        kind: *kind,
                       });
                     } else {
                       // A variable will appear only once in the `exports` mapping but
@@ -285,6 +293,7 @@ impl<'a> Fold for Hoist<'a> {
                 local: "*".into(),
                 imported: "*".into(),
                 loc: SourceLocation::from(&self.collect.source_map, export.span),
+                kind: ImportKind::Import,
               });
             }
             ModuleDecl::ExportDefaultExpr(export) => {
@@ -562,13 +571,15 @@ impl<'a> Fold for Hoist<'a> {
                     local: name,
                     imported: key.clone(),
                     loc: SourceLocation::from(&self.collect.source_map, member.span),
+                    kind: *kind,
                   });
                 } else {
                   return Expr::Ident(self.get_import_ident(
                     member.span,
                     source,
                     &key,
                     SourceLocation::from(&self.collect.source_map, member.span),
+                    *kind,
                   ));
                 }
               }
@@ -596,6 +607,7 @@ impl<'a> Fold for Hoist<'a> {
                 &source,
                 &key,
                 SourceLocation::from(&self.collect.source_map, member.span),
+                ImportKind::Require,
               ));
             }
           }
@@ -639,6 +651,7 @@ impl<'a> Fold for Hoist<'a> {
             &source,
             &("*".into()),
             SourceLocation::from(&self.collect.source_map, call.span),
+            ImportKind::Require,
           ));
         }
 
@@ -652,6 +665,7 @@ impl<'a> Fold for Hoist<'a> {
               local: name.clone(),
               imported: "*".into(),
               loc: SourceLocation::from(&self.collect.source_map, call.span),
+              kind: ImportKind::DynamicImport,
             });
           }
           return Expr::Ident(Ident::new(name, call.span));
@@ -733,6 +747,7 @@ impl<'a> Fold for Hoist<'a> {
               local: name,
               imported: specifier.clone(),
               loc: loc.clone(),
+              kind: *kind,
             });
           } else if self.collect.non_static_access.contains_key(&id!(node)) {
             let name: JsWord =
@@ -742,6 +757,7 @@ impl<'a> Fold for Hoist<'a> {
               local: name,
               imported: "*".into(),
               loc: loc.clone(),
+              kind: *kind,
             });
           }
         } else {
@@ -754,7 +770,7 @@ impl<'a> Fold for Hoist<'a> {
             return self.get_require_ident(&node.sym);
           }
 
-          return self.get_import_ident(node.span, source, specifier, loc.clone());
+          return self.get_import_ident(node.span, source, specifier, loc.clone(), *kind);
         }
       }
     }
@@ -960,13 +976,15 @@ impl<'a> Hoist<'a> {
     source: &JsWord,
     imported: &JsWord,
     loc: SourceLocation,
+    kind: ImportKind,
   ) -> Ident {
     let new_name = self.get_import_name(source, imported);
     self.imported_symbols.push(ImportedSymbol {
       source: source.clone(),
       local: new_name.clone(),
       imported: imported.clone(),
       loc,
+      kind,
     });
     Ident::new(new_name, span)
   }
@@ -1007,13 +1025,17 @@ impl<'a> Hoist<'a> {
       .get_non_const_binding_idents(&v.name, &mut non_const_bindings);
 
     for ident in non_const_bindings {
-      if let Some(Import { specifier, .. }) = self.collect.imports.get(&id!(ident)) {
+      if let Some(Import {
+        specifier, kind, ..
+      }) = self.collect.imports.get(&id!(ident))
+      {
         let require_id = self.get_require_ident(&ident.sym);
         let import_id = self.get_import_ident(
           v.span,
           source,
           specifier,
           SourceLocation::from(&self.collect.source_map, v.span),
+          *kind,
         );
         self
           .module_items
@@ -1047,7 +1069,7 @@ macro_rules! collect_visit_fn {
   };
 }
 
-#[derive(Debug, PartialEq, Clone, Copy, Serialize)]
+#[derive(Debug, Deserialize, PartialEq, Clone, Copy, Serialize)]
 pub enum ImportKind {
   Require,
   Import,

packages/transformers/js/src/JSTransformer.js

@@ -706,51 +706,69 @@ export default (new Transformer({
         asset.symbols.set(exported, local, convertLoc(loc));
       }
 
-      let deps = new Map(
-        asset
-          .getDependencies()
-          .map(dep => [dep.meta.placeholder ?? dep.specifier, dep]),
-      );
-      for (let dep of deps.values()) {
-        dep.symbols.ensure();
+      let deps = new Map();
+      for (let dep of asset.getDependencies()) {
+        if (!deps.has(dep.meta.placeholder ?? dep.specifier)) {
+          deps.set(dep.meta.placeholder ?? dep.specifier, []);
+        }
+        deps.get(dep.meta.placeholder ?? dep.specifier)?.push(dep);
+      }
+      for (let depArr of deps.values()) {
+        for (let dep of depArr) {
+          dep.symbols.ensure();
+        }
       }
 
       for (let {
         source,
         local,
         imported,
         loc,
+        kind,
       } of hoist_result.imported_symbols) {
-        let dep = deps.get(source);
-        if (!dep) continue;
-        dep.symbols.set(imported, local, convertLoc(loc));
+        for (let dep of nullthrows(deps.get(source))) {
+          if (
+            !dep ||
+            ((dep.meta.kind === 'Require' ||
+              dep.meta.kind === 'Import' ||
+              dep.meta.kind === 'DynamicImport') &&
+              dep.meta.kind !== kind)
+          ) {
+            continue;
+          }
+          dep.symbols.set(imported, local, convertLoc(loc));
+        }
       }
-
       for (let {source, local, imported, loc} of hoist_result.re_exports) {
-        let dep = deps.get(source);
-        if (!dep) continue;
+        for (let dep of nullthrows(deps.get(source))) {
+          if (!dep) continue;
 
-        if (local === '*' && imported === '*') {
-          dep.symbols.set('*', '*', convertLoc(loc), true);
-        } else {
-          let reExportName =
-            dep.symbols.get(imported)?.local ??
-            `$${asset.id}$re_export$${local}`;
-          asset.symbols.set(local, reExportName);
-          dep.symbols.set(imported, reExportName, convertLoc(loc), true);
+          if (local === '*' && imported === '*') {
+            dep.symbols.set('*', '*', convertLoc(loc), true);
+          } else {
+            let reExportName =
+              dep.symbols.get(imported)?.local ??
+              `$${asset.id}$re_export$${local}`;
+            asset.symbols.set(local, reExportName);
+            dep.symbols.set(imported, reExportName, convertLoc(loc), true);
+          }
         }
       }
 
       for (let specifier of hoist_result.wrapped_requires) {
-        let dep = deps.get(specifier);
-        if (!dep) continue;
-        dep.meta.shouldWrap = true;
+        for (let dep of nullthrows(deps.get(specifier))) {
+          if (!dep) continue;
+          dep.meta.shouldWrap = true;
+        }
       }
 
       for (let name in hoist_result.dynamic_imports) {
-        let dep = deps.get(hoist_result.dynamic_imports[name]);
-        if (!dep) continue;
-        dep.meta.promiseSymbol = name;
+        for (let dep of nullthrows(
+          deps.get(hoist_result.dynamic_imports[name]),
+        )) {
+          if (!dep) continue;
+          dep.meta.promiseSymbol = name;
+        }
       }
 
       if (hoist_result.self_references.length > 0) {
@@ -796,43 +814,52 @@ export default (new Transformer({
       asset.meta.shouldWrap = hoist_result.should_wrap;
     } else {
       if (symbol_result) {
-        let deps = new Map(
-          asset
-            .getDependencies()
-            .map(dep => [dep.meta.placeholder ?? dep.specifier, dep]),
-        );
+        let deps = new Map();
+        for (let dep of asset.getDependencies()) {
+          if (!deps.has(dep.meta.placeholder ?? dep.specifier)) {
+            deps.set(dep.meta.placeholder ?? dep.specifier, []);
+          }
+          deps.get(dep.meta.placeholder ?? dep.specifier)?.push(dep);
+        }
         asset.symbols.ensure();
 
         for (let {exported, local, loc, source} of symbol_result.exports) {
-          let dep = source ? deps.get(source) : undefined;
-          asset.symbols.set(
-            exported,
-            `${dep?.id ?? ''}$${local}`,
-            convertLoc(loc),
-          );
-          if (dep != null) {
-            dep.symbols.ensure();
-            dep.symbols.set(
-              local,
-              `${dep?.id ?? ''}$${local}`,
-              convertLoc(loc),
-              true,
-            );
+          if (!deps.get(source)) {
+            asset.symbols.set(exported, `${local}`, convertLoc(loc));
+          } else {
+            for (let dep of nullthrows(deps.get(source))) {
+              asset.symbols.set(
+                exported,
+                `${dep?.id ?? ''}$${local}`,
+                convertLoc(loc),
+              );
+              if (dep != null) {
+                dep.symbols.ensure();
+                dep.symbols.set(
+                  local,
+                  `${dep?.id ?? ''}$${local}`,
+                  convertLoc(loc),
+                  true,
+                );
+              }
+            }
           }
         }
 
         for (let {source, local, imported, loc} of symbol_result.imports) {
-          let dep = deps.get(source);
-          if (!dep) continue;
-          dep.symbols.ensure();
-          dep.symbols.set(imported, local, convertLoc(loc));
+          for (let dep of nullthrows(deps.get(source))) {
+            if (!dep) continue;
+            dep.symbols.ensure();
+            dep.symbols.set(imported, local, convertLoc(loc));
+          }
         }
 
         for (let {source, loc} of symbol_result.exports_all) {
-          let dep = deps.get(source);
-          if (!dep) continue;
-          dep.symbols.ensure();
-          dep.symbols.set('*', '*', convertLoc(loc), true);
+          for (let dep of nullthrows(deps.get(source))) {
+            if (!dep) continue;
+            dep.symbols.ensure();
+            dep.symbols.set('*', '*', convertLoc(loc), true);
+          }
         }
       }