-
Notifications
You must be signed in to change notification settings - Fork 5.4k
build: Enable React Compiler for Webpack builds #38007
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
base: jongsun/build/251103-enable-react-compiler
Are you sure you want to change the base?
build: Enable React Compiler for Webpack builds #38007
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
@metamaskbot update-policies |
|
No policy changes |
4df456e to
e1f1daa
Compare
|
@metamaskbot update-policies |
e1f1daa to
20b99b9
Compare
|
No policy changes |
|
@metamaskbot update-policies |
|
Policy update failed. You can review the logs or retry the policy update here |
1 similar comment
|
Policy update failed. You can review the logs or retry the policy update here |
|
@metamaskbot update-policies |
|
No policy changes |
1 similar comment
|
No policy changes |
20b99b9 to
3db3c08
Compare
|
@metamaskbot update-policies |
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
✨ Files requiring CODEOWNER review ✨🧩 @MetaMask/extension-devs (3 files, +19 -3)
📜 @MetaMask/policy-reviewers (3 files, +19 -3)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. 🔗 @MetaMask/supply-chain (3 files, +19 -3)
|
Builds ready [c4f11c5]
UI Startup Metrics (1222 ± 95 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
|
c4f11c5 to
0245a73
Compare
Builds ready [0245a73]
UI Startup Metrics (1199 ± 102 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
|
|
@metamaskbot update-policies |
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
Builds ready [0b22503]
UI Startup Metrics (1193 ± 82 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
|
0b22503 to
d9aa833
Compare
|
@metamaskbot update-policies |
|
Policy update failed. You can review the logs or retry the policy update here |
b9c8e5a to
d0ea0c7
Compare
Builds ready [d0ea0c7]
UI Startup Metrics (1210 ± 88 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
|
b571dfe to
92453b4
Compare
| use: [ | ||
| { | ||
| loader: reactCompilerLoader, | ||
| options: defineReactCompilerLoaderOption(reactCompilerOptions), | ||
| }, | ||
| ], |
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.
reactCompilerLoader is applied last, after codeFenceLoader and swcLoader.
- Placing
reactCompilerLoaderbeforecodeFenceLoaderresults in errors about/// END:directives not being found. - Placing it before
swcLoaderresults in errors about input source maps not being found.
| options: defineReactCompilerLoaderOption(reactCompilerOptions), | ||
| }, | ||
| ], | ||
| }, |
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.
Bug: React Compiler loader missing required loaders
The React Compiler loader rule only applies reactCompilerLoader without the preceding tsxLoader and codeFenceLoader that are necessary to process TypeScript and JSX syntax. According to the PR discussion, reactCompilerLoader must be applied after codeFenceLoader and swcLoader to avoid errors about missing directives and source maps. The current configuration will fail to properly transform files in the UI directory.
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.
reactCompilerLoader is applied last.
- The code-fence directives and input-source-map errors appear if I move
reactCompilerLoaderbetweenswcLoader,codeFenceLoaderin the use chains, or if I move the react compiler rule below the own-javascript/typescript rules. - I can verify that the UI files in the build are being correctly transformed.
This reverts commit 3ac0d1c.
|
@metamaskbot update-policies |
b816440 to
659c749
Compare
…n/build/enable-react-compiler-webpack
| export const UI_DIR_RE = new RegExp( | ||
| `${join(__dirname, '../../../').replaceAll(sep, slash)}ui${slash}(?:components|contexts|hooks|layouts|pages)${slash}.*$`, | ||
| 'u', | ||
| ); |
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.
Bug: Missing path separator in UI directory regex
The UI_DIR_RE regex is missing a path separator between the project root and ui directory. The expression join(__dirname, '../../../') returns a path without a trailing separator, so concatenating it directly with ui${slash} creates an invalid pattern like /path/to/projectui/ instead of /path/to/project/ui/. This causes the regex to never match any files in the ui/ directory, preventing the React Compiler from being applied to UI components.
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.
The opposite is true. Adding an extra slash breaks the regex. I triple-checked while writing this.
| type: 'asset/resource', | ||
| }, | ||
| { | ||
| test: /(?!\.(?:test|stories|container))\.(?:m?[jt]s|[jt]sx)$/u, |
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.
Bug: Regex fails to exclude test and story files
The negative lookahead in the test regex (?!\.(?:test|stories|container)) checks the position immediately before the file extension, not whether these patterns appear in the filename. For a file like Button.test.tsx, the lookahead checks if the position before .tsx is followed by .test, which it isn't (it's followed by .tsx), so the regex incorrectly matches. This causes the React Compiler to process test files, story files, and container files, which was explicitly intended to be excluded.
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.
The current regex is correct.
- test, story etc. files are being excluded as intended.
- The negative lookahead is supposed to only check the position immediately before the file extension.
Builds ready [54d4402]
UI Startup Metrics (1283 ± 100 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
|
…ongsun/build/enable-react-compiler-webpack
Builds ready [e790a49]
UI Startup Metrics (1230 ± 106 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
|
Description
--reactCompilerVerboseWebpack cli argument, which outputs file compilation status, errors, statistics.yarn webpack --reactCompilerVerboseoutput:Changelog
CHANGELOG entry: null (build)
Related issues
react-compiler/react-compilerESLint rule violations #37480: Previous PRManual testing steps
yarn webpack --no-cache.dist/{chrome,firefox}/js-ui_{com,d,p,pages}*Screenshots/Recordings
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Enable React Compiler via
react-compiler-webpackfor selected UI sources, with config, policies, and deps updates.react-compiler-webpack) with rule targetingui/(components|contexts|hooks|layouts|pages)viaUI_DIR_RE.reactCompilerOptions(target17) and wire into loader options.UI_DIR_REinutils/helpers.ts.reactCompilerOptionsinutils/config.ts.react-compiler-runtimeand map JSX syntax deps toreact-compiler-webpackin build-system policy.react-compiler-runtimeresources to MV2/MV3 webpack policies.react-compiler-webpacktopackage.jsonand ignore list in.depcheckrc.yml.Written by Cursor Bugbot for commit 54d4402. This will update automatically on new commits. Configure here.