From 7f471468b7371a12385ecca701f8494bb09c44ef Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Wed, 14 Dec 2022 16:07:29 -0500 Subject: [PATCH] Fix requires of external CommonJS SWC helpers (#8693) --- packages/core/integration-tests/test/fs.js | 1 - .../formats/commonjs-helpers/index.js | 5 +++ .../formats/commonjs-helpers/package.json | 7 ++++ .../formats/commonjs-helpers/yarn.lock | 0 .../core/integration-tests/test/javascript.js | 10 ++--- .../integration-tests/test/output-formats.js | 16 ++++++++ .../core/integration-tests/test/server.js | 1 - .../core/integration-tests/test/sourcemaps.js | 4 +- packages/packagers/js/src/DevPackager.js | 3 ++ .../js/core/src/dependency_collector.rs | 4 +- packages/transformers/js/src/JSTransformer.js | 41 ++++--------------- 11 files changed, 48 insertions(+), 44 deletions(-) create mode 100644 packages/core/integration-tests/test/integration/formats/commonjs-helpers/index.js create mode 100644 packages/core/integration-tests/test/integration/formats/commonjs-helpers/package.json create mode 100644 packages/core/integration-tests/test/integration/formats/commonjs-helpers/yarn.lock diff --git a/packages/core/integration-tests/test/fs.js b/packages/core/integration-tests/test/fs.js index 800432aca0d..f5ed788456c 100644 --- a/packages/core/integration-tests/test/fs.js +++ b/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( diff --git a/packages/core/integration-tests/test/integration/formats/commonjs-helpers/index.js b/packages/core/integration-tests/test/integration/formats/commonjs-helpers/index.js new file mode 100644 index 00000000000..8debcd04a98 --- /dev/null +++ b/packages/core/integration-tests/test/integration/formats/commonjs-helpers/index.js @@ -0,0 +1,5 @@ +class X { + x = new Map() +} + +output(new X()); diff --git a/packages/core/integration-tests/test/integration/formats/commonjs-helpers/package.json b/packages/core/integration-tests/test/integration/formats/commonjs-helpers/package.json new file mode 100644 index 00000000000..98fd5a788ec --- /dev/null +++ b/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" + } +} diff --git a/packages/core/integration-tests/test/integration/formats/commonjs-helpers/yarn.lock b/packages/core/integration-tests/test/integration/formats/commonjs-helpers/yarn.lock new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js index 0411db880b5..1e0a9878d8d 100644 --- a/packages/core/integration-tests/test/javascript.js +++ b/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); diff --git a/packages/core/integration-tests/test/output-formats.js b/packages/core/integration-tests/test/output-formats.js index fed35fa4a01..4e4e4236b15 100644 --- a/packages/core/integration-tests/test/output-formats.js +++ 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 () { diff --git a/packages/core/integration-tests/test/server.js b/packages/core/integration-tests/test/server.js index 76576b02969..bb25bad5650 100644 --- a/packages/core/integration-tests/test/server.js +++ b/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')); }); }); diff --git a/packages/core/integration-tests/test/sourcemaps.js b/packages/core/integration-tests/test/sourcemaps.js index 42de11f19ca..f5d58e3d898 100644 --- a/packages/core/integration-tests/test/sourcemaps.js +++ b/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', }); diff --git a/packages/packagers/js/src/DevPackager.js b/packages/packagers/js/src/DevPackager.js index 845f2af9183..d0b5b5234b6 100644 --- a/packages/packagers/js/src/DevPackager.js +++ b/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; } } diff --git a/packages/transformers/js/core/src/dependency_collector.rs b/packages/transformers/js/core/src/dependency_collector.rs index 97ec43bd29b..fe15422ba40 100644 --- a/packages/transformers/js/core/src/dependency_collector.rs +++ b/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 { diff --git a/packages/transformers/js/src/JSTransformer.js b/packages/transformers/js/src/JSTransformer.js index e19a9259237..961254b6d91 100644 --- a/packages/transformers/js/src/JSTransformer.js +++ b/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);