feat(react): add native React Compiler support - #1419
Conversation
0aaf260 to
89c2017
Compare
ArnaudBarre
left a comment
There was a problem hiding this comment.
I'm wondering if we would not have better stability by always using oxc-transform-react. You could then remove react refresh from the builtin oxc-transform, making it a lot more vendor neutral.
That's a trade off, because it would be a bit slower for people not using the compiler, but in my experience the overhead is not visible in real Vite application (and in build mode without compiler, the builtin transform could still be used)
| compiler ?? (await loadCompiler((message) => this.error(message))) | ||
|
|
||
| const result = await transform(id.split('?')[0]!, code, { | ||
| jsx: 'preserve', |
There was a problem hiding this comment.
Having this plugin doing this transformation with JSX preserve and then the builtin rolldown doing the jsx transformation will make the jsxDev output have wrong line numbers. This breaks various plugin and browser extensions that are using this to jump from the client to the editor in dev mode.
There was a problem hiding this comment.
Re both comments:
oxc-transform-react is intended to provide or react related transforms.
For @vitejs/plugin-react, I want to keep the blast radius minimal so it only uses the react compiler transform from oxc-transform-react.
Having this plugin doing this transformation with JSX preserve and then the builtin rolldown doing the jsx transformation will make the jsxDev output have wrong line numbers.
Is this a bug that I should fix? Does it happen with babel react compiler as well?
There was a problem hiding this comment.
It happens when people configure Babel to only use the react compiler, but it can be fixed by telling Babel to also handle jsx transform, which can't be done with the current setup
| "peerDependencies": { | ||
| "@rolldown/plugin-babel": "^0.1.7 || ^0.2.0", | ||
| "babel-plugin-react-compiler": "^1.0.0", | ||
| "oxc-transform-react": "^0.144.0", |
There was a problem hiding this comment.
Whenever, oxc bumps its minor version, we have to release plugin-react to solve the peer dep error / warning. We can live with it for a while but it's not ideal.
There was a problem hiding this comment.
Whenever, oxc bumps its minor version, we have to release plugin-react to solve the peer dep error / warning.
Can you explain how this happens?
|
Release plan: merge this integration PR, then after today's Oxc release ( |
|
@sapphi-red handing over to you and the team. |
| if (opts.compiler) { | ||
| return { | ||
| oxc: { jsx: 'preserve' }, | ||
| optimizeDeps: { | ||
| rolldownOptions: { transform: { jsx: 'preserve' } }, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we should not touch the oxc config here
- If the compiler ran, there is no jsx anymore so it doesn't matter
- If the compiler didn't run because of the include/exclude, the jsx should still be transformed to not ship it to the browser
I think the same idea also apply for the two others compiler opts.compiler conditions
There was a problem hiding this comment.
My intent is for oxc-transform-react to own React Compiler, TypeScript/JSX, and Fast Refresh in one pass. Vite/Rolldown’s React transforms are disabled to avoid duplicate transforms.
Can you advice on the correct change you are proposing?
There was a problem hiding this comment.
If the file was already transform by oxc-transform-react then there is no risk of duplicate work for the JSX transform. For fast refresh, it may indeed induce duplicated work.
It's important to not disable JSX transform all together, because there are some users that are using the include/exclude query to opt out of fast refresh transform but still expect jsx transform (when generating PDF from react component in workers for example)
Here is my suggestion that takes into account both things:
config(_userConfig, { command }) {
const refresh = command === 'serve' && opts.compiler
if (opts.jsxRuntime === 'classic') {
return {
oxc: {
jsx: { runtime: 'classic', refresh },
jsxRefreshInclude: refresh ? makeIdFiltersToMatchWithQuery(include) : undefined,
jsxRefreshExclude: refresh ? makeIdFiltersToMatchWithQuery(exclude) : undefined,
},
}
} else {
return {
oxc: {
jsx: {
runtime: 'automatic',
importSource: opts.jsxImportSource,
refresh,
},
jsxRefreshInclude: refresh ? makeIdFiltersToMatchWithQuery(include) : undefined,
jsxRefreshExclude: refresh ? makeIdFiltersToMatchWithQuery(exclude) : undefined,
},
optimizeDeps: {
rolldownOptions: { transform: { jsx: { runtime: 'automatic' } } },
},
}
}
},Co-authored-by: Arnaud Barré <arnaud.barre72@gmail.com>
| userConfig.server?.hmr, | ||
| ) | ||
| if (skipFastRefresh) { | ||
| if (skipFastRefresh && !opts.compiler) { |
There was a problem hiding this comment.
If compiler is enabled, this should be false already and I prefer to keep this condition simple instead of optimizing for one config merge in the rare usecase covered by this plugin (hmr disabled by plugin IIRC)
| if (skipFastRefresh && !opts.compiler) { | |
| if (skipFastRefresh) { |
| options.transform.jsx = opts.compiler | ||
| ? 'preserve' | ||
| : { |
There was a problem hiding this comment.
| options.transform.jsx = opts.compiler | |
| ? 'preserve' | |
| : { | |
| options.transform.jsx = { |
Add a
compileroption backed by the optionaloxc-transform-reactpackage.When enabled,
oxc-transform-reactowns the React Compiler, TypeScript/JSX, and Fast Refresh transforms in one pass, while Rolldown React transforms are disabled. React Compiler and Fast Refresh remain client-only; server environments use the same package for TypeScript/JSX.Keeping React Compiler in a separate package avoids adding framework-specific compiler size and complexity to Rolldown.