perf(react): improve react compiler preset so that slightly more modules are filtered out#1138
Conversation
…les are filtered out
| options.compilationMode === 'annotation' | ||
| ? /['"]use memo['"]/ | ||
| : /\b[A-Z]|\buse/, | ||
| : /\b[A-Z]|\buse[A-Z0-9]/, |
There was a problem hiding this comment.
I think the only way to define a function without a whitespace before is const A,B= ... which I don't know why you would do that in anyworld for defining hooks or components
| : /\b[A-Z]|\buse[A-Z0-9]/, | |
| : /\s[A-Z]|\suse[A-Z0-9]/, |
But I think the better tradeoff is always running on jsx|tsx|mdx (in a normal codebase, you have like 98/99% of jsx files that contrains components, so not sure trying to get a 2% improvement with a regex is worth it) and run on js|ts only with when code matches \suse[A-Z0-9] (Vite always ban using JSX inside JS, nobody writes manual jsx calls and plugin should not rename file ids once transform from jsx to js
The only issue with the approach if for people having custom DSL that compile to React, but I think these frameworks should provide their own react compiler preset
Also can you put the link from the PR into the source code? I think it's good to have it as a reference if someone challenges it later
There was a problem hiding this comment.
If it's possible to have , before the name, I think we can use [\s,]. I guess \s and [\s,] won't have much perf difference.
I think the better tradeoff is always running on
jsx|tsx|mdx(in a normal codebase, you have like 98/99% of jsx files that contrains components, so not sure trying to get a 2% improvement with a regex is worth it) and run onjs|tsonly with when code matches\suse[A-Z0-9](Vite always ban using JSX inside JS, nobody writes manualjsxcalls and plugin should not rename file ids once transform from jsx to js
It's currently difficult to achieve, but it would be easier to do once Vite implements composable filters (vitejs/rolldown-vite#605).
There was a problem hiding this comment.
After some AI checking, I think this is pretty safe and code not matching this regex will certainly break a lot a React rules:
(?:const|let|var|function)\s+(?:[A-Z]|use[A-Z0-9])
(The way to break it is using new Function or putting your component in a class method, both sounds horrible)
Speaking of classes, I checked and the playground doesn't touch class component, so no need to match this
Edit: Ok no actually this would be valid IMO but the playground doesn't touch it:
import { useState } from "react";
const hooks = {
useTest: () => {
const [state, setState] = useState(0);
if (state === 0) return <div>Nothing</div>
else return <div>{state}</div>
}
}I think the React compiler is only looking for top-level functions, and this sounds reasonable, so we can make an more aggressive regex
There was a problem hiding this comment.
The way to break it is using
new Functionor putting your component in a class method, both sounds horrible
I guess new Function cannot be handled by the compiler anyways. Classes (& class methods) are skipped here.
I think the React compiler is only looking for top-level functions
In compilationMode: 'all', it seems to only look for top-level ones. But I guess it does otherwise.
Something like this won't match the regex you suggested, while the compiler optimizes:
import React, { useState } from "react";
import { jsx } from "react/jsx-runtime"
export const components = {
A: React.memo(() => {
const [state, setStae] = useState(0);
return jsx("div", { children: state })
})
}playground
This case matches this part.
That said, I think we can ignore this case if the speed up is big enough.
There was a problem hiding this comment.
It feels quite unexpected that the component is optimized only if wrap a in a specific function.
We could also add ["']react['"] to the regex in that case, it should cover a lot of extra cases like this while still skipping files that are unrelated to React
I noticed that we can add
[A-Z0-9]to filter out a bit more modules.https://github.com/facebook/react/blob/9c0323e2cf9be543d6eaa44419598af56922603f/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts#L897-L899