-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
[Babel 8] Remove enums
option of flow plugin
#16792
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
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/57947 |
37fc336
to
8a5e70e
Compare
We can also do this in the next minor |
Isn't this a breaking change? |
In which case can it break some existing usage? |
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"); | ||
} | ||
} |
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.
Now the enums
option will have no effect, and we also explicitly throw an error here to align with allowDeclareFields
of TS.
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 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.
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.
This looks good, but in the next minor we should enable the option by default.
405c91f
to
18a0b16
Compare
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.
I wasn't sure the purpose of this test so just removed it.
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.
I think it was to make sure that we can parse valid non-TS syntax that would be invalid in TS files.
79b9393
to
3f25688
Compare
Uh oh!
There was an error while loading. Please reload this page.