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
Throw a better error when transforming imported bindings in types #13739
Throw a better error when transforming imported bindings in types #13739
Conversation
nicolo-ribaudo
commented
Sep 7, 2021
•
edited by gitpod-io
bot
edited by gitpod-io
bot
Q | A |
---|---|
Fixed Issues? | Fixes #13094 |
Patch: Bug Fix? | |
Major: Breaking Change? | |
Minor: New Feature? | |
Tests Added + Pass? | Yes |
Documentation PR Link | |
Any Dependency Changes? | |
License | MIT |
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/48770/ |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit d8151e3:
|
@@ -224,6 +243,13 @@ const rewriteReferencesVisitor: Visitor<RewriteReferencesVisitorState> = { | |||
|
|||
const importData = imported.get(localName); | |||
if (importData) { | |||
if (isInType(path)) { | |||
throw path.buildCodeFrameError( | |||
`Cannot transform the imported ${localName} binding since it's used in a type annotation. ` + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe this?
`Cannot transform the imported ${localName} binding since it's used in a type annotation. ` + | |
`Cannot transform the imported binding ${localName} since it's also used as a type annotation. ` + |
case "TSTypeAliasDeclaration": | ||
case "TSTypeReference": | ||
case "TypeAnnotation": | ||
case "TypeAlias": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't have an alias to cover all type usage for flow/ts? (or is this it anyway)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's enough to cover the nodes where the AST stops being an expression and starts being a type. The problem with a TypeScript
/Flow
catch-all is that both TS and Flow introduce some expression nodes (for example, in enums).
dd82738
to
d8151e3
Compare