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

(opt-in) Improve @babel/types with env.BABEL_TYPES_8_BREAKING #10917

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Dec 23, 2019

Q                       A
Fixed Issues? Closes #5971
Patch: Bug Fix?
Major: Breaking Change? Yes, but opt-in.
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

⚠️ This PR isn't actually breaking, but I used that label for changelog reasons.

As discussed during the Dec 04 meeting, I'm introducing the stricter AST validation of #5971 behind a flag.

We decided to introduce those breaking changes when the process.env.BABEL_TYPES_8_BREAKING env variable is enabled, to give time to some popular projects (like react-native) to update their code before the Babel 8 release, to provide a smoother update experience to our users.

I didn't include the changes related to the default value of decorators (since they are still highly experimental, I don't think we should pollute the AST unless the parser plugin is enabled). Also, I didn't include the validateDeep logic, but we can introduce it later in our test runner.

Our tests are already passing when BABEL_TYPES_8_BREAKING=true.

@nicolo-ribaudo nicolo-ribaudo added PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release pkg: types labels Dec 23, 2019
Co-authored-by: Justin Ridgewell <justin+github@ridgewell.name>
@nicolo-ribaudo nicolo-ribaudo force-pushed the cleanup-babel-types-babel-8-breaking branch from 60d964c to 9c15a55 Compare December 23, 2019 23:52
@nicolo-ribaudo nicolo-ribaudo added this to the v7.8.0 milestone Dec 23, 2019
export function assertClassExpression(node: Object, opts?: Object = {}): void {
assert("ClassExpression", node, opts);
}
export function assertClassDeclaration(node: Object, opts?: Object = {}): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think they are sorted alphabetically?

Copy link
Member Author

Choose a reason for hiding this comment

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

This file is automatically generated. I had to swap then in the definitions/es2015 file, so they got swapped here.

fields: {
// todo
},
fields: !process.env.BABEL_TYPES_8_BREAKING
Copy link
Member Author

Choose a reason for hiding this comment

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

I used ! everywhere so that, when deleting it for Babel 8, I only have to make one selection (from ! to :) rather than two (from ! to ? and then from : to the end).

Dev-optimization? 😄

@nicolo-ribaudo nicolo-ribaudo added the PR: Needs Review A pull request awaiting more approvals label Jan 1, 2020
@nicolo-ribaudo nicolo-ribaudo removed the PR: Needs Review A pull request awaiting more approvals label Jan 11, 2020
@nicolo-ribaudo nicolo-ribaudo merged commit e7b80a2 into babel:master Jan 11, 2020
@nicolo-ribaudo nicolo-ribaudo deleted the cleanup-babel-types-babel-8-breaking branch January 11, 2020 09:34
@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 Apr 12, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: types PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants