From fa1547d8f885b84d644e809dc281f323662b8da3 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Wed, 9 May 2018 17:05:29 +0200 Subject: [PATCH] fix(transform-typescript): do not elide injected imports (#7833) --- .../src/index.js | 59 ++++++++++--------- .../fixtures/imports/elide-injected/input.mjs | 1 + .../imports/elide-injected/options.json | 3 + .../imports/elide-injected/output.mjs | 2 + .../fixtures/imports/elide-injected/plugin.js | 26 ++++++++ 5 files changed, 63 insertions(+), 28 deletions(-) create mode 100644 packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/input.mjs create mode 100644 packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/options.json create mode 100644 packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/output.mjs create mode 100644 packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/plugin.js diff --git a/packages/babel-plugin-transform-typescript/src/index.js b/packages/babel-plugin-transform-typescript/src/index.js index e93f7a5b0d..e336eb9621 100644 --- a/packages/babel-plugin-transform-typescript/src/index.js +++ b/packages/babel-plugin-transform-typescript/src/index.js @@ -33,39 +33,42 @@ export default declare(api => { Program(path, state: State) { state.programPath = path; - }, - ImportDeclaration(path, state: State) { - // Note: this will allow both `import { } from "m"` and `import "m";`. - // In TypeScript, the former would be elided. - if (path.node.specifiers.length === 0) { - return; - } + // remove type imports + for (const stmt of path.get("body")) { + if (t.isImportDeclaration(stmt)) { + // Note: this will allow both `import { } from "m"` and `import "m";`. + // In TypeScript, the former would be elided. + if (stmt.node.specifiers.length === 0) { + return; + } - let allElided = true; - const importsToRemove: Path[] = []; + let allElided = true; + const importsToRemove: Path[] = []; - for (const specifier of path.node.specifiers) { - const binding = path.scope.getBinding(specifier.local.name); + for (const specifier of stmt.node.specifiers) { + const binding = stmt.scope.getBinding(specifier.local.name); - // The binding may not exist if the import node was explicitly - // injected by another plugin. Currently core does not do a good job - // of keeping scope bindings synchronized with the AST. For now we - // just bail if there is no binding, since chances are good that if - // the import statement was injected then it wasn't a typescript type - // import anyway. - if (binding && isImportTypeOnly(binding, state.programPath)) { - importsToRemove.push(binding.path); - } else { - allElided = false; - } - } + // The binding may not exist if the import node was explicitly + // injected by another plugin. Currently core does not do a good job + // of keeping scope bindings synchronized with the AST. For now we + // just bail if there is no binding, since chances are good that if + // the import statement was injected then it wasn't a typescript type + // import anyway. + if (binding && isImportTypeOnly(binding, state.programPath)) { + importsToRemove.push(binding.path); + } else { + allElided = false; + } + } - if (allElided) { - path.remove(); - } else { - for (const importPath of importsToRemove) { - importPath.remove(); + if (allElided) { + stmt.remove(); + } else { + for (const importPath of importsToRemove) { + importPath.remove(); + } + } } } }, diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/input.mjs b/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/input.mjs new file mode 100644 index 0000000000..0f2ce7ebf7 --- /dev/null +++ b/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/input.mjs @@ -0,0 +1 @@ +a(); diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/options.json b/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/options.json new file mode 100644 index 0000000000..be7a52a5c5 --- /dev/null +++ b/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/options.json @@ -0,0 +1,3 @@ +{ + "plugins": ["transform-typescript", "./plugin"] +} diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/output.mjs b/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/output.mjs new file mode 100644 index 0000000000..d049b7aa5d --- /dev/null +++ b/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/output.mjs @@ -0,0 +1,2 @@ +import local from "source"; +local(); diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/plugin.js b/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/plugin.js new file mode 100644 index 0000000000..26058a3abc --- /dev/null +++ b/packages/babel-plugin-transform-typescript/test/fixtures/imports/elide-injected/plugin.js @@ -0,0 +1,26 @@ +module.exports = function({ types: t }) { + return { + visitor: { + CallExpression(path) { + if (path.node.callee.name === "a") { + // the import we add here should not be elided + path.scope + .getProgramParent() + .path.unshiftContainer( + "body", + t.importDeclaration( + [t.importDefaultSpecifier(t.identifier("local"))], + t.stringLiteral("source") + ) + ); + + // with this crawl, babel-plugin-transform-typescript + // used to elide the import we just added + // https://github.com/babel/babel/issues/7592 + path.scope.crawl(); + path.node.callee = t.identifier("local"); + } + }, + }, + }; +};