feat(create-vite): support React Compiler #20704
Conversation
ArnaudBarre
changed the title
|
oh this is better, thank you! |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| const asObject = JSON.parse(content) | ||
| const devDepsEntries = Object.entries(asObject.devDependencies) | ||
| devDepsEntries.push([ | ||
| 'babel-plugin-react-compiler', |
There was a problem hiding this comment.
one thing that's missing is also using [email protected]. Version 6 will include compiler diagnostics as part of the plugin by default.
There was a problem hiding this comment.
That's quite a bump in dependencies. plugin-proposal-private-methods is deprecated, and I'm surprise of the need for zod in a linter plugin.
Is the hermes parser necessary given that the build will use the babel parser?
There was a problem hiding this comment.
Not as much an issue, but are you open to switching to ESM for the next major? ESLint v9 is now quite the norm and fully support ESM. I can make a PR to either reduce the dependency or switch to ESM if you think there is a chance it lands
There was a problem hiding this comment.
@ArnaudBarre yes, the eslint plugin now bundles the compiler as well since it can be used standalone. That means a few extra dependencies are needed. Is that blocking anything?
As for ESM I think that will require a larger discussion with the rest of the React team.
There was a problem hiding this comment.
The react compiler only requires @babel/types as a dependency so I'm still unsure what are the need for all these extra dependencies. It adds extra install size (zod 3 & hermes-parser probably not be present for a number of apps) and extra attack surface for package hijacking like what happen yesterday.
There was a problem hiding this comment.
@ArnaudBarre we inline most dependencies in babel-plugin-react-compiler which is why it only appears to depend on @babel/types (this is mostly a consequence of some Meta-internal infra quirks). For eslint-plugin-react-hooks we didn't, but I can inline as well if it's a hard requirement to get this shipped. Effectively the eslint plugin now includes the entire compiler as it needs to be able to be used standalone, hence the zod (validating compiler configs) and hermes-parser (parsing both Flow and JavaScript/TypeScript into a Babel compatible AST) dependencies.
There was a problem hiding this comment.
I don't think it's needed to depend on it, it's not directly used: https://github.com/search?q=repo%3Afacebook%2Freact%20path%3A%2F%5Epackages%5C%2Feslint-plugin-react-hooks%5C%2F%2F%20zod&type=code
Instead I think the eslint plugin should make babel-plugin-react-compiler a peer-dependency? (maybe optional if it's not required for some rules and lazy loaded for others)
Edit: Ok I see that the plugin rebundle the compiler, why not depend on it (to avoid having it twice). It would also make the runtime & lint rules in sync for users?
Also the current published version as two 2MB files, is it possible to only publish the development version?
There was a problem hiding this comment.
@poteto is there somewhere we could discuss this stuff outside of this PR too?
the modern approach to custom parsers in ESLint is to provide a language implementation, like how @eslint/json works. the hooks plugin shouldn't really need to care about parsing at all, since a language should've already been setup to parse the various formats correctly (in this case, flow and a custom JS parse with the compiler plugin).
this is a fairly modern part of ESLint though so makes sense the hooks plugin doesn't do it this way yet. so maybe we should track it in an issue somewhere?
|
I think there're two other ways:
Do you have any thoughts about the other ways? |
|
The compiler by default would mean we don't run OXC by default. Which can impact the boot time of the starter (I didn't yet benchmark it). This would also mean keeping Babel by default. But given what is the next version of eslint-plugin-react-hooks, we woud need to revisit eslint by default if our goal is to have a light template I think the benefit of having a template with already the compiler setup is to make it easy to quickly try a pattern on a empty app. That said, the way the template is setup forbid the usage via stackblitz which is the easiest way to create repro... |
ArnaudBarre
force-pushed
the
arnaud/create-vite-react-compiler
branch
from
bab7fb0 to
a86ad0b
Compare
ArnaudBarre
requested review from
bluwy and
sapphi-red
and removed request for
sapphi-red
|
@sapphi-red @bluwy I added a small note on performances, but not sure of the wording, open to suggestions |
5 participants
Alternative to #20702 and vitejs/vite-plugin-react#799 from @poteto which make it easier to keep the template in sync
close #20702, close vitejs/vite-plugin-react#799