Skip to content

perf(react): improve react compiler preset so that slightly more modules are filtered out #1138

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.

Sign up for GitHub

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

Open
sapphi-red wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf(react): improve react compiler preset so that slightly more modules are filtered out #1138

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/plugin-react/src/reactCompilerPreset.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const reactCompilerPreset = (
code:
options.compilationMode === 'annotation'
? /['"]use memo['"]/
: /\b[A-Z]|\buse/,
: /\b[A-Z]|\buse[A-Z0-9]/,
Copy link
Member

@ArnaudBarre ArnaudBarre Mar 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

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

Suggested change
: /\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

Copy link
Member Author

@sapphi-red sapphi-red Mar 5, 2026

Choose a reason for hiding this comment

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

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 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

It's currently difficult to achieve, but it would be easier to do once Vite implements composable filters (vitejs/rolldown-vite#605).

Copy link
Member

@ArnaudBarre ArnaudBarre Mar 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@sapphi-red sapphi-red Mar 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

The way to break it is using new Function or 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.

Copy link
Member

@ArnaudBarre ArnaudBarre Mar 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

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

},
applyToEnvironmentHook: (env) => env.config.consumer === 'client',
optimizeDeps: {
Expand Down
Loading