[Part 3]: Using buildNextStepNew function in IOU.ts#73816
[Part 3]: Using buildNextStepNew function in IOU.ts#73816danieldoglas merged 24 commits intoExpensify:mainfrom
Conversation
|
@jjcoffee Please ignore this PR, @allgandalf will review it |
|
let me know once this is ready for review |
|
Continue working on it tomorrrow |
Code Review SummaryFiles Reviewed: 18
Issues Found: 1PERF-6 Violation: Missing Dependency in useMemo HookFile: Issue: The Current Code: const messageContent = useMemo(() => {
const messageArray = nextStep?.message;
return parseMessage(messageArray, currentUserPersonalDetails.email ?? '');
}, [nextStep?.message]);Fix: const messageContent = useMemo(() => {
const messageArray = nextStep?.message;
return parseMessage(messageArray, currentUserPersonalDetails.email ?? '');
}, [nextStep?.message, currentUserPersonalDetails.email]);Reasoning: Passing entire objects as dependencies causes hooks to re-execute whenever any property changes, even unrelated ones. Specifying individual properties creates more granular dependency tracking, reducing unnecessary hook executions and improving performance predictability. Overall AssessmentThe PR is a large refactoring effort to replace the deprecated The code quality is generally good, with proper parameter typing and consistent patterns. The only issue found is a missing dependency in a |
|
@allgandalf Ready for your review |
|
jests are failing |
|
@allgandalf The failed UTs look unrelated. I will check again later. Please ignore it now |
|
Code refactor PR, unassigning and unsubscribing. 👍 |
|
jest is failing |
|
more failures :) |
|
🚀 Deployed to staging by https://github.com/danieldoglas in version: 9.2.46-0 🚀
|
Revert "Merge pull request #73816 from DylanDylann/refactor-808"
|
@DylanDylann This PR is failing because of a regression issue #74618 bug.mp4 |
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.2.46-3 🚀
|
|
🚀 Deployed to staging by https://github.com/danieldoglas in version: 9.2.47-0 🚀
|
|
@DylanDylann when can we expect a V2 PR? |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.2.47-1 🚀
|
6 participants
Explanation of Change
Fixed Issues
$ #66365
$ #66366
$ #66367
$ #66368
PROPOSAL:
Tests
Offline 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
Screen.Recording.2025-10-20.at.15.38.22.mov
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop