From 6f3403b4af1960dae4ce7f3cc7f2c43fbc553b2e Mon Sep 17 00:00:00 2001
From: thebriando <bdo@atlassian.com>
Date: Thu, 21 Apr 2022 14:59:13 -0700
Subject: [PATCH 01/16] Add test case for missing default interop for commonjs
 import

---
 .../scope-hoisting/commonjs/interop-commonjs/a.js    |  5 +++++
 .../scope-hoisting/commonjs/interop-commonjs/b.js    |  1 +
 .../core/integration-tests/test/scope-hoisting.js    | 12 ++++++++++++
 3 files changed, 18 insertions(+)
 create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/a.js
 create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/b.js

diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/a.js b/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/a.js
new file mode 100644
index 00000000000..7ab86bc3b87
--- /dev/null
+++ b/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/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;
diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/b.js b/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/b.js
new file mode 100644
index 00000000000..f64238e365c
--- /dev/null
+++ b/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/b.js
@@ -0,0 +1 @@
+module.exports = {foo: 1};
diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js
index 63ddafd760a..f30cbfd9293 100644
--- a/packages/core/integration-tests/test/scope-hoisting.js
+++ b/packages/core/integration-tests/test/scope-hoisting.js
@@ -4359,6 +4359,18 @@ describe('scope hoisting', function () {
       assert.deepEqual(output.default, obj);
     });
 
+    it.only('should add a default interop for a CJS module', async function () {
+      let b = await bundle(
+        path.join(
+          __dirname,
+          '/integration/scope-hoisting/commonjs/interop-commonjs/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(

From 86ca04e666b12660f84e5a472a4539e9a3e5b776 Mon Sep 17 00:00:00 2001
From: thebriando <bdo@atlassian.com>
Date: Thu, 21 Apr 2022 15:00:29 -0700
Subject: [PATCH 02/16] Add interopDefault helper if the dep kind is require

---
 packages/core/integration-tests/test/scope-hoisting.js | 2 +-
 packages/packagers/js/src/ScopeHoistingPackager.js     | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js
index f30cbfd9293..2349289ee43 100644
--- a/packages/core/integration-tests/test/scope-hoisting.js
+++ b/packages/core/integration-tests/test/scope-hoisting.js
@@ -4359,7 +4359,7 @@ describe('scope hoisting', function () {
       assert.deepEqual(output.default, obj);
     });
 
-    it.only('should add a default interop for a CJS module', async function () {
+    it('should add a default interop for a CJS module', async function () {
       let b = await bundle(
         path.join(
           __dirname,
diff --git a/packages/packagers/js/src/ScopeHoistingPackager.js b/packages/packagers/js/src/ScopeHoistingPackager.js
index 34425e5a8c7..45b3e11286d 100644
--- a/packages/packagers/js/src/ScopeHoistingPackager.js
+++ b/packages/packagers/js/src/ScopeHoistingPackager.js
@@ -834,7 +834,10 @@ ${code}
       // use a helper to check the __esModule flag at runtime.
       let kind = dep?.meta.kind;
       if (
-        (!dep || kind === 'Import' || kind === 'Export') &&
+        (!dep ||
+          kind === 'Import' ||
+          kind === 'Require' ||
+          kind === 'Export') &&
         exportSymbol === 'default' &&
         resolvedAsset.symbols.hasExportSymbol('*') &&
         this.needsDefaultInterop(resolvedAsset)

From e01a71f8d47c9079d3d13d7272d4711ff08643a0 Mon Sep 17 00:00:00 2001
From: lettertwo <lettertwo@gmail.com>
Date: Thu, 21 Apr 2022 20:06:45 -0400
Subject: [PATCH 03/16] Add test case for 'static' hybrid interop

---
 .../commonjs/interop-commonjs-hybrid/a.js            |  5 +++++
 .../commonjs/interop-commonjs-hybrid/b.js            |  1 +
 .../core/integration-tests/test/scope-hoisting.js    | 12 ++++++++++++
 3 files changed, 18 insertions(+)
 create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid/a.js
 create mode 100644 packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid/b.js

diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid/a.js b/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid/a.js
new file mode 100644
index 00000000000..754877b30f8
--- /dev/null
+++ b/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;
diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid/b.js b/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid/b.js
new file mode 100644
index 00000000000..f64238e365c
--- /dev/null
+++ b/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid/b.js
@@ -0,0 +1 @@
+module.exports = {foo: 1};
diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js
index 2349289ee43..4fc50121680 100644
--- a/packages/core/integration-tests/test/scope-hoisting.js
+++ b/packages/core/integration-tests/test/scope-hoisting.js
@@ -4359,6 +4359,18 @@ 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', async function () {
       let b = await bundle(
         path.join(

From b24c60a1271fd769a37f7211d64145e9035c7d26 Mon Sep 17 00:00:00 2001
From: Brian Do <bdo@atlassian.com>
Date: Thu, 21 Apr 2022 18:17:15 -0700
Subject: [PATCH 04/16] Update test description

Co-authored-by: Eric Eldredge <lettertwo@gmail.com>
---
 packages/core/integration-tests/test/scope-hoisting.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js
index 4fc50121680..df31550ccc8 100644
--- a/packages/core/integration-tests/test/scope-hoisting.js
+++ b/packages/core/integration-tests/test/scope-hoisting.js
@@ -4371,7 +4371,7 @@ describe('scope hoisting', function () {
       assert.equal(output, 2);
     });
 
-    it('should add a default interop for a CJS module', async function () {
+    it('should add a default interop for a CJS module used dynamically in a hybrid module', async function () {
       let b = await bundle(
         path.join(
           __dirname,

From 1234fa03099cd5c1ad6c57b52ff59972fc296170 Mon Sep 17 00:00:00 2001
From: Brian Do <bdo@atlassian.com>
Date: Thu, 21 Apr 2022 18:17:38 -0700
Subject: [PATCH 05/16] Update test dirname

Co-authored-by: Eric Eldredge <lettertwo@gmail.com>
---
 packages/core/integration-tests/test/scope-hoisting.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js
index df31550ccc8..6b850e96e3a 100644
--- a/packages/core/integration-tests/test/scope-hoisting.js
+++ b/packages/core/integration-tests/test/scope-hoisting.js
@@ -4375,7 +4375,7 @@ describe('scope hoisting', function () {
       let b = await bundle(
         path.join(
           __dirname,
-          '/integration/scope-hoisting/commonjs/interop-commonjs/a.js',
+          '/integration/scope-hoisting/commonjs/interop-commonjs-hybrid-dynamic/a.js',
         ),
       );
 

From 7f61d93d3682bdb069df3763f4a4416de339944e Mon Sep 17 00:00:00 2001
From: lettertwo <lettertwo@gmail.com>
Date: Fri, 22 Apr 2022 11:46:32 -0400
Subject: [PATCH 06/16] Fix dirname

---
 .../{interop-commonjs => interop-commonjs-hybrid-dynamic}/a.js    | 0
 .../{interop-commonjs => interop-commonjs-hybrid-dynamic}/b.js    | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 rename packages/core/integration-tests/test/integration/scope-hoisting/commonjs/{interop-commonjs => interop-commonjs-hybrid-dynamic}/a.js (100%)
 rename packages/core/integration-tests/test/integration/scope-hoisting/commonjs/{interop-commonjs => interop-commonjs-hybrid-dynamic}/b.js (100%)

diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/a.js b/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid-dynamic/a.js
similarity index 100%
rename from packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/a.js
rename to packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid-dynamic/a.js
diff --git a/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/b.js b/packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid-dynamic/b.js
similarity index 100%
rename from packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs/b.js
rename to packages/core/integration-tests/test/integration/scope-hoisting/commonjs/interop-commonjs-hybrid-dynamic/b.js

From 6ab2b2ed5c9e5e224a203aa8073575498d3bd638 Mon Sep 17 00:00:00 2001
From: Brian Do <bdo@atlassian.com>
Date: Mon, 25 Apr 2022 09:39:51 -0700
Subject: [PATCH 07/16] Update
 packages/core/integration-tests/test/scope-hoisting.js

Co-authored-by: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com>
---
 packages/core/integration-tests/test/scope-hoisting.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js
index 6b850e96e3a..d39a7cdbe5f 100644
--- a/packages/core/integration-tests/test/scope-hoisting.js
+++ b/packages/core/integration-tests/test/scope-hoisting.js
@@ -4371,7 +4371,7 @@ describe('scope hoisting', function () {
       assert.equal(output, 2);
     });
 
-    it('should add a default interop for a CJS module used dynamically in a hybrid module', async function () {
+    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,

From 7bf63b3a21679fa7ab041c110cd31dcf9b0ebe72 Mon Sep 17 00:00:00 2001
From: thebriando <bdo@atlassian.com>
Date: Mon, 9 May 2022 00:30:45 -0400
Subject: [PATCH 08/16] Store import kinds in imported symbols and only set
 symbols when kinds match while iterating

---
 .../packagers/js/src/ScopeHoistingPackager.js |   5 +-
 packages/transformers/js/core/src/hoist.rs    |  30 +++-
 packages/transformers/js/src/JSTransformer.js | 135 +++++++++++-------
 3 files changed, 108 insertions(+), 62 deletions(-)

diff --git a/packages/packagers/js/src/ScopeHoistingPackager.js b/packages/packagers/js/src/ScopeHoistingPackager.js
index 45b3e11286d..34425e5a8c7 100644
--- a/packages/packagers/js/src/ScopeHoistingPackager.js
+++ b/packages/packagers/js/src/ScopeHoistingPackager.js
@@ -834,10 +834,7 @@ ${code}
       // use a helper to check the __esModule flag at runtime.
       let kind = dep?.meta.kind;
       if (
-        (!dep ||
-          kind === 'Import' ||
-          kind === 'Require' ||
-          kind === 'Export') &&
+        (!dep || kind === 'Import' || kind === 'Export') &&
         exportSymbol === 'default' &&
         resolvedAsset.symbols.hasExportSymbol('*') &&
         this.needsDefaultInterop(resolvedAsset)
diff --git a/packages/transformers/js/core/src/hoist.rs b/packages/transformers/js/core/src/hoist.rs
index 59f7f10101c..86708876cac 100644
--- a/packages/transformers/js/core/src/hoist.rs
+++ b/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,7 +241,10 @@ 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 {
@@ -245,6 +252,7 @@ impl<'a> Fold for Hoist<'a> {
                         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,6 +571,7 @@ 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(
@@ -569,6 +579,7 @@ impl<'a> Fold for Hoist<'a> {
                     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,6 +976,7 @@ 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 {
@@ -967,6 +984,7 @@ impl<'a> Hoist<'a> {
       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,
diff --git a/packages/transformers/js/src/JSTransformer.js b/packages/transformers/js/src/JSTransformer.js
index fdd954d0405..dacc88af57d 100644
--- a/packages/transformers/js/src/JSTransformer.js
+++ b/packages/transformers/js/src/JSTransformer.js
@@ -706,13 +706,17 @@ 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 {
@@ -720,37 +724,51 @@ export default (new Transformer({
         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);
+          }
         }
       }
 

From ac1038496b0d7456a30faf9d97cb44fc92f8118a Mon Sep 17 00:00:00 2001
From: thebriando <bdo@atlassian.com>
Date: Thu, 12 May 2022 12:03:19 -0700
Subject: [PATCH 09/16] Remove unnecessary conditions

---
 packages/transformers/js/src/JSTransformer.js | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/packages/transformers/js/src/JSTransformer.js b/packages/transformers/js/src/JSTransformer.js
index dacc88af57d..94157fbd56a 100644
--- a/packages/transformers/js/src/JSTransformer.js
+++ b/packages/transformers/js/src/JSTransformer.js
@@ -728,11 +728,10 @@ export default (new Transformer({
       } of hoist_result.imported_symbols) {
         for (let dep of nullthrows(deps.get(source))) {
           if (
-            !dep ||
-            ((dep.meta.kind === 'Require' ||
+            (dep.meta.kind === 'Require' ||
               dep.meta.kind === 'Import' ||
               dep.meta.kind === 'DynamicImport') &&
-              dep.meta.kind !== kind)
+            dep.meta.kind !== kind
           ) {
             continue;
           }
@@ -741,8 +740,6 @@ export default (new Transformer({
       }
       for (let {source, local, imported, loc} of hoist_result.re_exports) {
         for (let dep of nullthrows(deps.get(source))) {
-          if (!dep) continue;
-
           if (local === '*' && imported === '*') {
             dep.symbols.set('*', '*', convertLoc(loc), true);
           } else {
@@ -757,7 +754,6 @@ export default (new Transformer({
 
       for (let specifier of hoist_result.wrapped_requires) {
         for (let dep of nullthrows(deps.get(specifier))) {
-          if (!dep) continue;
           dep.meta.shouldWrap = true;
         }
       }
@@ -766,7 +762,6 @@ export default (new Transformer({
         for (let dep of nullthrows(
           deps.get(hoist_result.dynamic_imports[name]),
         )) {
-          if (!dep) continue;
           dep.meta.promiseSymbol = name;
         }
       }
@@ -848,7 +843,6 @@ export default (new Transformer({
 
         for (let {source, local, imported, loc} of symbol_result.imports) {
           for (let dep of nullthrows(deps.get(source))) {
-            if (!dep) continue;
             dep.symbols.ensure();
             dep.symbols.set(imported, local, convertLoc(loc));
           }
@@ -856,7 +850,6 @@ export default (new Transformer({
 
         for (let {source, loc} of symbol_result.exports_all) {
           for (let dep of nullthrows(deps.get(source))) {
-            if (!dep) continue;
             dep.symbols.ensure();
             dep.symbols.set('*', '*', convertLoc(loc), true);
           }

From 30eea7b5df4d0fceb254f28b22b5a470bf93ac48 Mon Sep 17 00:00:00 2001
From: thebriando <bdo@atlassian.com>
Date: Fri, 13 May 2022 14:42:31 -0700
Subject: [PATCH 10/16] Only set symbols for esm deps when iterating through
 hoist_result.re_exports

---
 packages/transformers/js/src/JSTransformer.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/packages/transformers/js/src/JSTransformer.js b/packages/transformers/js/src/JSTransformer.js
index 94157fbd56a..47ec6461832 100644
--- a/packages/transformers/js/src/JSTransformer.js
+++ b/packages/transformers/js/src/JSTransformer.js
@@ -740,6 +740,9 @@ export default (new Transformer({
       }
       for (let {source, local, imported, loc} of hoist_result.re_exports) {
         for (let dep of nullthrows(deps.get(source))) {
+          if (dep.specifierType !== 'esm') {
+            continue;
+          }
           if (local === '*' && imported === '*') {
             dep.symbols.set('*', '*', convertLoc(loc), true);
           } else {

From 0f586a7f83091621f313d39fe08cce5a05b84c9a Mon Sep 17 00:00:00 2001
From: thebriando <bdo@atlassian.com>
Date: Mon, 23 May 2022 13:32:40 -0700
Subject: [PATCH 11/16] Add comments for new dep Map structure

---
 packages/transformers/js/src/JSTransformer.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/packages/transformers/js/src/JSTransformer.js b/packages/transformers/js/src/JSTransformer.js
index 47ec6461832..4db177d4212 100644
--- a/packages/transformers/js/src/JSTransformer.js
+++ b/packages/transformers/js/src/JSTransformer.js
@@ -706,6 +706,8 @@ export default (new Transformer({
         asset.symbols.set(exported, local, convertLoc(loc));
       }
 
+      // deps is a Map of specifiers/placeholders to an array of dependencies
+      // since multiple dependencies can be attributed to a single specifier
       let deps = new Map();
       for (let dep of asset.getDependencies()) {
         if (!deps.has(dep.meta.placeholder ?? dep.specifier)) {
@@ -727,6 +729,10 @@ export default (new Transformer({
         kind,
       } of hoist_result.imported_symbols) {
         for (let dep of nullthrows(deps.get(source))) {
+          // dep.meta.kind is a DependencyKind while kind is an ImportKind.
+          // We only set the symbols for the dep if both dep.meta.kind is an
+          // ImportKind and if dep.meta.kind and kind are the same, or if
+          // dep.meta.kind is not an ImportKind
           if (
             (dep.meta.kind === 'Require' ||
               dep.meta.kind === 'Import' ||

From 3cc8069b58bef6712fe23a52055e24c8f595b3aa Mon Sep 17 00:00:00 2001
From: thebriando <bdo@atlassian.com>
Date: Fri, 27 May 2022 14:08:45 -0700
Subject: [PATCH 12/16] Key by specifier + specifierType instead of keeping an
 array of deps

---
 packages/transformers/js/src/JSTransformer.js | 152 ++++++++----------
 1 file changed, 69 insertions(+), 83 deletions(-)

diff --git a/packages/transformers/js/src/JSTransformer.js b/packages/transformers/js/src/JSTransformer.js
index 4db177d4212..7665fd618ff 100644
--- a/packages/transformers/js/src/JSTransformer.js
+++ b/packages/transformers/js/src/JSTransformer.js
@@ -706,19 +706,18 @@ export default (new Transformer({
         asset.symbols.set(exported, local, convertLoc(loc));
       }
 
-      // deps is a Map of specifiers/placeholders to an array of dependencies
-      // since multiple dependencies can be attributed to a single specifier
-      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();
-        }
+      let deps = new Map(
+        asset.getDependencies().map(dep => {
+          return [
+            (dep.meta.placeholder
+              ? String(dep.meta.placeholder)
+              : dep.specifier) + dep.specifierType,
+            dep,
+          ];
+        }),
+      );
+      for (let dep of deps.values()) {
+        dep.symbols.ensure();
       }
 
       for (let {
@@ -728,51 +727,40 @@ export default (new Transformer({
         loc,
         kind,
       } of hoist_result.imported_symbols) {
-        for (let dep of nullthrows(deps.get(source))) {
-          // dep.meta.kind is a DependencyKind while kind is an ImportKind.
-          // We only set the symbols for the dep if both dep.meta.kind is an
-          // ImportKind and if dep.meta.kind and kind are the same, or if
-          // dep.meta.kind is not an ImportKind
-          if (
-            (dep.meta.kind === 'Require' ||
-              dep.meta.kind === 'Import' ||
-              dep.meta.kind === 'DynamicImport') &&
-            dep.meta.kind !== kind
-          ) {
-            continue;
-          }
-          dep.symbols.set(imported, local, convertLoc(loc));
-        }
+        let dep = deps.get(
+          source +
+            (kind === 'Import' || kind === 'Export' || kind === 'DynamicImport'
+              ? 'esm'
+              : 'commonjs'),
+        );
+        if (!dep) continue;
+        dep.symbols.set(imported, local, convertLoc(loc));
       }
+
       for (let {source, local, imported, loc} of hoist_result.re_exports) {
-        for (let dep of nullthrows(deps.get(source))) {
-          if (dep.specifierType !== 'esm') {
-            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);
-          }
+        let dep = deps.get(source + 'esm');
+        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);
         }
       }
 
       for (let specifier of hoist_result.wrapped_requires) {
-        for (let dep of nullthrows(deps.get(specifier))) {
-          dep.meta.shouldWrap = true;
-        }
+        let dep = deps.get(specifier + 'commonjs');
+        if (!dep) continue;
+        dep.meta.shouldWrap = true;
       }
 
       for (let name in hoist_result.dynamic_imports) {
-        for (let dep of nullthrows(
-          deps.get(hoist_result.dynamic_imports[name]),
-        )) {
-          dep.meta.promiseSymbol = name;
-        }
+        let dep = deps.get(hoist_result.dynamic_imports[name] + 'esm');
+        if (!dep) continue;
+        dep.meta.promiseSymbol = name;
       }
 
       if (hoist_result.self_references.length > 0) {
@@ -818,50 +806,48 @@ export default (new Transformer({
       asset.meta.shouldWrap = hoist_result.should_wrap;
     } else {
       if (symbol_result) {
-        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);
-        }
+        let deps = new Map(
+          asset
+            .getDependencies()
+            .map(dep => [
+              (dep.meta.placeholder
+                ? String(dep.meta.placeholder)
+                : dep.specifier) + dep.specifierType,
+              dep,
+            ]),
+        );
         asset.symbols.ensure();
 
         for (let {exported, local, loc, source} of symbol_result.exports) {
-          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,
-                );
-              }
-            }
+          let dep = source ? deps.get(source + 'esm') : 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,
+            );
           }
         }
 
         for (let {source, local, imported, loc} of symbol_result.imports) {
-          for (let dep of nullthrows(deps.get(source))) {
-            dep.symbols.ensure();
-            dep.symbols.set(imported, local, convertLoc(loc));
-          }
+          let dep = deps.get(source + 'esm');
+          if (!dep) continue;
+          dep.symbols.ensure();
+          dep.symbols.set(imported, local, convertLoc(loc));
         }
 
         for (let {source, loc} of symbol_result.exports_all) {
-          for (let dep of nullthrows(deps.get(source))) {
-            dep.symbols.ensure();
-            dep.symbols.set('*', '*', convertLoc(loc), true);
-          }
+          let dep = deps.get(source + 'esm');
+          if (!dep) continue;
+          dep.symbols.ensure();
+          dep.symbols.set('*', '*', convertLoc(loc), true);
         }
       }
 

From 49136f07bb8532ca3eb80ddedd67c278ad6759fe Mon Sep 17 00:00:00 2001
From: thebriando <bdo@atlassian.com>
Date: Sat, 4 Jun 2022 20:07:07 -0700
Subject: [PATCH 13/16] Make import specifiers unique in packager

---
 .../packagers/js/src/ScopeHoistingPackager.js |  2 +-
 packages/transformers/js/core/src/hoist.rs    | 68 +++++++++++++++----
 2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/packages/packagers/js/src/ScopeHoistingPackager.js b/packages/packagers/js/src/ScopeHoistingPackager.js
index 34425e5a8c7..dd30daed61a 100644
--- a/packages/packagers/js/src/ScopeHoistingPackager.js
+++ b/packages/packagers/js/src/ScopeHoistingPackager.js
@@ -581,7 +581,7 @@ ${code}
     let depMap = new Map();
     let replacements = new Map();
     for (let dep of deps) {
-      depMap.set(`${assetId}:${getSpecifier(dep)}`, dep);
+      depMap.set(`${assetId}:${getSpecifier(dep)}:${dep.specifierType}`, dep);
 
       let asyncResolution = this.bundleGraph.resolveAsyncDependency(
         dep,
diff --git a/packages/transformers/js/core/src/hoist.rs b/packages/transformers/js/core/src/hoist.rs
index 86708876cac..0c164549933 100644
--- a/packages/transformers/js/core/src/hoist.rs
+++ b/packages/transformers/js/core/src/hoist.rs
@@ -129,6 +129,14 @@ macro_rules! hoist_visit_fn {
   };
 }
 
+fn get_specifier_type(import_kind: ImportKind) -> String {
+  match import_kind {
+    ImportKind::Import => "esm".to_string(),
+    ImportKind::DynamicImport => "esm".to_string(),
+    ImportKind::Require => "commonjs".to_string(),
+  }
+}
+
 impl<'a> Fold for Hoist<'a> {
   fn fold_module(&mut self, node: Module) -> Module {
     let mut node = node;
@@ -143,10 +151,15 @@ impl<'a> Fold for Hoist<'a> {
                   specifiers: vec![],
                   asserts: None,
                   span: DUMMY_SP,
-                  src: format!("{}:{}", self.module_id, import.src.value).into(),
+                  src: format!(
+                    "{}:{}:{}",
+                    self.module_id,
+                    import.src.value,
+                    get_specifier_type(ImportKind::Import)
+                  )
+                  .into(),
                   type_only: false,
                 })));
-
               // Ensure that all import specifiers are constant.
               for specifier in &import.specifiers {
                 let local = match specifier {
@@ -190,7 +203,13 @@ impl<'a> Fold for Hoist<'a> {
                     asserts: None,
                     span: DUMMY_SP,
                     src: Str {
-                      value: format!("{}:{}", self.module_id, src.value).into(),
+                      value: format!(
+                        "{}:{}:{}",
+                        self.module_id,
+                        src.value,
+                        get_specifier_type(ImportKind::Import)
+                      )
+                      .into(),
                       span: DUMMY_SP,
                       raw: None,
                     },
@@ -285,7 +304,13 @@ impl<'a> Fold for Hoist<'a> {
                   specifiers: vec![],
                   asserts: None,
                   span: DUMMY_SP,
-                  src: format!("{}:{}", self.module_id, export.src.value).into(),
+                  src: format!(
+                    "{}:{}:{}",
+                    self.module_id,
+                    export.src.value,
+                    get_specifier_type(ImportKind::Import)
+                  )
+                  .into(),
                   type_only: false,
                 })));
               self.re_exports.push(ImportedSymbol {
@@ -389,7 +414,13 @@ impl<'a> Fold for Hoist<'a> {
                               asserts: None,
                               span: DUMMY_SP,
                               src: Str {
-                                value: format!("{}:{}", self.module_id, source).into(),
+                                value: format!(
+                                  "{}:{}:{}",
+                                  self.module_id,
+                                  source,
+                                  get_specifier_type(ImportKind::Require)
+                                )
+                                .into(),
                                 span: DUMMY_SP,
                                 raw: None,
                               },
@@ -423,7 +454,6 @@ impl<'a> Fold for Hoist<'a> {
                                 .module_items
                                 .push(ModuleItem::Stmt(Stmt::Decl(Decl::Var(var))));
                             }
-
                             self
                               .module_items
                               .push(ModuleItem::ModuleDecl(ModuleDecl::Import(ImportDecl {
@@ -431,7 +461,13 @@ impl<'a> Fold for Hoist<'a> {
                                 asserts: None,
                                 span: DUMMY_SP,
                                 src: Str {
-                                  value: format!("{}:{}", self.module_id, source).into(),
+                                  value: format!(
+                                    "{}:{}:{}",
+                                    self.module_id,
+                                    source,
+                                    get_specifier_type(ImportKind::Require)
+                                  )
+                                  .into(),
                                   span: DUMMY_SP,
                                   raw: None,
                                 },
@@ -492,7 +528,7 @@ impl<'a> Fold for Hoist<'a> {
               {
                 // Require in statement position (`require('other');`) should behave just
                 // like `import 'other';` in that it doesn't add any symbols (not even '*').
-                self.add_require(&source);
+                self.add_require(&source, ImportKind::Require);
               } else {
                 let d = expr.fold_with(self);
                 self
@@ -601,7 +637,7 @@ impl<'a> Fold for Hoist<'a> {
             if let Some(source) =
               match_require(&member.obj, &self.collect.decls, self.collect.ignore_mark)
             {
-              self.add_require(&source);
+              self.add_require(&source, ImportKind::Require);
               return Expr::Ident(self.get_import_ident(
                 member.span,
                 &source,
@@ -645,7 +681,7 @@ impl<'a> Fold for Hoist<'a> {
       Expr::Call(ref call) => {
         // require('foo') -> $id$import$foo
         if let Some(source) = match_require(&node, &self.collect.decls, self.collect.ignore_mark) {
-          self.add_require(&source);
+          self.add_require(&source, ImportKind::Require);
           return Expr::Ident(self.get_import_ident(
             call.span,
             &source,
@@ -656,7 +692,7 @@ impl<'a> Fold for Hoist<'a> {
         }
 
         if let Some(source) = match_import(&node, self.collect.ignore_mark) {
-          self.add_require(&source);
+          self.add_require(&source, ImportKind::DynamicImport);
           let name: JsWord = format!("${}$importAsync${:x}", self.module_id, hash!(source)).into();
           self.dynamic_imports.insert(name.clone(), source.clone());
           if self.collect.non_static_requires.contains(&source) || self.collect.should_wrap {
@@ -944,14 +980,20 @@ impl<'a> Fold for Hoist<'a> {
 }
 
 impl<'a> Hoist<'a> {
-  fn add_require(&mut self, source: &JsWord) {
+  fn add_require(&mut self, source: &JsWord, import_kind: ImportKind) {
     self
       .module_items
       .push(ModuleItem::ModuleDecl(ModuleDecl::Import(ImportDecl {
         specifiers: vec![],
         asserts: None,
         span: DUMMY_SP,
-        src: format!("{}:{}", self.module_id, source).into(),
+        src: format!(
+          "{}:{}:{}",
+          self.module_id,
+          source,
+          get_specifier_type(import_kind)
+        )
+        .into(),
         type_only: false,
       })));
   }

From daee349b20f60db16a4b7f98ef15a4fb29301d42 Mon Sep 17 00:00:00 2001
From: thebriando <bdo@atlassian.com>
Date: Mon, 6 Jun 2022 23:28:09 -0700
Subject: [PATCH 14/16] Track specifier types in wrapped_requires

---
 packages/transformers/js/core/src/hoist.rs    | 26 ++++++++++++++-----
 packages/transformers/js/src/JSTransformer.js | 10 +++----
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/packages/transformers/js/core/src/hoist.rs b/packages/transformers/js/core/src/hoist.rs
index 0c164549933..c9df2280136 100644
--- a/packages/transformers/js/core/src/hoist.rs
+++ b/packages/transformers/js/core/src/hoist.rs
@@ -75,7 +75,7 @@ pub struct HoistResult {
   exported_symbols: Vec<ExportedSymbol>,
   re_exports: Vec<ImportedSymbol>,
   self_references: HashSet<JsWord>,
-  wrapped_requires: HashSet<JsWord>,
+  wrapped_requires: HashSet<String>,
   dynamic_imports: HashMap<JsWord, JsWord>,
   static_cjs_exports: bool,
   has_cjs_exports: bool,
@@ -1152,7 +1152,7 @@ pub struct Collect {
   pub non_static_access: HashMap<IdentId, Vec<Span>>,
   pub non_const_bindings: HashMap<IdentId, Vec<Span>>,
   pub non_static_requires: HashSet<JsWord>,
-  pub wrapped_requires: HashSet<JsWord>,
+  pub wrapped_requires: HashSet<String>,
   pub bailouts: Option<Vec<Bailout>>,
   in_module_this: bool,
   in_top_level: bool,
@@ -1745,7 +1745,11 @@ impl Visit for Collect {
     // If we reached this visitor, this is a non-top-level require that isn't in a variable
     // declaration. We need to wrap the referenced module to preserve side effect ordering.
     if let Some(source) = self.match_require(node) {
-      self.wrapped_requires.insert(source);
+      self.wrapped_requires.insert(format!(
+        "{}{}",
+        source.to_string(),
+        get_specifier_type(ImportKind::Require)
+      ));
       let span = match node {
         Expr::Call(c) => c.span,
         _ => unreachable!(),
@@ -1755,7 +1759,11 @@ impl Visit for Collect {
 
     if let Some(source) = match_import(node, self.ignore_mark) {
       self.non_static_requires.insert(source.clone());
-      self.wrapped_requires.insert(source);
+      self.wrapped_requires.insert(format!(
+        "{}{}",
+        source.to_string(),
+        get_specifier_type(ImportKind::Import)
+      ));
       let span = match node {
         Expr::Call(c) => c.span,
         _ => unreachable!(),
@@ -1923,7 +1931,11 @@ impl Visit for Collect {
                   self.add_pat_imports(param, &source, ImportKind::DynamicImport);
                 } else {
                   self.non_static_requires.insert(source.clone());
-                  self.wrapped_requires.insert(source);
+                  self.wrapped_requires.insert(format!(
+                    "{}{}",
+                    source.to_string(),
+                    get_specifier_type(ImportKind::Import)
+                  ));
                   self.add_bailout(node.span, BailoutReason::NonStaticDynamicImport);
                 }
 
@@ -1948,7 +1960,9 @@ impl Collect {
 
   fn add_pat_imports(&mut self, node: &Pat, src: &JsWord, kind: ImportKind) {
     if !self.in_top_level {
-      self.wrapped_requires.insert(src.clone());
+      self
+        .wrapped_requires
+        .insert(format!("{}{}", src.clone(), get_specifier_type(kind)));
       if kind != ImportKind::DynamicImport {
         self.non_static_requires.insert(src.clone());
         let span = match node {
diff --git a/packages/transformers/js/src/JSTransformer.js b/packages/transformers/js/src/JSTransformer.js
index 7665fd618ff..09a3a578bc3 100644
--- a/packages/transformers/js/src/JSTransformer.js
+++ b/packages/transformers/js/src/JSTransformer.js
@@ -707,14 +707,14 @@ export default (new Transformer({
       }
 
       let deps = new Map(
-        asset.getDependencies().map(dep => {
-          return [
+        asset
+          .getDependencies()
+          .map(dep => [
             (dep.meta.placeholder
               ? String(dep.meta.placeholder)
               : dep.specifier) + dep.specifierType,
             dep,
-          ];
-        }),
+          ]),
       );
       for (let dep of deps.values()) {
         dep.symbols.ensure();
@@ -752,7 +752,7 @@ export default (new Transformer({
       }
 
       for (let specifier of hoist_result.wrapped_requires) {
-        let dep = deps.get(specifier + 'commonjs');
+        let dep = deps.get(specifier);
         if (!dep) continue;
         dep.meta.shouldWrap = true;
       }

From 48e40ad72833d2bc4b831ace8b8a10c110d9391d Mon Sep 17 00:00:00 2001
From: thebriando <bdo@atlassian.com>
Date: Wed, 15 Jun 2022 22:27:25 -0700
Subject: [PATCH 15/16] Don't append specifier types to placeholders, since
 they are already hashed with DependencyKind's

---
 .../packagers/js/src/ScopeHoistingPackager.js |  9 +-
 packages/transformers/js/core/src/hoist.rs    | 89 +++++--------------
 packages/transformers/js/src/JSTransformer.js | 30 ++++---
 3 files changed, 46 insertions(+), 82 deletions(-)

diff --git a/packages/packagers/js/src/ScopeHoistingPackager.js b/packages/packagers/js/src/ScopeHoistingPackager.js
index dd30daed61a..2e230767a4d 100644
--- a/packages/packagers/js/src/ScopeHoistingPackager.js
+++ b/packages/packagers/js/src/ScopeHoistingPackager.js
@@ -581,7 +581,14 @@ ${code}
     let depMap = new Map();
     let replacements = new Map();
     for (let dep of deps) {
-      depMap.set(`${assetId}:${getSpecifier(dep)}:${dep.specifierType}`, dep);
+      let specifierType =
+        dep.specifierType === 'esm' ? `:${dep.specifierType}` : '';
+      depMap.set(
+        `${assetId}:${getSpecifier(dep)}${
+          !dep.meta.placeholder ? specifierType : ''
+        }`,
+        dep,
+      );
 
       let asyncResolution = this.bundleGraph.resolveAsyncDependency(
         dep,
diff --git a/packages/transformers/js/core/src/hoist.rs b/packages/transformers/js/core/src/hoist.rs
index c9df2280136..d940639b3fa 100644
--- a/packages/transformers/js/core/src/hoist.rs
+++ b/packages/transformers/js/core/src/hoist.rs
@@ -129,14 +129,6 @@ macro_rules! hoist_visit_fn {
   };
 }
 
-fn get_specifier_type(import_kind: ImportKind) -> String {
-  match import_kind {
-    ImportKind::Import => "esm".to_string(),
-    ImportKind::DynamicImport => "esm".to_string(),
-    ImportKind::Require => "commonjs".to_string(),
-  }
-}
-
 impl<'a> Fold for Hoist<'a> {
   fn fold_module(&mut self, node: Module) -> Module {
     let mut node = node;
@@ -151,13 +143,7 @@ impl<'a> Fold for Hoist<'a> {
                   specifiers: vec![],
                   asserts: None,
                   span: DUMMY_SP,
-                  src: format!(
-                    "{}:{}:{}",
-                    self.module_id,
-                    import.src.value,
-                    get_specifier_type(ImportKind::Import)
-                  )
-                  .into(),
+                  src: format!("{}:{}:{}", self.module_id, import.src.value, "esm").into(),
                   type_only: false,
                 })));
               // Ensure that all import specifiers are constant.
@@ -203,13 +189,7 @@ impl<'a> Fold for Hoist<'a> {
                     asserts: None,
                     span: DUMMY_SP,
                     src: Str {
-                      value: format!(
-                        "{}:{}:{}",
-                        self.module_id,
-                        src.value,
-                        get_specifier_type(ImportKind::Import)
-                      )
-                      .into(),
+                      value: format!("{}:{}:{}", self.module_id, src.value, "esm").into(),
                       span: DUMMY_SP,
                       raw: None,
                     },
@@ -304,13 +284,7 @@ impl<'a> Fold for Hoist<'a> {
                   specifiers: vec![],
                   asserts: None,
                   span: DUMMY_SP,
-                  src: format!(
-                    "{}:{}:{}",
-                    self.module_id,
-                    export.src.value,
-                    get_specifier_type(ImportKind::Import)
-                  )
-                  .into(),
+                  src: format!("{}:{}:{}", self.module_id, export.src.value, "esm").into(),
                   type_only: false,
                 })));
               self.re_exports.push(ImportedSymbol {
@@ -414,13 +388,7 @@ impl<'a> Fold for Hoist<'a> {
                               asserts: None,
                               span: DUMMY_SP,
                               src: Str {
-                                value: format!(
-                                  "{}:{}:{}",
-                                  self.module_id,
-                                  source,
-                                  get_specifier_type(ImportKind::Require)
-                                )
-                                .into(),
+                                value: format!("{}:{}", self.module_id, source).into(),
                                 span: DUMMY_SP,
                                 raw: None,
                               },
@@ -461,13 +429,7 @@ impl<'a> Fold for Hoist<'a> {
                                 asserts: None,
                                 span: DUMMY_SP,
                                 src: Str {
-                                  value: format!(
-                                    "{}:{}:{}",
-                                    self.module_id,
-                                    source,
-                                    get_specifier_type(ImportKind::Require)
-                                  )
-                                  .into(),
+                                  value: format!("{}:{}", self.module_id, source,).into(),
                                   span: DUMMY_SP,
                                   raw: None,
                                 },
@@ -981,19 +943,17 @@ impl<'a> Fold for Hoist<'a> {
 
 impl<'a> Hoist<'a> {
   fn add_require(&mut self, source: &JsWord, import_kind: ImportKind) {
+    let src = match import_kind {
+      ImportKind::Import => format!("{}:{}:{}", self.module_id, source, "esm"),
+      ImportKind::DynamicImport | ImportKind::Require => format!("{}:{}", self.module_id, source),
+    };
     self
       .module_items
       .push(ModuleItem::ModuleDecl(ModuleDecl::Import(ImportDecl {
         specifiers: vec![],
         asserts: None,
         span: DUMMY_SP,
-        src: format!(
-          "{}:{}:{}",
-          self.module_id,
-          source,
-          get_specifier_type(import_kind)
-        )
-        .into(),
+        src: src.into(),
         type_only: false,
       })));
   }
@@ -1745,11 +1705,7 @@ impl Visit for Collect {
     // If we reached this visitor, this is a non-top-level require that isn't in a variable
     // declaration. We need to wrap the referenced module to preserve side effect ordering.
     if let Some(source) = self.match_require(node) {
-      self.wrapped_requires.insert(format!(
-        "{}{}",
-        source.to_string(),
-        get_specifier_type(ImportKind::Require)
-      ));
+      self.wrapped_requires.insert(source.to_string());
       let span = match node {
         Expr::Call(c) => c.span,
         _ => unreachable!(),
@@ -1759,11 +1715,7 @@ impl Visit for Collect {
 
     if let Some(source) = match_import(node, self.ignore_mark) {
       self.non_static_requires.insert(source.clone());
-      self.wrapped_requires.insert(format!(
-        "{}{}",
-        source.to_string(),
-        get_specifier_type(ImportKind::Import)
-      ));
+      self.wrapped_requires.insert(source.to_string());
       let span = match node {
         Expr::Call(c) => c.span,
         _ => unreachable!(),
@@ -1931,11 +1883,7 @@ impl Visit for Collect {
                   self.add_pat_imports(param, &source, ImportKind::DynamicImport);
                 } else {
                   self.non_static_requires.insert(source.clone());
-                  self.wrapped_requires.insert(format!(
-                    "{}{}",
-                    source.to_string(),
-                    get_specifier_type(ImportKind::Import)
-                  ));
+                  self.wrapped_requires.insert(source.to_string());
                   self.add_bailout(node.span, BailoutReason::NonStaticDynamicImport);
                 }
 
@@ -1960,9 +1908,14 @@ impl Collect {
 
   fn add_pat_imports(&mut self, node: &Pat, src: &JsWord, kind: ImportKind) {
     if !self.in_top_level {
-      self
-        .wrapped_requires
-        .insert(format!("{}{}", src.clone(), get_specifier_type(kind)));
+      match kind {
+        ImportKind::Import => self
+          .wrapped_requires
+          .insert(format!("{}{}", src.clone(), "esm")),
+        ImportKind::DynamicImport | ImportKind::Require => {
+          self.wrapped_requires.insert(src.to_string())
+        }
+      };
       if kind != ImportKind::DynamicImport {
         self.non_static_requires.insert(src.clone());
         let span = match node {
diff --git a/packages/transformers/js/src/JSTransformer.js b/packages/transformers/js/src/JSTransformer.js
index 09a3a578bc3..fb7fce58a1f 100644
--- a/packages/transformers/js/src/JSTransformer.js
+++ b/packages/transformers/js/src/JSTransformer.js
@@ -706,13 +706,18 @@ export default (new Transformer({
         asset.symbols.set(exported, local, convertLoc(loc));
       }
 
+      // deps is a map of dependencies that are keyed by placeholder or specifier
+      // If a placeholder is present, that is used first since placeholders are
+      // hashed with DependencyKind's.
+      // If not, the specifier is used along with its specifierType appended to
+      // it to separate dependencies with the same specifier.
       let deps = new Map(
         asset
           .getDependencies()
           .map(dep => [
-            (dep.meta.placeholder
-              ? String(dep.meta.placeholder)
-              : dep.specifier) + dep.specifierType,
+            dep.meta.placeholder ??
+              dep.specifier +
+                (dep.specifierType === 'esm' ? dep.specifierType : ''),
             dep,
           ]),
       );
@@ -727,12 +732,11 @@ export default (new Transformer({
         loc,
         kind,
       } of hoist_result.imported_symbols) {
-        let dep = deps.get(
-          source +
-            (kind === 'Import' || kind === 'Export' || kind === 'DynamicImport'
-              ? 'esm'
-              : 'commonjs'),
-        );
+        let specifierType = '';
+        if (kind === 'Import' || kind === 'Export') {
+          specifierType = 'esm';
+        }
+        let dep = deps.get(source + specifierType);
         if (!dep) continue;
         dep.symbols.set(imported, local, convertLoc(loc));
       }
@@ -758,7 +762,7 @@ export default (new Transformer({
       }
 
       for (let name in hoist_result.dynamic_imports) {
-        let dep = deps.get(hoist_result.dynamic_imports[name] + 'esm');
+        let dep = deps.get(hoist_result.dynamic_imports[name]);
         if (!dep) continue;
         dep.meta.promiseSymbol = name;
       }
@@ -810,9 +814,9 @@ export default (new Transformer({
           asset
             .getDependencies()
             .map(dep => [
-              (dep.meta.placeholder
-                ? String(dep.meta.placeholder)
-                : dep.specifier) + dep.specifierType,
+              dep.meta.placeholder ??
+                dep.specifier +
+                  (dep.specifierType === 'esm' ? dep.specifierType : ''),
               dep,
             ]),
         );

From 4c5e7fa3bbeea52db50933cd97258080162922f4 Mon Sep 17 00:00:00 2001
From: thebriando <bdo@atlassian.com>
Date: Thu, 16 Jun 2022 14:04:30 -0700
Subject: [PATCH 16/16] Fix hoist unit tests

---
 packages/transformers/js/core/src/hoist.rs | 56 +++++++++++-----------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/packages/transformers/js/core/src/hoist.rs b/packages/transformers/js/core/src/hoist.rs
index d27a4e9b1f0..cd5d62b7d83 100644
--- a/packages/transformers/js/core/src/hoist.rs
+++ b/packages/transformers/js/core/src/hoist.rs
@@ -2596,7 +2596,7 @@ mod tests {
     );
     assert_eq_set!(collect.non_static_access.into_keys(), set! {});
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2612,7 +2612,7 @@ mod tests {
     );
     assert_eq_set!(collect.non_static_access.into_keys(), set! { w!("x") });
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2626,7 +2626,7 @@ mod tests {
       map! { w!("foo") => (w!("other"), w!("foo"), true) }
     );
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2640,7 +2640,7 @@ mod tests {
       map! { w!("bar") => (w!("other"), w!("foo"), true) }
     );
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2653,7 +2653,7 @@ mod tests {
     );
     assert_eq_set!(collect.non_static_access.into_keys(), set! {});
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2666,7 +2666,7 @@ mod tests {
     );
     assert_eq_set!(collect.non_static_access.into_keys(), set! { w!("x") });
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2678,7 +2678,7 @@ mod tests {
       map! { w!("foo") => (w!("other"), w!("foo"), true) }
     );
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2690,7 +2690,7 @@ mod tests {
       map! { w!("bar") => (w!("other"), w!("foo"), true) }
     );
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2703,7 +2703,7 @@ mod tests {
     );
     assert_eq_set!(collect.non_static_access.into_keys(), set! {});
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2716,7 +2716,7 @@ mod tests {
     );
     assert_eq_set!(collect.non_static_access.into_keys(), set! { w!("x") });
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2728,7 +2728,7 @@ mod tests {
       map! { w!("foo") => (w!("other"), w!("foo"), true) }
     );
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2740,7 +2740,7 @@ mod tests {
       map! { w!("bar") => (w!("other"), w!("foo"), true) }
     );
     assert_eq!(collect.non_static_requires, set! {});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2749,7 +2749,7 @@ mod tests {
     );
     assert_eq_imports!(collect.imports, map! {});
     assert_eq!(collect.non_static_requires, set! {w!("other")});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2758,7 +2758,7 @@ mod tests {
     );
     assert_eq_imports!(collect.imports, map! {});
     assert_eq!(collect.non_static_requires, set! {w!("other")});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
 
     let (collect, _code, _hoist) = parse(
       r#"
@@ -2769,7 +2769,7 @@ mod tests {
     );
     assert_eq_imports!(collect.imports, map! {});
     assert_eq!(collect.non_static_requires, set! {w!("other")});
-    assert_eq!(collect.wrapped_requires, set! {w!("other")});
+    assert_eq!(collect.wrapped_requires, set! {String::from("other")});
   }
 
   #[test]
@@ -2788,7 +2788,7 @@ mod tests {
     assert_eq!(
       code,
       indoc! {r#"
-    import "abc:other";
+    import "abc:other:esm";
     let $abc$var$test = {
         bar: 3
     };
@@ -2809,7 +2809,7 @@ mod tests {
     assert_eq!(
       code,
       indoc! {r#"
-    import "abc:other";
+    import "abc:other:esm";
     console.log($abc$import$70a00e0a8474f72a$d927737047eb3867);
     $abc$import$70a00e0a8474f72a$d927737047eb3867();
     "#}
@@ -2836,7 +2836,7 @@ mod tests {
     assert_eq!(
       code,
       indoc! {r#"
-    import "abc:other";
+    import "abc:other:esm";
     $abc$import$70a00e0a8474f72a.bar();
     let $abc$var$y = "bar";
     $abc$import$70a00e0a8474f72a[$abc$var$y]();
@@ -2856,7 +2856,7 @@ mod tests {
     assert_eq!(
       code,
       indoc! {r#"
-    import "abc:other";
+    import "abc:other:esm";
     console.log((0, $abc$import$70a00e0a8474f72a$2e2bcd8739ae039), (0, $abc$import$70a00e0a8474f72a$2e2bcd8739ae039).bar);
     (0, $abc$import$70a00e0a8474f72a$2e2bcd8739ae039)();
     "#}
@@ -2877,8 +2877,8 @@ mod tests {
     assert_eq!(
       code,
       indoc! {r#"
-    import "abc:other";
-    import "abc:bar";
+    import "abc:other:esm";
+    import "abc:bar:esm";
     console.log((0, $abc$import$70a00e0a8474f72a$2e2bcd8739ae039));
     console.log((0, $abc$import$d927737047eb3867$2e2bcd8739ae039));
     "#}
@@ -2898,8 +2898,8 @@ mod tests {
     assert_eq!(
       code,
       indoc! {r#"
-    import "abc:other";
-    import "abc:bar";
+    import "abc:other:esm";
+    import "abc:bar:esm";
     console.log((0, $abc$import$70a00e0a8474f72a$2e2bcd8739ae039));
     import "abc:x";
     console.log($abc$import$d141bba7fdc215a3);
@@ -3095,7 +3095,7 @@ mod tests {
     );
     assert_eq!(
       hoist.wrapped_requires,
-      HashSet::<JsWord>::from_iter(vec![JsWord::from("other")])
+      HashSet::<String>::from_iter(vec![String::from("other")])
     );
 
     let (_collect, code, hoist) = parse(
@@ -3121,7 +3121,7 @@ mod tests {
     );
     assert_eq!(
       hoist.wrapped_requires,
-      HashSet::<JsWord>::from_iter(vec![JsWord::from("other")])
+      HashSet::<String>::from_iter(vec![String::from("other")])
     );
 
     let (_collect, code, _hoist) = parse(
@@ -3449,7 +3449,7 @@ mod tests {
     assert_eq!(
       code,
       indoc! {r#"
-    import "abc:bar";
+    import "abc:bar:esm";
     "#}
     );
 
@@ -3462,7 +3462,7 @@ mod tests {
     assert_eq!(
       code,
       indoc! {r#"
-    import "abc:bar";
+    import "abc:bar:esm";
     "#}
     );
 
@@ -3476,7 +3476,7 @@ mod tests {
     assert_eq!(
       code,
       indoc! {r#"
-    import "abc:./settings";
+    import "abc:./settings:esm";
     const $abc$export$a5a6e0b888b2c992 = "hi";
     "#}
     );