Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: register injected importDeclaration #10172

merged 2 commits into from Jul 6, 2019


Copy link

@JLHwung JLHwung commented Jul 5, 2019

Q                       A
Fixed Issues? REPL
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?
Documentation PR Link
Any Dependency Changes?
License MIT

When transform-typescript combines with proposal-export-default-from, the export { _foo as foo }; will be erased as it seems that _foo is not defined in its own scope. This PR register the injected importDeclaration so that it can work well with elideImports from transform-typescript.

Note that #7592 uses scope.crawl() to manually sync nodes to AST. As crawl will incur great performance overkill, our approach check the injected import specifier only and register when necessary.

Copy link

@babel-bot babel-bot commented Jul 5, 2019

Build successful! You can test your changes in the REPL here:

@JLHwung JLHwung force-pushed the JLHwung:fix-export-default-from branch from eac1b1c to c301efb Jul 5, 2019
@JLHwung JLHwung force-pushed the JLHwung:fix-export-default-from branch from c301efb to c092c1e Jul 5, 2019
@JLHwung JLHwung changed the title fix: export should not be erased when mixed with `proposal-export-def… fix: register injected importDeclaration Jul 5, 2019
@nicolo-ribaudo nicolo-ribaudo added this to the v7.5.2 milestone Jul 6, 2019
@@ -35,6 +35,11 @@ export default declare(api => {

ImportDefaultSpecifier(path) {
if (!path.scope.hasOwnBinding( {

This comment has been minimized.


nicolo-ribaudo Jul 6, 2019

I would prefer to do this as soon as the import declaration is created, in the ExportNamedDeclaration visitor. This makes it clearer why we are doing it, and it prevents other plugin from running between the creating the declaration and registering it.

path.replaceWithMultiple(nodes); returns an array containing the new inserted paths: the first one should be the import declaration, and you can get it's default specifier.

This comment has been minimized.


JLHwung Jul 6, 2019
Author Contributor

path.replaceWithMultiple(nodes) returns an array containing the new inserted paths.

Yes it is. It seems that @types/babel__traverse declares out-of-dated signature here.

@existentialism existentialism merged commit 075a502 into babel:master Jul 6, 2019
5 checks passed
5 checks passed
babel/repl REPL preview is available
buildsize No significant change
ci/circleci Your tests passed on CircleCI!
codecov/project 87.56% (target 80%)
continuous-integration/travis-ci/pr The Travis CI build passed
@JLHwung JLHwung deleted the JLHwung:fix-export-default-from branch Jul 30, 2019
@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants