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

[Discussion] Remove flow support from @babel/parser #16264

Open
nicolo-ribaudo opened this issue Feb 6, 2024 · 10 comments
Open

[Discussion] Remove flow support from @babel/parser #16264

nicolo-ribaudo opened this issue Feb 6, 2024 · 10 comments

Comments

@nicolo-ribaudo
Copy link
Member

This is a very open-ended discussion, since it might be a too big breaking change.

In the past couple years, Flow kept evolving but new features have never been added to @babel/parser. This used to be done mostly by PR by the Flow team themselves, and sometimes by us from the Babel team.

I was reading the release post for Flow's new as type casts support, and I finally discovered why (note: if I were a flow user, I would probably have discovered much earlier 😅). The Flow team maintains their own parser, that can be used in Babel via babel-plugin-syntax-hermes-parser.

I think we should consider just dropping support for Flow in our own parser, and instead officially recommend their plugin in our docs. @babel/plugin-syntax-flow has 12M weekly downloads (https://www.npmjs.com/package/@babel/plugin-syntax-flow), and arguably all these users should migrate to the official parser which currently has 8k downloads per week.

Maybe we should also drop our @babel/preset-flow, or upgrade it to use babel-plugin-syntax-hermes-parser?

Pinging a bunch of random people that will probably have something to contribute to the discussion given the projects y'all work on :)

@sosukesuzuki
Copy link
Member

I think the Prettier team is fine with removing Flow support from Babel. This is because support for using the official flow-parser remains.

@tsapeta
Copy link

tsapeta commented Feb 7, 2024

Same for the Expo team. We don't use Flow at all, we bet on TypeScript 🙂 It's mostly the question to folks from React Native team where Flow is used extensively.
Besides that, it sounds like a good idea 👍

@kelset
Copy link

kelset commented Feb 7, 2024

Microsoft-wise, I think we are fine with the approach you are proposing, like Expo we are all in on TS 👍

Let me ping a few more Meta folks so that we can make sure they will reply here.

@robhogan
Copy link

robhogan commented Feb 7, 2024

From the React Native / Meta JS infra perspective this sounds fine to me too - indeed we use Hermes parser for Flow (almost) everywhere now.

@SamChou19815
Copy link

babel-plugin-syntax-hermes-parser works by parsing with hermes-parser, and then replacing some of the newer syntax AST nodes that Babel doesn't know into any node. It doesn't do stripping of all types yet, although it can be done. We haven't done it yet since there are still some unfortunate transform code that will look at type annotations and do something different.

It will probably not be a problem if Babel stopped Flow's parsing support, but it will break things if you also removed those AST nodes and relevant type stripping transforms.

@pieterv
Copy link

pieterv commented Feb 7, 2024

@nicolo-ribaudo Its not clear to me what the proposal is here? Is it to just remove the parsing logic for Flow but maintain existing AST node, transformation, scope analysis and printing support or to strip all support for Flow syntax? Like @SamChou19815 said, the former is probably fine, the latter will break a large number of existing tools. e.g. React doc gen, ReactNative codegen.

@nicolo-ribaudo
Copy link
Member Author

Yes, parsing only. Our flow parsing logic is effectively not maintained and there is a better alternative, so I was wondering whether we still need our parser implementation.

One option for now might be to add an option to @babel/preset-flow to use the Hermes-based parser, to start encouraging mass migration and see if there are any problems.

@liuxingbaoyu
Copy link
Member

I'm not sure what to do with the generator, it has always been consistent with the logic of the parser, and I'm worried that the generator will bring new complexity.
Additionally, the issues also exist in AST definitions and generators, which have not been updated.
If one day we have to add a new AST definition to make it work, I'm worried that overall it won't be easier than it is now.

@pieterv
Copy link

pieterv commented Feb 7, 2024

One option for now might be to add an option to @babel/preset-flow to use the Hermes-based parser

Ok that makes sense and seems reasonable. There are a couple of issues i could see coming up with hermes-parser:

  • Our Babel AST is not fully consistent with the latest version of babel. We have versions of babel 7.2.0 around at Meta but the latest version of babel has naturally changed a bit, so we just pick an arbitrary AST format in between. We probably should allow a version to be passed so we can more precisely match the used version of babel.
  • We currently don't do comment attachment but i think we have a reasonable path forward here by reusing the prettier attachment logic like we do for our ESTree version of the AST.

@nicolo-ribaudo
Copy link
Member Author

For now we released 7.24.0 with a experimental_useHermesParser option in @babel/preset-flow, hoping to get some more feedback if there are any problems.

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

No branches or pull requests

8 participants