[BUGFIX] Fix Babel transform interactions with other transforms #53
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While working on some quality of life improvements in the repo, I found some buggy interactions between the Babel transform and other transforms. Specifically, the `@babel/preset-typescript` transform and the `@glimmer/babel-plugin-glimmer-env` transform. In the first case, the TypeScript transform removes all imports bindings that no longer have references once types are removed, and it does this _before_ our transform has a chance to run. This results in imported values being removed, even though they are used in templates. In the case of the `@glimmer/babel-plugin-glimmer-env`, referencing the Program path directly causes errors when it goes to _replace_ references to the `DEBUG` binding. This PR updates the logic to create an empty path that is added to the AST and referenced properly, so TS doesn't remove values and `@glimmer/env` can "replace" it correctly. We also avoid adding this extra reference to any _type_ bindings, because then they _won't_ be removed. The alternative would be to skip the queue by using the `Program` to iterate the AST, but that seems like it would be more brittle than this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While working on some quality of life improvements in the repo, I found
some buggy interactions between the Babel transform and other
transforms. Specifically, the
@babel/preset-typescripttransform andthe
@glimmer/babel-plugin-glimmer-envtransform.In the first case, the TypeScript transform removes all imports bindings
that no longer have references once types are removed, and it does this
before our transform has a chance to run. This results in imported
values being removed, even though they are used in templates.
In the case of the
@glimmer/babel-plugin-glimmer-env, referencing theProgram path directly causes errors when it goes to replace references
to the
DEBUGbinding.This PR updates the logic to create an empty path that is added to the
AST and referenced properly, so TS doesn't remove values and
@glimmer/envcan "replace" it correctly. We also avoid adding thisextra reference to any type bindings, because then they won't be
removed.
The alternative would be to skip the queue by using the
Programtoiterate the AST, but that seems like it would be more brittle than this.