Skip to content

Throw a better error when transforming imported bindings in types#13739

Merged
nicolo-ribaudo merged 3 commits intobabel:mainfrom
nicolo-ribaudo:better-error-module-transforms-in-type-annotation
Sep 16, 2021
Merged

Throw a better error when transforming imported bindings in types#13739
nicolo-ribaudo merged 3 commits intobabel:mainfrom
nicolo-ribaudo:better-error-module-transforms-in-type-annotation

Conversation

@nicolo-ribaudo
Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo commented Sep 7, 2021

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

@nicolo-ribaudo nicolo-ribaudo added PR: Internal 🏠 A type of pull request used for our changelog categories area: errors labels Sep 7, 2021
@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Sep 7, 2021

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/48770/

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci bot commented Sep 7, 2021

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:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

if (importData) {
if (isInType(path)) {
throw path.buildCodeFrameError(
`Cannot transform the imported ${localName} binding since it's used in a type annotation. ` +
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this?

Suggested change
`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":
Copy link
Copy Markdown
Member

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)

Copy link
Copy Markdown
Member Author

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).

@nicolo-ribaudo nicolo-ribaudo force-pushed the better-error-module-transforms-in-type-annotation branch from dd82738 to d8151e3 Compare September 16, 2021 21:02
@nicolo-ribaudo nicolo-ribaudo merged commit 8fb5042 into babel:main Sep 16, 2021
@nicolo-ribaudo nicolo-ribaudo deleted the better-error-module-transforms-in-type-annotation branch September 16, 2021 21:14
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Dec 17, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: errors outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@babel/helper-module-transforms rewrites type annotations containing named imports incorrectly

5 participants