-
Notifications
You must be signed in to change notification settings - Fork 49.9k
Revert [eprh] Remove hermes-parser #34747
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
Adds back HermesParser to eslint-plugin-react-hooks. There are still [external users of Flow](#34719 (comment)) using the plugin, so we shouldn't break the plugin for them. However, we still have the problem of double parsing: once from eslint (which we discard) and then another via babel/hermes parser. In the long run we should investigate a translation layer from estree to babel (or alternatively, update the compiler to take estree as input). For now I am reverting that PR. This does mean that [Sandpack in react.dev](https://github.com/reactjs/react.dev/blob/11cb6b591571caf5fa2a192117b6a6445c3f2027/src/components/MDX/Sandpack/runESLint.tsx#L31) cannot update to the latest eprh as HermesParser does not appear to be able to be run in a browser. I discovered this while trying to update eprh on react.dev last week, but didn't investigate deeply. I'll need to double check that again to find out more.
|
Comparing: c786258...8de3085 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
josephsavona
left a comment
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.
Thanks for reverting, our Flow users are pretty quiet but it's great to see folks using React with Flow (and hopefully taking advantage of component/hook syntax and render types).
|
Thanks for considering your outside Flow users! There are dozens of us! 😅
We started using React+Flow in musicbrainz-server in 2015, when TypeScript wasn't a monopoly yet. It felt logical since React itself used Flow (and the focus on soundness was appealing). Today we have 100K+ lines of Flow, and no real desire to port those to TS. Flow still works great for us, outside of lacking up-to-date libdefs. We are taking huge advantage of the new |
Adds back HermesParser to eslint-plugin-react-hooks. There are still [external users of Flow](#34719 (comment)) using the plugin, so we shouldn't break the plugin for them. However, we still have the problem of double parsing: once from eslint (which we discard) and then another via babel/hermes parser. In the long run we should investigate a translation layer from estree to babel (or alternatively, update the compiler to take estree as input). But for now, I am reverting the PR. This does mean that [Sandpack in react.dev](https://github.com/reactjs/react.dev/blob/11cb6b591571caf5fa2a192117b6a6445c3f2027/src/components/MDX/Sandpack/runESLint.tsx#L31) cannot update to the latest eprh as HermesParser does not appear to be able to be run in a browser. I discovered this while trying to update eprh on react.dev last week, but didn't investigate deeply. I'll need to double check that again to find out more. DiffTrain build for [b65e6fc](b65e6fc)
Adds back HermesParser to eslint-plugin-react-hooks. There are still [external users of Flow](#34719 (comment)) using the plugin, so we shouldn't break the plugin for them. However, we still have the problem of double parsing: once from eslint (which we discard) and then another via babel/hermes parser. In the long run we should investigate a translation layer from estree to babel (or alternatively, update the compiler to take estree as input). But for now, I am reverting the PR. This does mean that [Sandpack in react.dev](https://github.com/reactjs/react.dev/blob/11cb6b591571caf5fa2a192117b6a6445c3f2027/src/components/MDX/Sandpack/runESLint.tsx#L31) cannot update to the latest eprh as HermesParser does not appear to be able to be run in a browser. I discovered this while trying to update eprh on react.dev last week, but didn't investigate deeply. I'll need to double check that again to find out more. DiffTrain build for [b65e6fc](b65e6fc)
|
I have uploaded a PR to fix the issue where the flow link in the PR template is incorrectly linked, preventing access to the official flow website. Could you please check it? |
Adds back HermesParser to eslint-plugin-react-hooks. There are still external users of Flow using the plugin, so we shouldn't break the plugin for them. However, we still have the problem of double parsing: once from eslint (which we discard) and then another via babel/hermes parser.
In the long run we should investigate a translation layer from estree to babel (or alternatively, update the compiler to take estree as input). But for now, I am reverting the PR.
This does mean that Sandpack in react.dev cannot update to the latest eprh as HermesParser does not appear to be able to be run in a browser. I discovered this while trying to update eprh on react.dev last week, but didn't investigate deeply. I'll need to double check that again to find out more.