Fix requires of external CommonJS SWC helpers (#8693)

packages/core/integration-tests/test/fs.js

@@ -198,7 +198,6 @@ describe('fs', function () {
         path.join(distDir, 'index.js'),
         'utf8',
       );
-      assert(contents.includes(`require("fs")`));
       assert(contents.includes('readFileSync'));
 
       await outputFS.writeFile(

packages/core/integration-tests/test/integration/formats/commonjs-helpers/index.js

@@ -0,0 +1,5 @@
+class X {
+  x = new Map()
+}
+
+output(new X());

packages/core/integration-tests/test/integration/formats/commonjs-helpers/package.json

@@ -0,0 +1,7 @@
+{
+  "main": "dist/main.js",
+  "browserslist": "Chrome 70",
+  "dependencies": {
+    "@swc/helpers": "^0.4.14"
+  }
+}

packages/core/integration-tests/test/javascript.js

@@ -2694,27 +2694,27 @@ describe('javascript', function () {
     let dist = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
     assert(
       dist.includes(
-        'require("path").resolve(__dirname, "../test/integration/env-node-replacements")',
+        'resolve(__dirname, "../test/integration/env-node-replacements")',
       ),
     );
     assert(
       dist.includes(
-        'require("path").resolve(__dirname, "../test/integration/env-node-replacements/other")',
+        'resolve(__dirname, "../test/integration/env-node-replacements/other")',
       ),
     );
     assert(
       dist.includes(
-        'require("path").resolve(__dirname, "../test/integration/env-node-replacements", "index.js")',
+        'resolve(__dirname, "../test/integration/env-node-replacements", "index.js")',
       ),
     );
     assert(
       dist.includes(
-        'require("path").resolve(__dirname, "../test/integration/env-node-replacements/sub")',
+        'resolve(__dirname, "../test/integration/env-node-replacements/sub")',
       ),
     );
     assert(
       dist.includes(
-        'require("path").resolve(__dirname, "../test/integration/env-node-replacements/sub", "index.js")',
+        'resolve(__dirname, "../test/integration/env-node-replacements/sub", "index.js")',
       ),
     );
     let f = await run(b);

packages/core/integration-tests/test/output-formats.js

@@ -560,6 +560,22 @@ describe('output formats', function () {
 
       assert.deepEqual(out, [1, 2]);
     });
+
+    it('should work with SWC helpers', async function () {
+      let b = await bundle(
+        path.join(__dirname, '/integration/formats/commonjs-helpers/index.js'),
+      );
+
+      let out = [];
+      await run(b, {
+        require,
+        output(o) {
+          out.push(o);
+        },
+      });
+
+      assert.deepEqual(out[0].x, new Map());
+    });
   });
 
   describe('esmodule', function () {

packages/core/integration-tests/test/server.js

@@ -639,6 +639,5 @@ describe('server', function () {
     invariant(localCSS);
 
     assert(data.includes(path.basename(localCSS.filePath)));
-    assert(data.includes('css-loader'));
   });
 });

packages/core/integration-tests/test/sourcemaps.js

@@ -445,7 +445,7 @@ describe('sourcemaps', function () {
       source: inputs[0],
       generated: raw,
       str: 'const local',
-      generatedStr: 'const t',
+      generatedStr: 'const r',
       sourcePath: 'index.js',
     });
 
@@ -454,7 +454,7 @@ describe('sourcemaps', function () {
       source: inputs[0],
       generated: raw,
       str: 'local.a',
-      generatedStr: 't.a',
+      generatedStr: 'r.a',
       sourcePath: 'index.js',
     });
 

packages/packagers/js/src/DevPackager.js

@@ -106,6 +106,9 @@ export class DevPackager {
           } else if (resolved) {
             deps[getSpecifier(dep)] =
               this.bundleGraph.getAssetPublicId(resolved);
+          } else {
+            // An external module - map placeholder to original specifier.
+            deps[getSpecifier(dep)] = dep.specifier;
           }
         }
 

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

@@ -115,12 +115,12 @@ impl<'a> DependencyCollector<'a> {
       }
     }
 
-    // For normal imports/requires, the specifier will remain unchanged.
+    // For ESM imports, the specifier will remain unchanged.
     // For other types of dependencies, the specifier will be changed to a hash
     // that also contains the dependency kind. This way, multiple kinds of dependencies
     // to the same specifier can be used within the same file.
     let placeholder = match kind {
-      DependencyKind::Import | DependencyKind::Export | DependencyKind::Require => {
+      DependencyKind::Import | DependencyKind::Export => {
         if is_specifier_rewritten {
           Some(specifier.as_ref().to_owned())
         } else {

packages/transformers/js/src/JSTransformer.js

@@ -730,12 +730,7 @@ export default (new Transformer({
       let deps = new Map(
         asset
           .getDependencies()
-          .map(dep => [
-            dep.meta.placeholder ??
-              dep.specifier +
-                (dep.specifierType === 'esm' ? dep.specifierType : ''),
-            dep,
-          ]),
+          .map(dep => [dep.meta.placeholder ?? dep.specifier, dep]),
       );
       for (let dep of deps.values()) {
         dep.symbols.ensure();
@@ -746,19 +741,14 @@ export default (new Transformer({
         local,
         imported,
         loc,
-        kind,
       } of hoist_result.imported_symbols) {
-        let specifierType = '';
-        if (kind === 'Import' || kind === 'Export') {
-          specifierType = 'esm';
-        }
-        let dep = deps.get(source + specifierType);
+        let dep = deps.get(source);
         if (!dep) continue;
         dep.symbols.set(imported, local, convertLoc(loc));
       }
 
       for (let {source, local, imported, loc} of hoist_result.re_exports) {
-        let dep = deps.get(source + 'esm');
+        let dep = deps.get(source);
         if (!dep) continue;
         if (local === '*' && imported === '*') {
           dep.symbols.set('*', '*', convertLoc(loc), true);
@@ -829,17 +819,12 @@ export default (new Transformer({
         let deps = new Map(
           asset
             .getDependencies()
-            .map(dep => [
-              dep.meta.placeholder ??
-                dep.specifier +
-                  (dep.specifierType === 'esm' ? dep.specifierType : ''),
-              dep,
-            ]),
+            .map(dep => [dep.meta.placeholder ?? dep.specifier, dep]),
         );
         asset.symbols.ensure();
 
         for (let {exported, local, loc, source} of symbol_result.exports) {
-          let dep = source ? deps.get(source + 'esm') : undefined;
+          let dep = source ? deps.get(source) : undefined;
           asset.symbols.set(
             exported,
             `${dep?.id ?? ''}$${local}`,
@@ -856,25 +841,15 @@ export default (new Transformer({
           }
         }
 
-        for (let {
-          source,
-          local,
-          imported,
-          kind,
-          loc,
-        } of symbol_result.imports) {
-          let specifierType = '';
-          if (kind === 'Import' || kind === 'Export') {
-            specifierType = 'esm';
-          }
-          let dep = deps.get(source + specifierType);
+        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 {source, loc} of symbol_result.exports_all) {
-          let dep = deps.get(source + 'esm');
+          let dep = deps.get(source);
           if (!dep) continue;
           dep.symbols.ensure();
           dep.symbols.set('*', '*', convertLoc(loc), true);