[No QA] Rewrite React Compiler CI compliance check#87182
Merged
roryabraham merged 26 commits intomainfrom Apr 15, 2026
Merged
Conversation
Allow the last positional arg in a CLI config to be marked `variadic: true`, which collects all remaining positional args into a string[]. The type system correctly infers string[] for variadic args. Made-with: Cursor
New method returns {filename, status}[] using paginated GitHub API
in CI and git diff locally. Refactors getChangedFileNames() to be
a lightweight wrapper around it, fixing a pagination bug where
large PRs could have files silently dropped.
Made-with: Cursor
The compiler config was duplicated in babel.config.js and the eslint-plugin-react-compiler-compat. Now both import from a single source of truth at scripts/utils/reactCompilerConfig.js. Made-with: Cursor
Replace the 860-line script that shelled out to react-compiler-healthcheck and parsed stdout with regexes. The new implementation uses @babel/core's transformSync with babel-plugin-react-compiler directly (~200 lines). Two modes: - `check <files...>` -- local CLI for checking specific files - `check-changed` -- CI mode with two rules: 1. New files with components/hooks must compile 2. Modified files must not regress vs main Uses a three-state return (compiled/failed/no-components) to correctly skip non-React files without false positives. Made-with: Cursor
Hooks typically live in .ts files, not just .tsx. The path trigger now includes .ts, .tsx, and .jsx so the compliance check covers all files that could contain React components or hooks. Made-with: Cursor
The compliance check now uses babel-plugin-react-compiler directly, making the react-compiler-healthcheck package unnecessary. Made-with: Cursor
Simplify docs to reflect the rewritten tool: direct babel transform, two CI rules (new files must compile, no regressions on modified files), and simpler CLI usage. Made-with: Cursor
The repo has zero .jsx files — everything is TypeScript. Made-with: Cursor
Use `variadic?: true` literal type and explicit cast at the call site to work around TypeScript's inability to narrow conditional mapped types through tuple inference. Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
The `check` command now accepts directories (recursively finds .ts/.tsx files) and glob patterns in addition to individual file paths. Inputs are deduplicated and non-.ts/.tsx files are filtered. Made-with: Cursor
chrispader
approved these changes
Apr 7, 2026
Contributor
chrispader
left a comment
chrispader
left a comment
There was a problem hiding this comment.
Looks good to me aside from minor requests! 🙌🏼
- Move resolveFilePaths to FileUtils.ts with configurable extensions - Cache fs.existsSync/statSync to avoid redundant calls - Use FILE_EXTENSIONS variable in log message Made-with: Cursor
…JS check The typecheck workflow blocks new .js files; add an exclusion for config/babel/*.js since this config must be plain JS (consumed by babel.config.js before TS compilation). Also rebuild GitHub Action bundles to reflect getChangedFilesWithStatus() changes in Git.ts. Made-with: Cursor
melvin-bot
Bot
requested review from
parasharrajat
and removed request for
a team
|
@parasharrajat Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Contributor
Author
|
@mountiny do you think we should add this check as just a warning first, or just enable it? There might be some cases where it would be a bit confusing for people to fix. If we decide to merge through a failure, it will not continue to fail on subsequent PRs, because it only complains when:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 270c39d10d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
For renamed files, the GitHub API provides previous_filename which we now surface through getChangedFilesWithStatus(). The regression check uses this to look up the file on main at its old path, correctly detecting regressions even when a file is renamed. Made-with: Cursor
Use -M flag in git diff to detect file renames locally. Without this, renames appeared as delete + add, incorrectly subjecting renamed files to the "new files must compile" rule. Now renames are properly detected with their previous path, matching the behavior of the GitHub API in CI. Made-with: Cursor
Contributor
Author
|
@mountiny ready for review |
Member
|
Do we need C+ review? If not, please unassign me. |
mountiny
previously approved these changes
Apr 13, 2026
Contributor
mountiny
left a comment
mountiny
left a comment
There was a problem hiding this comment.
Looks good to me overall, Rory! I just had one comment, but approving and feel free to merge when you can monitor how its doing after merging. Thanks
Comment on lines
+101
to
+102
| logError(`FAILED ${file}`); | ||
| hasFailure = true; |
Contributor
There was a problem hiding this comment.
Will it show what line exactly is causing the failure? I think the healthcheck before was specific in terms of why the file is failing, no?
Contributor
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
Contributor
Author
|
I think that's feedback worth addressing before merge. working on it |
The compliance check now collects and displays error details from the React Compiler logger including reason, file:line:column, and function location. In GitHub Actions, errors for each file are wrapped in collapsible ::group:: sections. Made-with: Cursor
- Add non-null assertions for .at(0) in test assertions - Add 'endgroup' to cspell dictionary for GitHub Actions syntax Made-with: Cursor
Made-with: Cursor
Contributor
Author
|
@mountiny ready for another review |
mountiny
previously approved these changes
Apr 14, 2026
Contributor
mountiny
left a comment
mountiny
left a comment
There was a problem hiding this comment.
Some NAB comments, looks good to me
Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>
Made-with: Cursor
Contributor
Author
|
@mountiny ready for another review |
mountiny
approved these changes
Apr 15, 2026
Contributor
mountiny
left a comment
mountiny
left a comment
There was a problem hiding this comment.
Thanks, feel free to merge whenever you ready
Contributor
|
🚧 @roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
Contributor
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Contributor
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
54 tasks
Contributor
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.3.60-0 🚀
Bundle Size Analysis (Sentry): |
Contributor
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.60-22 🚀
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
Rewrites the React Compiler compliance check CI workflow from scratch. The new implementation is much simpler and has much simpler requirements - it just enforces:
The previous implementation (~860 lines) shelled out to
npx react-compiler-healthcheck, parsed its stdout with regexes, and applied complex line-level diff analysis with manual memoization checking. The new implementation is ~200 lines and uses@babel/core'stransformSyncwithbabel-plugin-react-compilerdirectly, which is simpler, faster, and more reliable. The healthcheck is deprecated anyways.One key difference is that we _don't suppress
BuildHIRexceptions. In my experience it's pretty trivial to work around these, and I haven't really found any cases that are insurmountable. So I'm letting those surface so they can be fixed.Key changes:
compiled/failed/no-components(leverages the compiler's own component/hook detection)npm run react-compiler-compliance-check check <files...>ReactCompilerConfigto eliminate duplication across babel.config.js, ESLint plugin, and this scriptGit.getChangedFileNames()(was only fetching first page of GitHub API results)scripts/utils/CLI.tsreact-compiler-healthcheckdependency and its 3 patch files.tsand.jsxfiles (hooks live in.tsfiles)Fixed Issues
$ #68765
Tests
npm run react-compiler-compliance-check check src/components/Button/index.tsxand verify it reportsCOMPILEDnpm run react-compiler-compliance-check check src/CONST.tsand verify it skips (no components)npx jest tests/unit/CLIVariadicTest.ts tests/unit/ReactCompilerComplianceCheckTest.tsand verify all 9 tests passOffline tests
N/A - CI tooling only, no runtime behavior.
QA Steps
N/A - CI tooling only.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
N/A - CI tooling only
Android: mWeb Chrome
N/A - CI tooling only
iOS: Native
N/A - CI tooling only
iOS: mWeb Safari
N/A - CI tooling only
MacOS: Chrome / Safari
N/A - CI tooling only
Made with Cursor