[No QA] Use OXC to transform Reassure perf tests - #100429
Conversation
Reassure runs Jest under --max-opt=1, so Babel's JS transform of ~7k files is about half of each measure job. Route perf runs through oxc-transform-react + esbuild for app source (Flow in node_modules still uses babel-jest) so the transform stays native-speed without TurboFan.
OXC does not run babel-plugin-jest-hoist, so measureRenders tests lost NavigationContainer mocks. Route tests/, jest/, and __mocks__/ through babel-jest; only app src/ uses OXC. Move the transformer under config/babel/ so CI's no-new-JS-in-jest/ check is satisfied.
|
@codex review |
|
@MelvinBot review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
esbuild's default CJS emit leaves import() as ESM, which would skip Jest's registry for React.lazy in src/. Disable dynamic-import support so it becomes require(). Also hash the React Compiler config and the esbuild / oxc-transform-react versions so a restored .jest-cache cannot serve stale output.
|
Addressed the review in de96a66:
Also inlined the dead Skipped the rest (silent Babel fallback, path-regex parent-dir false positives, Fullstory/class-field tree skew, extra test gaps) as polish — none blocked merge after |
roryabraham
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de96a6674b
ℹ️ 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".
| const oxcResult = transformSync(sourcePath, sourceText, { | ||
| lang: getLang(sourcePath), | ||
| sourcemap: true, | ||
| jsx: {runtime: 'automatic', development: true}, | ||
| reactCompiler: REACT_COMPILER_OPTIONS, |
There was a problem hiding this comment.
For every measureRenders suite, this sends application components through OXC while the React Native production/Jest path still uses Babel. These compilers are documented as behaviorally different, and this route also skips Babel-only transforms such as the FullStory JSX annotations in babel.config.js, so measured render counts and durations can diverge from the native code the benchmark is intended to protect; a regression caused by Babel's compiler output or annotation overhead can consequently pass this suite. Preserve the production transforms for measured React modules, or reproduce them before using OXC for the remaining lowering. CLAUDE.mdL57-L59
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
web uses oxc transformer and the Rust React Compiler. iOS/Android will soon: https://expensify.slack.com/archives/C05LX9D6E07/p1787741223437369
| sourcefile: sourcePath, | ||
| sourcemap: true, |
There was a problem hiding this comment.
When transformed application code throws during a performance test, this source map treats oxcResult.code as though it were the original sourcePath, while the actual OXC map produced above is discarded. React Compiler rewrites can substantially move generated statements, so Jest stack frames point to incorrect source lines; compose oxcResult.map with the esbuild map (as the existing Rsbuild OXC loader does) before returning it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Duplicate from a previous review, chose not to address
neil-marcellini
left a comment
neil-marcellini
left a comment
There was a problem hiding this comment.
Looks good overall. Only one comment that is non-blocking. I used a lot of AI to explain this to me and it all checks out, but you might want to have someone with more experience in this area review it to. Up to you.
| // Reassure re-transforms ~7k files under `--max-opt=1` (V8 sparkplug only), which | ||
| // makes Babel ~half of each measure job. OXC + esbuild is native and stays fast | ||
| // without TurboFan. Test files stay on babel-jest so `jest.mock` is still hoisted. | ||
| '^.+\\.[jt]sx?$': isPerfTestRun ? '<rootDir>/config/babel/oxcJestTransformer.js' : 'babel-jest', |
There was a problem hiding this comment.
I'm new to this area of our codebase so just about anything you write here will be a bit hard for me to understand, but there's a lot in here which is specifically hard to understand and goes beyond the scope of what this line does. Maybe this version would explain the why behind this line without adding more questions?
// For performance test runs use oxc to transform the source files using native code, instead of Babel running JS. Performance tests use a really slow but stable compiler for stable results, which makes the transformation slow. Oxc avoids that slow down, saving developer time.
There was a problem hiding this comment.
going to merge and then follow-up with a better comment
|
🚧 roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
4 participants
Explanation of Change
Reassure's measure jobs re-transform ~7k files through Babel on every run. That is worse than it looks: Reassure launches Jest with
--max-opt=1, so V8 stays on Sparkplug and Babel's JS AST walk is about half of each job (issue #100428). Caching that output (#100297) is the right first cut. This PR asks whether we still need Babel at all for the perf path.We already use
oxc-transform-reacton web (TS strip + JSX + React Compiler). The same pipeline covers what Jest actually needs frombabel.config.jsfor app source:jsxDEVin tests)tests/the same way Babel'ssourcesfilter doesWhat Babel still uniquely does — Flow in
node_modules/react-native, Fullstory, worklets, Expo env inlining, andjest.mockhoisting — is not needed for appsrc/. Those files still go throughbabel-jest. Regular unit tests keepbabel-jestentirely.The 12s/0.39s numbers are a 300-file microbench of the transform itself. The Reassure job wall clock is the row that matters.
src/)--max-opt=1Run … performance testsstepFixed Issues
$ n/a
Tests
Automated tests only.
Offline tests
N/A - CI transformer change, no app/network surface.
QA Steps
None.
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari