Skip to content

Conversation

liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Sep 1, 2024

Q                       A
Fixed Issues? #16456
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link babel/website#2965
Any Dependency Changes?
License MIT

@liuxingbaoyu liuxingbaoyu added PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release area: flow pkg: parser labels Sep 1, 2024
@babel-bot
Copy link
Collaborator

babel-bot commented Sep 1, 2024

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

@nicolo-ribaudo
Copy link
Member

We can also do this in the next minor

@nicolo-ribaudo nicolo-ribaudo added PR: New Feature (next major) 🚀 A type of pull request used for our changelog categories for next major release and removed PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release labels Sep 1, 2024
@liuxingbaoyu
Copy link
Member Author

Isn't this a breaking change?

@nicolo-ribaudo
Copy link
Member

In which case can it break some existing usage?

Comment on lines 19 to 29
if (process.env.BABEL_8_BREAKING) {
if (enums !== undefined) {
throw new Error(
"The .enums option has been removed and it's now always enabled. Please remove it from your config.",
);
}
} else {
if (typeof enums !== "boolean" && typeof enums !== "undefined") {
throw new Error(".enums must be a boolean, or undefined");
}
}
Copy link
Member Author

@liuxingbaoyu liuxingbaoyu Sep 1, 2024

Choose a reason for hiding this comment

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

Now the enums option will have no effect, and we also explicitly throw an error here to align with allowDeclareFields of TS.

Copy link
Member

Choose a reason for hiding this comment

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

We can keep the error 8-only, but enable the feature in Babel 7 too.

This ways users can start migrating now by removing the config option.

@nicolo-ribaudo nicolo-ribaudo added this to the v8.0.0-beta milestone Sep 3, 2024
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

This looks good, but in the next minor we should enable the option by default.

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 wasn't sure the purpose of this test so just removed it.

Copy link
Member

Choose a reason for hiding this comment

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

I think it was to make sure that we can parse valid non-TS syntax that would be invalid in TS files.

@nicolo-ribaudo nicolo-ribaudo added PR: Docs 📝 A type of pull request used for our changelog categories PR: Needs Docs and removed PR: Docs 📝 A type of pull request used for our changelog categories labels Sep 16, 2024
@nicolo-ribaudo nicolo-ribaudo merged commit 60b9670 into babel:main Sep 16, 2024
52 checks passed
@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, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: flow outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: New Feature (next major) 🚀 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.

3 participants