From 3d355566dca3d2197c45a1c306a909e83d85afd8 Mon Sep 17 00:00:00 2001 From: Sebastian McKenzie Date: Sun, 25 Jan 2015 20:39:41 +1100 Subject: [PATCH] avoid duplicate requires when importing modules --- lib/6to5/transformation/modules/_default.js | 167 +++++++++--------- lib/6to5/transformation/modules/amd.js | 21 +-- lib/6to5/transformation/modules/common.js | 42 +++-- .../exports-from/expected.js | 18 +- .../hoist-function-exports/expected.js | 2 +- .../imports-default/expected.js | 6 +- .../imports-mixing/expected.js | 6 +- .../imports-named/expected.js | 14 +- .../es6-modules-common/overview/actual.js | 8 +- .../es6-modules-common/overview/expected.js | 10 +- .../self-contained/full/expected.js | 8 +- 11 files changed, 158 insertions(+), 144 deletions(-) diff --git a/lib/6to5/transformation/modules/_default.js b/lib/6to5/transformation/modules/_default.js index de9456cdee..f9d4901fec 100644 --- a/lib/6to5/transformation/modules/_default.js +++ b/lib/6to5/transformation/modules/_default.js @@ -10,108 +10,71 @@ var _ = require("lodash"); function DefaultFormatter(file) { this.file = file; + this.ids = object(); - this.localExports = this.getLocalExports(); - this.localImports = this.getLocalImports(); + this.hasLocalExports = false; + this.hasLocalImports = false; + + this.localImportOccurences = object(); + this.localExports = object(); + this.localImports = object(); + + this.getLocalExports(); + this.getLocalImports(); this.remapAssignments(); - - //this.checkCollisions(); } +DefaultFormatter.prototype.bumpImportOccurences = function (node) { + var source = node.source.value; + this.localImportOccurences[source] = this.localImportOccurences[source] || 0; + this.localImportOccurences[source] += node.specifiers.length; +}; + var exportsVisitor = { - enter: function (node, parent, scope, context, localExports) { + enter: function (node, parent, scope, context, formatter) { var declar = node && node.declaration; - if (t.isExportDeclaration(node) && declar && t.isStatement(declar)) { - _.extend(localExports, t.getIds(declar, true)); + if (t.isExportDeclaration(node)) { + formatter.hasLocalImports = true; + if (declar && t.isStatement(declar)) { + _.extend(formatter.localExports, t.getIds(declar, true)); + } + + if (node.source) { + formatter.bumpImportOccurences(node); + } } } }; DefaultFormatter.prototype.getLocalExports = function () { - var localExports = object(); - traverse(this.file.ast, exportsVisitor, this.file.scope, localExports); - return localExports; + traverse(this.file.ast, exportsVisitor, this.file.scope, this); }; var importsVisitor = { - enter: function (node, parent, scope, context, localImports) { + enter: function (node, parent, scope, context, formatter) { if (t.isImportDeclaration(node)) { - _.extend(localImports, t.getIds(node, true)); + formatter.hasLocalImports = true; + _.extend(formatter.localImports, t.getIds(node, true)); + formatter.bumpImportOccurences(node); } } }; DefaultFormatter.prototype.getLocalImports = function () { - var localImports = object(); - traverse(this.file.ast, importsVisitor, this.file.scope, localImports); - return localImports; -}; - -DefaultFormatter.prototype.isLocalReference = function (node) { - var localImports = this.localImports; - return t.isIdentifier(node) && localImports[node.name] && localImports[node.name] !== node; -}; - -DefaultFormatter.prototype.checkLocalReference = function (node) { - var file = this.file; - if (this.isLocalReference(node)) { - throw file.errorWithNode(node, "Illegal assignment of module import"); - } -}; - -var collissionsVisitor = { - enter: function (node, parent, scope, context, state) { - var self = state.self; - if (t.isAssignmentExpression(node)) { - - var left = node.left; - if (t.isMemberExpression(left)) { - while (left.object) left = left.object; - } - - self.checkLocalReference(left); - } else if (t.isDeclaration(node)) { - _.each(t.getIds(node, true), self.checkLocalReference); - } - } -}; - -DefaultFormatter.prototype.checkCollisions = function () { - // todo: all check export collissions - var state = { self: this }; - traverse(this.file.ast, collissionsVisitor, null, state); -}; - -DefaultFormatter.prototype.remapExportAssignment = function (node) { - return t.assignmentExpression( - "=", - node.left, - t.assignmentExpression( - node.operator, - t.memberExpression(t.identifier("exports"), node.left), - node.right - ) - ); -}; - -DefaultFormatter.prototype.isLocalReference = function (node, scope) { - var localExports = this.localExports; - var name = node.name; - return t.isIdentifier(node) && localExports[name] && localExports[name] === scope.get(name, true); + traverse(this.file.ast, importsVisitor, this.file.scope, this); }; var remapVisitor = { - enter: function (node, parent, scope, context, state) { - var self = state.self; - if (t.isUpdateExpression(node) && self.isLocalReference(node.argument, scope)) { + enter: function (node, parent, scope, context, formatter) { + if (t.isUpdateExpression(node) && formatter.isLocalReference(node.argument, scope)) { context.skip(); // expand to long file assignment expression var assign = t.assignmentExpression(node.operator[0] + "=", node.argument, t.literal(1)); // remap this assignment expression - var remapped = self.remapExportAssignment(assign); + var remapped = formatter.remapExportAssignment(assign); // we don't need to change the result if (t.isExpressionStatement(parent) || node.prefix) { @@ -132,16 +95,47 @@ var remapVisitor = { return t.sequenceExpression(nodes); } - if (t.isAssignmentExpression(node) && self.isLocalReference(node.left, scope)) { + if (t.isAssignmentExpression(node) && formatter.isLocalReference(node.left, scope)) { context.skip(); - return self.remapExportAssignment(node); + return formatter.remapExportAssignment(node); } } }; DefaultFormatter.prototype.remapAssignments = function () { - var state = { self: this }; - traverse(this.file.ast, remapVisitor, this.file.scope, state); + if (this.hasLocalImports) { + traverse(this.file.ast, remapVisitor, this.file.scope, this); + } +}; + +DefaultFormatter.prototype.isLocalReference = function (node) { + var localImports = this.localImports; + return t.isIdentifier(node) && localImports[node.name] && localImports[node.name] !== node; +}; + +DefaultFormatter.prototype.checkLocalReference = function (node) { + var file = this.file; + if (this.isLocalReference(node)) { + throw file.errorWithNode(node, "Illegal assignment of module import"); + } +}; + +DefaultFormatter.prototype.remapExportAssignment = function (node) { + return t.assignmentExpression( + "=", + node.left, + t.assignmentExpression( + node.operator, + t.memberExpression(t.identifier("exports"), node.left), + node.right + ) + ); +}; + +DefaultFormatter.prototype.isLocalReference = function (node, scope) { + var localExports = this.localExports; + var name = node.name; + return t.isIdentifier(node) && localExports[name] && localExports[name] === scope.get(name, true); }; DefaultFormatter.prototype.getModuleName = function () { @@ -195,21 +189,34 @@ DefaultFormatter.prototype._hoistExport = function (declar, assign, priority) { return assign; }; -DefaultFormatter.prototype._exportSpecifier = function (getRef, specifier, node, nodes) { +DefaultFormatter.prototype.push = function (node, nodes) { + var ids = this.ids; + var id = node.source.value; + + if (ids[id]) { + return ids[id]; + } else { + return this.ids[id] = this._push(node, nodes); + } +}; + +DefaultFormatter.prototype.exportSpecifier = function (specifier, node, nodes) { var inherits = false; if (node.specifiers.length === 1) inherits = node; if (node.source) { + var ref = this.push(node, nodes); + if (t.isExportBatchSpecifier(specifier)) { // export * from "foo"; - nodes.push(this._exportsWildcard(getRef(), node)); + nodes.push(this._exportsWildcard(ref, node)); } else { var ref; if (t.isSpecifierDefault(specifier) && !this.noInteropRequire) { // importing a default so we need to normalise it - ref = t.callExpression(this.file.addHelper("interop-require"), [getRef()]); + ref = t.callExpression(this.file.addHelper("interop-require"), [ref]); } else { - ref = t.memberExpression(getRef(), t.getSpecifierId(specifier)); + ref = t.memberExpression(ref, t.getSpecifierId(specifier)); } // export { foo } from "test"; diff --git a/lib/6to5/transformation/modules/amd.js b/lib/6to5/transformation/modules/amd.js index cdc8252443..ffeaf727fc 100644 --- a/lib/6to5/transformation/modules/amd.js +++ b/lib/6to5/transformation/modules/amd.js @@ -10,7 +10,6 @@ var _ = require("lodash"); function AMDFormatter() { CommonFormatter.apply(this, arguments); - this.ids = {}; } util.inherits(AMDFormatter, DefaultFormatter); @@ -69,23 +68,16 @@ AMDFormatter.prototype.getModuleName = function () { }; AMDFormatter.prototype._push = function (node) { - var id = node.source.value; - var ids = this.ids; - - if (ids[id]) { - return ids[id]; - } else { - return this.ids[id] = this.file.generateUidIdentifier(id); - } + return this.file.generateUidIdentifier(node.source.value); }; AMDFormatter.prototype.importDeclaration = function (node) { - this._push(node); + this.push(node); }; AMDFormatter.prototype.importSpecifier = function (specifier, node, nodes) { var key = t.getSpecifierName(specifier); - var ref = this._push(node); + var ref = this.push(node); if (_.contains(this.file.dynamicImported, node)) { // Prevent unnecessary renaming of dynamic imports. @@ -113,10 +105,3 @@ AMDFormatter.prototype.exportDeclaration = function (node) { CommonFormatter.prototype.exportDeclaration.apply(this, arguments); }; - -AMDFormatter.prototype.exportSpecifier = function (specifier, node, nodes) { - var self = this; - return this._exportSpecifier(function () { - return self._push(node); - }, specifier, node, nodes); -}; diff --git a/lib/6to5/transformation/modules/common.js b/lib/6to5/transformation/modules/common.js index d8be6f82d5..de5ff6b1d7 100644 --- a/lib/6to5/transformation/modules/common.js +++ b/lib/6to5/transformation/modules/common.js @@ -10,7 +10,10 @@ var _ = require("lodash"); var visitor = { enter: function (node, parent, scope, context, state) { - if (t.isExportDeclaration(node) && !node.default) state.hasNonDefaultExports = true; + if (t.isExportDeclaration(node) && !node.default) { + state.hasNonDefaultExports = true; + context.stop(); + } } }; @@ -29,11 +32,10 @@ util.inherits(CommonJSFormatter, DefaultFormatter); CommonJSFormatter.prototype.importSpecifier = function (specifier, node, nodes) { var variableName = t.getSpecifierName(specifier); + var ref = this.push(node, nodes); + // import foo from "foo"; if (t.isSpecifierDefault(specifier)) { - var ref = util.template("require", { - MODULE_NAME: node.source - }); if (!_.contains(this.file.dynamicImported, node)) { ref = t.callExpression(this.file.addHelper("interop-require"), [ref]); } @@ -45,17 +47,18 @@ CommonJSFormatter.prototype.importSpecifier = function (specifier, node, nodes) t.variableDeclarator( variableName, t.callExpression(this.file.addHelper("interop-require-wildcard"), [ - t.callExpression(t.identifier("require"), [node.source]) + ref ]) ) ])); } else { - // import foo from "foo"; - nodes.push(util.template("require-assign-key", { - VARIABLE_NAME: variableName, - MODULE_NAME: node.source, - KEY: t.getSpecifierId(specifier) - })); + // import { foo } from "foo"; + nodes.push(t.variableDeclaration("var", [ + t.variableDeclarator( + variableName, + t.memberExpression(ref, t.getSpecifierId(specifier)) + ) + ])); } } }; @@ -95,8 +98,17 @@ CommonJSFormatter.prototype.exportDeclaration = function (node, nodes) { DefaultFormatter.prototype.exportDeclaration.apply(this, arguments); }; -CommonJSFormatter.prototype.exportSpecifier = function (specifier, node, nodes) { - this._exportSpecifier(function () { - return t.callExpression(t.identifier("require"), [node.source]); - }, specifier, node, nodes); +CommonJSFormatter.prototype._push = function (node, nodes) { + var source = node.source.value; + var call = t.callExpression(t.identifier("require"), [node.source]); + + if (this.localImportOccurences[source] > 1) { + var uid = this.file.generateUidIdentifier(source); + nodes.push(t.variableDeclaration("var", [ + t.variableDeclarator(uid, call) + ])); + return uid; + } else { + return call; + } }; diff --git a/test/fixtures/transformation/es6-modules-common/exports-from/expected.js b/test/fixtures/transformation/es6-modules-common/exports-from/expected.js index 690de1d0fa..b325991de6 100644 --- a/test/fixtures/transformation/es6-modules-common/exports-from/expected.js +++ b/test/fixtures/transformation/es6-modules-common/exports-from/expected.js @@ -16,12 +16,14 @@ var _defaults = function (obj, defaults) { return obj; }; -_defaults(exports, _interopRequireWildcard(require("foo"))); +var _foo = require("foo"); -exports.foo = require("foo").foo; -exports.foo = require("foo").foo; -exports.bar = require("foo").bar; -exports.bar = require("foo").foo; -exports["default"] = require("foo").foo; -exports["default"] = require("foo").foo; -exports.bar = require("foo").bar; +_defaults(exports, _interopRequireWildcard(_foo)); + +exports.foo = _foo.foo; +exports.foo = _foo.foo; +exports.bar = _foo.bar; +exports.bar = _foo.foo; +exports["default"] = _foo.foo; +exports["default"] = _foo.foo; +exports.bar = _foo.bar; diff --git a/test/fixtures/transformation/es6-modules-common/hoist-function-exports/expected.js b/test/fixtures/transformation/es6-modules-common/hoist-function-exports/expected.js index 5a074e1496..be7a1da579 100644 --- a/test/fixtures/transformation/es6-modules-common/hoist-function-exports/expected.js +++ b/test/fixtures/transformation/es6-modules-common/hoist-function-exports/expected.js @@ -10,4 +10,4 @@ var isOdd = exports.isOdd = (function (isEven) { return function (n) { return !isEven(n); }; -})(isEven); +})(isEven); \ No newline at end of file diff --git a/test/fixtures/transformation/es6-modules-common/imports-default/expected.js b/test/fixtures/transformation/es6-modules-common/imports-default/expected.js index 1a287b128a..8ab754a5fd 100644 --- a/test/fixtures/transformation/es6-modules-common/imports-default/expected.js +++ b/test/fixtures/transformation/es6-modules-common/imports-default/expected.js @@ -4,6 +4,8 @@ var _interopRequire = function (obj) { return obj && obj.__esModule ? obj["default"] : obj; }; -var foo = _interopRequire(require("foo")); +var _foo = require("foo"); -var foo2 = _interopRequire(require("foo")); +var foo = _interopRequire(_foo); + +var foo2 = _interopRequire(_foo); \ No newline at end of file diff --git a/test/fixtures/transformation/es6-modules-common/imports-mixing/expected.js b/test/fixtures/transformation/es6-modules-common/imports-mixing/expected.js index 2ea5ac36b3..5aba49cf35 100644 --- a/test/fixtures/transformation/es6-modules-common/imports-mixing/expected.js +++ b/test/fixtures/transformation/es6-modules-common/imports-mixing/expected.js @@ -4,6 +4,8 @@ var _interopRequire = function (obj) { return obj && obj.__esModule ? obj["default"] : obj; }; -var foo = _interopRequire(require("foo")); +var _foo = require("foo"); -var xyz = require("foo").baz; +var foo = _interopRequire(_foo); + +var xyz = _foo.baz; \ No newline at end of file diff --git a/test/fixtures/transformation/es6-modules-common/imports-named/expected.js b/test/fixtures/transformation/es6-modules-common/imports-named/expected.js index 6fa7137b22..54de3191ad 100644 --- a/test/fixtures/transformation/es6-modules-common/imports-named/expected.js +++ b/test/fixtures/transformation/es6-modules-common/imports-named/expected.js @@ -1,8 +1,10 @@ "use strict"; -var bar = require("foo").bar; -var bar2 = require("foo").bar2; -var baz = require("foo").baz; -var baz2 = require("foo").bar; -var baz3 = require("foo").bar; -var xyz = require("foo").xyz; +var _foo = require("foo"); + +var bar = _foo.bar; +var bar2 = _foo.bar2; +var baz = _foo.baz; +var baz2 = _foo.bar; +var baz3 = _foo.bar; +var xyz = _foo.xyz; diff --git a/test/fixtures/transformation/es6-modules-common/overview/actual.js b/test/fixtures/transformation/es6-modules-common/overview/actual.js index 589b4bd8cc..dcc23274f9 100644 --- a/test/fixtures/transformation/es6-modules-common/overview/actual.js +++ b/test/fixtures/transformation/es6-modules-common/overview/actual.js @@ -1,10 +1,10 @@ import "foo"; import "foo-bar"; import "./directory/foo-bar"; -import foo from "foo"; -import * as foo2 from "foo"; -import {bar} from "foo"; -import {foo as bar2} from "foo"; +import foo from "foo2"; +import * as foo2 from "foo3"; +import {bar} from "foo4"; +import {foo as bar2} from "foo5"; export {test}; export var test = 5; diff --git a/test/fixtures/transformation/es6-modules-common/overview/expected.js b/test/fixtures/transformation/es6-modules-common/overview/expected.js index dfe2d85e62..5ac34f1849 100644 --- a/test/fixtures/transformation/es6-modules-common/overview/expected.js +++ b/test/fixtures/transformation/es6-modules-common/overview/expected.js @@ -16,11 +16,11 @@ require("foo-bar"); require("./directory/foo-bar"); -var foo = _interopRequire(require("foo")); +var foo = _interopRequire(require("foo2")); -var foo2 = _interopRequireWildcard(require("foo")); +var foo2 = _interopRequireWildcard(require("foo3")); -var bar = require("foo").bar; -var bar2 = require("foo").foo; +var bar = require("foo4").bar; +var bar2 = require("foo5").foo; exports.test = test; -var test = exports.test = 5; +var test = exports.test = 5; \ No newline at end of file diff --git a/test/fixtures/transformation/self-contained/full/expected.js b/test/fixtures/transformation/self-contained/full/expected.js index afb25b3150..d03280c36c 100644 --- a/test/fixtures/transformation/self-contained/full/expected.js +++ b/test/fixtures/transformation/self-contained/full/expected.js @@ -20,8 +20,10 @@ var giveWord = _regeneratorRuntime.mark(function giveWord() { }); exports.giveWord = giveWord; -var foo = _to5Helpers.interopRequire(require("someModule")); +var _someModule = require("someModule"); -var bar = _to5Helpers.interopRequireWildcard(require("someModule")); +var foo = _to5Helpers.interopRequire(_someModule); -var myWord = exports.myWord = _core.Symbol("abc"); +var bar = _to5Helpers.interopRequireWildcard(_someModule); + +var myWord = exports.myWord = _core.Symbol("abc"); \ No newline at end of file