Skip to content

improve node type definitions to avoid any's in generated types#11687

Merged
nicolo-ribaudo merged 4 commits intobabel:masterfrom
zxbodya:babel-types-fix-anys
Jun 8, 2020
Merged

improve node type definitions to avoid any's in generated types#11687
nicolo-ribaudo merged 4 commits intobabel:masterfrom
zxbodya:babel-types-fix-anys

Conversation

@zxbodya
Copy link
Copy Markdown
Contributor

@zxbodya zxbodya commented Jun 6, 2020

Updates node type definitions in babel-types, to avoid any in generated types.

Q                       A
Fixed Issues? #11474
Patch: Bug Fix? yes(improve generate types)
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci bot commented Jun 6, 2020

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 7470c76:

Sandbox Source
focused-perlman-jdd26 Configuration
sharp-cookies-6uog7 Configuration

@nicolo-ribaudo nicolo-ribaudo added area: typescript pkg: types PR: Bug Fix 🐛 A type of pull request used for our changelog categories labels Jun 6, 2020
@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Jun 6, 2020

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

value: {
validate: assertNodeType("StringLiteral"),
},
},
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.

This is technically a breaking change, but this node is so new that I'd be fine with considering it a bug fix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think generally such changes should be considered a bug fix because the validator should try its best to align with the parser behaviour.

@zxbodya zxbodya force-pushed the babel-types-fix-anys branch from d05cfaa to 7470c76 Compare June 8, 2020 21:34
@zxbodya
Copy link
Copy Markdown
Contributor Author

zxbodya commented Jun 8, 2020

rebased it on current master(resolving conflicts because of prettier update)

@nicolo-ribaudo
Copy link
Copy Markdown
Member

Thanks!

Comment on lines +282 to +285
comments: {
validate: assertEach(assertNodeType("Comment")),
optional: true,
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just FYI this seems to have broken Jests toMatchInlineSnapshot matcher for tests written in TypeScript (jestjs/jest#10208)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good catch, this is not intended, effect… probably this should be considered a breaking change and so to go to babel 8

@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 Sep 27, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: typescript outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: types PR: Bug Fix 🐛 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants