Default interop missing when importing a CommonJS module (#7991)

* Add test case for missing default interop for commonjs import

* Add interopDefault helper if the dep kind is require

* Add test case for 'static' hybrid interop

* Update test description

Co-authored-by: Eric Eldredge <lettertwo@gmail.com>

* Update test dirname

Co-authored-by: Eric Eldredge <lettertwo@gmail.com>

* Fix dirname

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

Co-authored-by: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com>

* Store import kinds in imported symbols and only set symbols when kinds match while iterating

* Remove unnecessary conditions

* Only set symbols for esm deps when iterating through hoist_result.re_exports

* Add comments for new dep Map structure

* Key by specifier + specifierType instead of keeping an array of deps

* Make import specifiers unique in packager

* Track specifier types in wrapped_requires

* Don't append specifier types to placeholders, since they are already hashed with DependencyKind's

* Fix hoist unit tests

Co-authored-by: lettertwo <lettertwo@gmail.com>
Co-authored-by: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com>

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

@@ -4486,6 +4486,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/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);
+      let specifierType =
+        dep.specifierType === 'esm' ? `:${dep.specifierType}` : '';
+      depMap.set(
+        `${assetId}:${getSpecifier(dep)}${
+          !dep.meta.placeholder ? specifierType : ''
+        }`,
+        dep,
+      );
 
       let asyncResolution = this.bundleGraph.resolveAsyncDependency(
         dep,