Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Strips Flow types if React is enabled #1119

Closed
wants to merge 1 commit into from

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Oct 31, 2017

Release note: none

With the reactEnabled option turned on, Flow now get parsed by Babylon, which can break the output. So this PR traverses the AST after serialization and strips all Flow types using the same method from the core Babel strip Flow types plugin to ensure the Flow types aren't in the final output.


// taken directly from Babel:
// https://github.com/babel/babel/blob/master/packages/babel-plugin-transform-flow-strip-types/src/index.js
export function stripFlowTypeAnnotations(ast: BabelNode): void {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is all taken from Babel

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just use the code from the upstream package rather than copy then?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we can't use the code as is by reference, we should document the exact differences but we also need to include copyright header (which is McKenzie I guess).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't use it from upstream but I'll make make those changes in another PR.

Copy link
Contributor

@jeffreytan81 jeffreytan81 left a comment

Choose a reason for hiding this comment

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

LGTM.
High level, why leaving the react flow types parsed by babel will break Prepack react compiler?

@trueadm
Copy link
Contributor Author

trueadm commented Oct 31, 2017

@yinghuitan They don't break the compiler, they just result in invalid output after which Babel can't parse.

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@trueadm is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@sebmarkbage
Copy link
Contributor

I see you already landed, but I'd like to request a follow up. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants