[NoQA] Add React Compiler context to AI reviewer and improve general PERF-4 performance#76981
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. |
…kReactCompilerOptimization
- Reduce PERF-4 from ~230 lines to ~60 lines while preserving all information - Add "Applies ONLY to" section to prevent flagging code in callbacks - Fix decision flow order: parent optimized → custom comparator → child memoized - Simplify examples from 6 to 3 (Flag, Skip-comparator, Skip-optimized) - Add note: "File not found" → Assume parent is optimized and skip PERF-4 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…hecking - Script now outputs sourcePath for each child component variant - Updated decision flow step 2 with explicit instructions to use sourcePath - Claude can now read child's source file to check for custom memo comparators 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Changed from "MANDATORY for ANY potential PERF-4" to "YOU MUST call on EVERY .tsx file from the diff" This removes ambiguity - Claude must run the script on all files, not decide which files might need it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This reverts commit 71b3bbd.
This reverts commit 9337cc7.
|
@abdulrahuman5196 Do you want to review/ test this? what is your ETA if yes? |
This comment was marked as outdated.
This comment was marked as outdated.
kacper-mikolajczak
left a comment
There was a problem hiding this comment.
Overall LGTM and left couple of comments where I feel like need more clarification.
Thanks Mateusz!
chrispader
left a comment
There was a problem hiding this comment.
LGTM overall, i just have a few minor questions
chrispader
left a comment
There was a problem hiding this comment.
LGTM! Thanks for migrating the script to TypeScript 🙌🏼
|
@mateuuszzzzz Is there a way to check if this change is working properly? |
If you ask about AI reviewer itself then I believe we need to see how it performs on future PRs. But in PR details I included steps for testing new tool we added: |
|
Awesome we are so close to get this ready 🚀 |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
mountiny
left a comment
There was a problem hiding this comment.
Thanks, lets give it a try now, we should keep in eye on the actions time after this is merge
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.2.89-1 🚀
|
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.2.89-6 🚀
|
Explanation of Change
PR adds new tool that can be used by Claude to get extra React Compiler context from health-check script.
Additionally, it slightly refactors prompt to eliminate false positives reported by developers.
Fixed Issues
$ #75686 #74471
PROPOSAL:
Tests
Since this is change to AI reviewer that suppose to reduce false positives, adds react compiler context and overall aims to improve performance of executing PERF-4 rule it might be hard to define strict testing steps. Nevertheless, new script was added and should be tested.
parentOptimizedandchildComponents, run without args or with invalid path, expect exit code 1childComponentsOffline tests
QA Steps
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