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

Flow enums parsing #10344

Merged
merged 7 commits into from Oct 29, 2019

Conversation

@gkz
Copy link
Contributor

gkz commented Aug 15, 2019

Q                       A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Yes (added behind optional flag)
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes? No
License MIT

Flow is working on adding an optional (and by default off) enums feature, gated by the experimental.enums Flowconfig flag. Parsing was implemented in the Flow Parser 1.5 months ago (facebook/flow@e3ee40c).

Here, we add support for parsing Flow Enums in the Flow babel-parser plugin, also gated by an option, enums. This brings Babel's Flow parsing in line with Flow Parser's.

Here are some examples:

enum E {
  A,
  B,
}
enum E {
  A = 1,
  B = 2,
}

For more information, see this document: https://github.com/gkz/enums

@babel-bot

This comment has been minimized.

Copy link
Collaborator

babel-bot commented Aug 15, 2019

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

@buildsize

This comment has been minimized.

Copy link

buildsize bot commented Aug 15, 2019

File name Previous Size New Size Change
babel-preset-env.js 2.36 MB 2.39 MB 27.7 KB (1%)
babel-preset-env.min.js 1.35 MB 1.36 MB 13.47 KB (1%)
babel.js 2.91 MB 2.94 MB 27.88 KB (1%)
babel.min.js 1.61 MB 1.62 MB 13.58 KB (1%)
Copy link
Member

nicolo-ribaudo left a comment

Can enums be exported? Is this valid?

export default enum {}
packages/babel-parser/src/plugins/flow.js Outdated Show resolved Hide resolved
packages/babel-parser/src/plugins/flow.js Outdated Show resolved Hide resolved
@existentialism

This comment has been minimized.

Copy link
Member

existentialism commented Aug 20, 2019

@gkz

This comment has been minimized.

Copy link
Contributor Author

gkz commented Aug 26, 2019

Thank you for reviewing @nicolo-ribaudo! I will update/respond this week

@gkz

This comment has been minimized.

Copy link
Contributor Author

gkz commented Aug 29, 2019

Will add support for parsing export of enums

@gkz

This comment has been minimized.

Copy link
Contributor Author

gkz commented Aug 30, 2019

I have updated the pull request as per comments

@nicolo-ribaudo nicolo-ribaudo added this to the v7.7.0 milestone Sep 17, 2019
@nicolo-ribaudo nicolo-ribaudo self-requested a review Oct 9, 2019
packages/babel-parser/src/plugins/flow.js Outdated Show resolved Hide resolved
packages/babel-parser/src/plugins/flow.js Outdated Show resolved Hide resolved
packages/babel-parser/src/plugins/flow.js Outdated Show resolved Hide resolved
packages/babel-parser/src/plugins/flow.js Outdated Show resolved Hide resolved
@gkz gkz requested a review from nicolo-ribaudo Oct 15, 2019
@gkz

This comment has been minimized.

Copy link
Contributor Author

gkz commented Oct 15, 2019

Not sure exactly the purpose of the "request a review" button on GitHub, but I clicked it. Actually looking at this it doesn't look like PRs need to be "approved" before they are merged, so feel free to ignore the request (if you have no additional comments)

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

nicolo-ribaudo commented Oct 15, 2019

We need to ✔️ to merge a PR (and I won't merge it until the 7.7.0 release anyway)

@nicolo-ribaudo nicolo-ribaudo merged commit ec3345b into babel:master Oct 29, 2019
3 checks passed
3 checks passed
Travis CI - Pull Request Build Passed
Details
codecov/project 88.23% (target 80%)
Details
test Workflow: test
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.