-
Notifications
You must be signed in to change notification settings - Fork 649
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
Parse Member and TypeCast expressions in super class types #1408
Conversation
Hi @xleoooo! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
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.
In addition to the inline comments please add test cases.
I'm also curious what you are using this tool chain for?
tools/hermes-parser/js/flow-api-translator/src/flowToFlowDef.js
Outdated
Show resolved
Hide resolved
Still working on those test cases, I'm having some trouble with jest, but was hoping to get a review on the source changes in the meantime.
|
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.
tools/hermes-parser/js/flow-api-translator/src/flowToFlowDef.js
Outdated
Show resolved
Hide resolved
tools/hermes-parser/js/flow-api-translator/src/flowToFlowDef.js
Outdated
Show resolved
Hide resolved
tools/hermes-parser/js/flow-api-translator/src/flowToFlowDef.js
Outdated
Show resolved
Hide resolved
yes, with some small caveat: some of the current RN source isn't valid input.
AFAIK this is valid flow source code, but neither of the following typedefs are OK based on flow playground
So, for type cast super classes, they are only valid for |
398eafb
to
c999d49
Compare
c999d49
to
892decf
Compare
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
@huntie has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
892decf
to
e26c61b
Compare
@xleoooo has updated the pull request. You must reimport the pull request before landing. |
Summary: Previously, member expressions in parent classes like `class A extends Foo.Bar`, and type casts such as `class A extends (Foo: typeof Bar)`, were not parseable by flow-api-translator. This change adds support.
e26c61b
to
c6a37ed
Compare
@xleoooo has updated the pull request. You must reimport the pull request before landing. |
@huntie has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
…1408) Summary: Previously, member expressions in parent classes like `class A extends Foo.Bar`, and type casts such as `class A extends (Foo: Bar)`, were not parseable by flow-api-translator. This change adds support. Pull Request resolved: facebook#1408 Test Plan: Previously unparseable files in `react-native` are now parseable Reviewed By: pieterv Differential Revision: D58577282 Pulled By: huntie fbshipit-source-id: 437f43841c2bd7eed7f9ca412984203e598ae1a9
…1408) Summary: Original Author: [email protected] Original Git: 14d3201 Original Reviewed By: pieterv Original Revision: D58577282 Previously, member expressions in parent classes like `class A extends Foo.Bar`, and type casts such as `class A extends (Foo: Bar)`, were not parseable by flow-api-translator. This change adds support. Pull Request resolved: #1408 Pulled By: huntie Reviewed By: avp Differential Revision: D61574236 fbshipit-source-id: 29ef06ffe577befa7e03b8db413e28ddac29bdfb
Summary
Previously, member expressions in parent classes like
class A extends Foo.Bar
, and type casts such asclass A extends (Foo: Bar)
, were not parseable by flow-api-translator. This change adds support.Test Plan
Previously unparseable files in
react-native
are now parseable