Skip to content

Commit 3897273

Browse files
nicolo-ribaudonicolo-ribaudo
authored
Avoid duplicate side-effects-only require() calls (#55)
1 parent 3364145 commit 3897273

4 files changed

Lines changed: 112 additions & 8 deletions

File tree

‎packages/babel-helper-define-polyfill-provider/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
"@babel/cli": "^7.11.5",
3636
"@babel/core": "^7.11.5",
3737
"@babel/generator": "^7.11.5",
38+
"@babel/plugin-transform-modules-commonjs": "^7.10.4",
3839
"babel-loader": "^8.1.0",
3940
"rollup": "^2.3.2",
4041
"rollup-plugin-babel": "^4.4.0",

‎packages/babel-helper-define-polyfill-provider/src/imports-cache.js‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export default class ImportsCache {
2424
// eslint-disable-next-line no-undef
2525
getVal: (isScript: boolean, source: t.StringLiteral) => t.Node,
2626
) {
27-
const key = this._normalizeKey(programPath, url, "");
27+
const key = this._normalizeKey(programPath, url);
2828
const imports = this._ensure(this._anonymousImports, programPath, Set);
2929

3030
if (imports.has(key)) return;
@@ -89,7 +89,12 @@ export default class ImportsCache {
8989
return collection;
9090
}
9191

92-
_normalizeKey(programPath: NodePath, url: string, name: string): string {
93-
return `${programPath.node.sourceType}::${url}::${name}`;
92+
_normalizeKey(programPath: NodePath, url: string, name: string = ""): string {
93+
const { sourceType } = programPath.node;
94+
95+
// If we rely on the imported binding (the "name" parameter), we also need to cache
96+
// based on the sourceType. This is because the module transforms change the names
97+
// of the import variables.
98+
return `${name && sourceType}::${url}::${name}`;
9499
}
95100
}

‎packages/babel-helper-define-polyfill-provider/test/injectors.js‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as babel from "@babel/core";
22
import definePolyfillProvider from "../lib";
33
import astToCode from "./helpers/ast-to-code-serializer.js";
4+
import pluginCJS from "@babel/plugin-transform-modules-commonjs";
45

56
function withUtils(code, fn, opts) {
67
let result;
@@ -311,5 +312,101 @@ describe("injectors", () => {
311312
foo;
312313
`);
313314
});
315+
316+
describe("with the commonjs transform", () => {
317+
function testImportsCacheWithModulesTransform(method, provider) {
318+
const source = "foo;";
319+
320+
return babel.transformSync(source, {
321+
configFile: false,
322+
plugins: [
323+
[definePolyfillProvider(() => provider), { method }],
324+
pluginCJS,
325+
326+
// After converting ESM to CJS, we inject a new
327+
// reference that needs to be polyfilled.
328+
({ template }) => ({
329+
visitor: {
330+
Program: {
331+
exit(path) {
332+
path.pushContainer("body", template.ast(source));
333+
},
334+
},
335+
},
336+
}),
337+
],
338+
ast: true,
339+
code: false,
340+
sourceType: "module",
341+
});
342+
}
343+
344+
it("injectGlobalImport", () => {
345+
const { ast } = testImportsCacheWithModulesTransform("usage-global", {
346+
usageGlobal(meta, utils) {
347+
if (meta.kind === "global" && meta.name === "foo") {
348+
utils.injectGlobalImport("./polyfill/foo");
349+
}
350+
},
351+
});
352+
353+
expect(ast).toMatchInlineSnapshot(`
354+
"use strict";
355+
356+
require("./polyfill/foo");
357+
358+
foo;
359+
foo;
360+
`);
361+
});
362+
363+
it("injectNamedImport", () => {
364+
const { ast } = testImportsCacheWithModulesTransform("usage-global", {
365+
usageGlobal(meta, utils, path) {
366+
if (meta.kind === "global" && meta.name === "foo") {
367+
path.replaceWith(
368+
utils.injectNamedImport("./polyfill/foo", "foo"),
369+
);
370+
}
371+
},
372+
});
373+
374+
expect(ast).toMatchInlineSnapshot(`
375+
"use strict";
376+
377+
var _foo3 = require("./polyfill/foo").foo;
378+
379+
var _foo2 = require("./polyfill/foo");
380+
381+
_foo2.foo;
382+
_foo3;
383+
`);
384+
});
385+
386+
it("injectDefaultImport", () => {
387+
const { ast } = testImportsCacheWithModulesTransform("usage-global", {
388+
usageGlobal(meta, utils, path) {
389+
if (meta.kind === "global" && meta.name === "foo") {
390+
path.replaceWith(
391+
utils.injectDefaultImport("./polyfill/foo", "foo"),
392+
);
393+
}
394+
},
395+
});
396+
397+
expect(ast).toMatchInlineSnapshot(`
398+
"use strict";
399+
400+
var _foo3 = require("./polyfill/foo");
401+
402+
var _foo2 = _interopRequireDefault(require("./polyfill/foo"));
403+
404+
function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
405+
406+
_foo2.default;
407+
_foo3;
408+
`);
409+
});
410+
});
314411
});
315412
});

‎yarn.lock‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ __metadata:
166166
languageName: node
167167
linkType: hard
168168

169-
"@babel/helper-define-polyfill-provider@^0.0.6, @babel/helper-define-polyfill-provider@workspace:packages/babel-helper-define-polyfill-provider":
169+
"@babel/helper-define-polyfill-provider@^0.0.7, @babel/helper-define-polyfill-provider@workspace:packages/babel-helper-define-polyfill-provider":
170170
version: 0.0.0-use.local
171171
resolution: "@babel/helper-define-polyfill-provider@workspace:packages/babel-helper-define-polyfill-provider"
172172
dependencies:
@@ -176,6 +176,7 @@ __metadata:
176176
"@babel/helper-compilation-targets": ^7.10.4
177177
"@babel/helper-module-imports": ^7.10.4
178178
"@babel/helper-plugin-utils": ^7.10.4
179+
"@babel/plugin-transform-modules-commonjs": ^7.10.4
179180
"@babel/traverse": ^7.11.5
180181
babel-loader: ^8.1.0
181182
debug: ^4.1.1
@@ -3596,7 +3597,7 @@ __metadata:
35963597
dependencies:
35973598
"@babel/compat-data": ^7.11.0
35983599
"@babel/core": ^7.11.5
3599-
"@babel/helper-define-polyfill-provider": ^0.0.6
3600+
"@babel/helper-define-polyfill-provider": ^0.0.7
36003601
"@babel/helper-plugin-test-runner": ^7.10.4
36013602
"@babel/plugin-transform-for-of": ^7.10.4
36023603
"@babel/plugin-transform-modules-commonjs": ^7.10.4
@@ -3611,7 +3612,7 @@ __metadata:
36113612
resolution: "babel-plugin-polyfill-corejs3@workspace:packages/babel-plugin-polyfill-corejs3"
36123613
dependencies:
36133614
"@babel/core": ^7.11.5
3614-
"@babel/helper-define-polyfill-provider": ^0.0.6
3615+
"@babel/helper-define-polyfill-provider": ^0.0.7
36153616
"@babel/helper-plugin-test-runner": ^7.10.4
36163617
"@babel/plugin-syntax-dynamic-import": ^7.8.3
36173618
"@babel/plugin-transform-for-of": ^7.10.4
@@ -3628,7 +3629,7 @@ __metadata:
36283629
resolution: "babel-plugin-polyfill-es-shims@workspace:packages/babel-plugin-polyfill-es-shims"
36293630
dependencies:
36303631
"@babel/core": ^7.11.5
3631-
"@babel/helper-define-polyfill-provider": ^0.0.6
3632+
"@babel/helper-define-polyfill-provider": ^0.0.7
36323633
"@babel/helper-plugin-test-runner": ^7.10.4
36333634
array.from: ^1.1.0
36343635
peerDependencies:
@@ -3641,7 +3642,7 @@ __metadata:
36413642
resolution: "babel-plugin-polyfill-regenerator@workspace:packages/babel-plugin-polyfill-regenerator"
36423643
dependencies:
36433644
"@babel/core": ^7.11.5
3644-
"@babel/helper-define-polyfill-provider": ^0.0.6
3645+
"@babel/helper-define-polyfill-provider": ^0.0.7
36453646
"@babel/helper-plugin-test-runner": ^7.10.4
36463647
peerDependencies:
36473648
"@babel/core": ^7.0.0-0

0 commit comments

Comments
 (0)