From 4ca25ac7a9eab7098c8bf03648d157123fab077a Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 12 Jan 2016 18:01:31 -0500 Subject: [PATCH 1/2] Add test of import hoisting that fails with the runtime transform. --- .../test/fixtures/interop/imports-hoisting/actual.js | 1 + .../fixtures/interop/imports-hoisting/expected.js | 11 +++++++++++ .../fixtures/interop/imports-hoisting/options.json | 7 +++++++ 3 files changed, 19 insertions(+) create mode 100644 packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/actual.js create mode 100644 packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/expected.js create mode 100644 packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/options.json diff --git a/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/actual.js b/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/actual.js new file mode 100644 index 0000000000..ebbb389139 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/actual.js @@ -0,0 +1 @@ +tag`foo`; diff --git a/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/expected.js b/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/expected.js new file mode 100644 index 0000000000..86a1f39430 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/expected.js @@ -0,0 +1,11 @@ +"use strict"; + +var _taggedTemplateLiteral2 = require("babel-runtime/helpers/taggedTemplateLiteral"); + +var _taggedTemplateLiteral3 = _interopRequireDefault(_taggedTemplateLiteral2); + +var _templateObject = (0, _taggedTemplateLiteral3.default)(["foo"], ["foo"]); + +function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } + +tag(_templateObject); diff --git a/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/options.json b/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/options.json new file mode 100644 index 0000000000..662e998a43 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/imports-hoisting/options.json @@ -0,0 +1,7 @@ +{ + "plugins": [ + "transform-runtime", + "transform-es2015-template-literals", + "transform-es2015-modules-commonjs" + ] +} From e1ec9ef985dd49c8baa3db684875d84d2f78ebda Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 12 Jan 2016 18:01:31 -0500 Subject: [PATCH 2/2] Make require declarations inherit ._blockHoist values from imports. This commit fixes the failing test I introduced in pull request #3118. --- .../src/index.js | 79 +++++++++++++++---- 1 file changed, 64 insertions(+), 15 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-modules-commonjs/src/index.js b/packages/babel-plugin-transform-es2015-modules-commonjs/src/index.js index ed744a5815..2a4d90c138 100644 --- a/packages/babel-plugin-transform-es2015-modules-commonjs/src/index.js +++ b/packages/babel-plugin-transform-es2015-modules-commonjs/src/index.js @@ -157,15 +157,23 @@ export default function () { let requires = Object.create(null); - function addRequire(source) { + function addRequire(source, blockHoist) { let cached = requires[source]; if (cached) return cached; let ref = path.scope.generateUidIdentifier(basename(source, extname(source))); - topNodes.push(t.variableDeclaration("var", [ - t.variableDeclarator(ref, buildRequire(t.stringLiteral(source)).expression) - ])); + let varDecl = t.variableDeclaration("var", [ + t.variableDeclarator(ref, buildRequire( + t.stringLiteral(source) + ).expression) + ]); + + if (typeof blockHoist === "number" && blockHoist > 0) { + varDecl._blockHoist = blockHoist; + } + + topNodes.push(varDecl); return requires[source] = ref; } @@ -190,7 +198,24 @@ export default function () { if (path.isImportDeclaration()) { hasImports = true; - addTo(imports, path.node.source.value, path.node.specifiers); + + let key = path.node.source.value; + let importsEntry = imports[key] || { + specifiers: [], + maxBlockHoist: 0 + }; + + importsEntry.specifiers.push(...path.node.specifiers); + + if (typeof path.node._blockHoist === "number") { + importsEntry.maxBlockHoist = Math.max( + path.node._blockHoist, + importsEntry.maxBlockHoist + ); + } + + imports[key] = importsEntry; + path.remove(); } else if (path.isExportDefaultDeclaration()) { let declaration = path.get("declaration"); @@ -262,7 +287,7 @@ export default function () { let nodes = []; let source = path.node.source if (source) { - let ref = addRequire(source.value); + let ref = addRequire(source.value, path.node._blockHoist); for (let specifier of specifiers) { if (specifier.isExportNamespaceSpecifier()) { @@ -288,16 +313,16 @@ export default function () { } else if (path.isExportAllDeclaration()) { topNodes.push(buildExportAll({ KEY: path.scope.generateUidIdentifier("key"), - OBJECT: addRequire(path.node.source.value) + OBJECT: addRequire(path.node.source.value, path.node._blockHoist) })); path.remove(); } } for (let source in imports) { - let specifiers = imports[source]; + let {specifiers, maxBlockHoist} = imports[source]; if (specifiers.length) { - let uid = addRequire(source); + let uid = addRequire(source, maxBlockHoist); let wildcard; @@ -307,9 +332,21 @@ export default function () { if (strict) { remaps[specifier.local.name] = uid; } else { - topNodes.push(t.variableDeclaration("var", [ - t.variableDeclarator(specifier.local, t.callExpression(this.addHelper("interopRequireWildcard"), [uid])) - ])); + const varDecl = t.variableDeclaration("var", [ + t.variableDeclarator( + specifier.local, + t.callExpression( + this.addHelper("interopRequireWildcard"), + [uid] + ) + ) + ]); + + if (maxBlockHoist > 0) { + varDecl._blockHoist = maxBlockHoist; + } + + topNodes.push(varDecl); } wildcard = specifier.local; } else if (t.isImportDefaultSpecifier(specifier)) { @@ -325,9 +362,21 @@ export default function () { target = wildcard; } else { target = wildcard = path.scope.generateUidIdentifier(uid.name); - topNodes.push(t.variableDeclaration("var", [ - t.variableDeclarator(target, t.callExpression(this.addHelper("interopRequireDefault"), [uid])) - ])); + const varDecl = t.variableDeclaration("var", [ + t.variableDeclarator( + target, + t.callExpression( + this.addHelper("interopRequireDefault"), + [uid] + ) + ) + ]); + + if (maxBlockHoist > 0) { + varDecl._blockHoist = maxBlockHoist; + } + + topNodes.push(varDecl); } } remaps[specifier.local.name] = t.memberExpression(target, specifier.imported);