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

Resolve bare specifiers PRIOR to compilation. #3405

Closed
wants to merge 3 commits into from

Conversation

usergenic
Copy link
Contributor

There's an issue about specifiers NOT being transformed when the AMD transform is involved as reported in #3402 #3403

This PR is a possible alternative to #3404, inferring from that work that the ordering of babel transform plugins was at issue. This simply moves specifier resolution to the beginning of the js-transform sequence.

@usergenic
Copy link
Contributor Author

cc @btopro @Westbrook

@btopro
Copy link
Contributor

btopro commented Apr 24, 2019

were you able to confirm this fixes the transform issue? I believe I tried this first initially and the issue is that it doesn't process the plugins in the same order. It seems like CallExpression are processed after Program, so the other PR changes the transform statement to be a looping Program as opposed to single CallExpression that runs post-transformation.

@usergenic
Copy link
Contributor Author

@btopro OH the nature of the problem comes from the fact that our babel-plugin-dynamic-import-amd.ts is doing a path.traverse in its own Program visitor 🤦‍♂️

I'm going to try and fix that right now too.

@usergenic usergenic closed this Apr 25, 2019
@usergenic
Copy link
Contributor Author

@btopro I'm going to go with your solution in #3404

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants