[Part 13] Remove Onyx.connect from LocalePhoneNumber#96192
[Part 13] Remove Onyx.connect from LocalePhoneNumber#96192marufsharifi wants to merge 3 commits into
Conversation
|
@shubham1206agra 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] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| personalDetails: OnyxEntry<OnyxTypes.PersonalDetailsList>; | ||
| isTrackIntentUser: boolean | undefined; | ||
| delegateAccountID: number | undefined; | ||
| formatPhoneNumber?: LocaleContextProps['formatPhoneNumber']; |
There was a problem hiding this comment.
Why is this optional?
There was a problem hiding this comment.
Good catch, I missed that. I'll make it required and update all the callers.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| isTrackIntentUser, | ||
| formatPhoneNumber, | ||
| } = moneyRequestInformation; | ||
| const formatPhone = formatPhoneNumber ?? ((n: string) => n); |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The identical no-op fallback const formatPhone = formatPhoneNumber ?? ((n: string) => n); is duplicated across 6 files (MoneyRequestBuilder.ts, PerDiem.ts, SendInvoice.ts, Split.ts twice, and Task.ts twice). Every migrated function re-declares the same identity function to substitute for a missing formatPhoneNumber. This is copy-pasted logic that raises maintenance overhead — if the fallback behavior ever needs to change (e.g. to a shared default formatter), it must be edited in 7 places.
Extract the fallback into a single shared helper and reuse it everywhere the parameter is optional. For example, in @libs/LocalePhoneNumber (or a small local util):
// LocalePhoneNumber.ts
export const identityFormatPhoneNumber = (phoneNumber: string) => phoneNumber;Then at each call site:
const formatPhone = formatPhoneNumber ?? identityFormatPhoneNumber;This removes the duplication and gives the fallback a single, documented home.
Reviewed at: 88e5b9e | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
2 participants
Explanation of Change
This update continues the
Onyx.connectdeprecation forONYXKEYS.COUNTRY_CODEby migrating the IOU action files (Split, SendInvoice, PerDiem, MoneyRequestBuilder, SplitTransactionUpdate) and Task action to acceptformatPhoneNumberas a parameter instead of importing it directly fromLocalePhoneNumber. This ensures the country code is explicitly passed through the call chain rather than read from a module-level global subscription.All caller files (React hooks and components) are updated to pass formatPhoneNumber from
useLocalize()to the migrated functions. This includesuseExpenseSubmission, ScanSkipConfirmation, IOURequestStepCompanyInfo, DynamicNewTaskPage, useDeleteTransactions, and TravelLegalNamePage.Fixed Issues
$ #66327
PROPOSAL:
Tests
Offline tests
QA Steps
Case 1: Split Expense (from Chat)
Case 2: Split Expense (from Scan)
Case 3: Send Invoices
Case 4: Task Creation
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
Case 1:
Screencast.From.2026-07-15.19-54-49.webm
Case 2:
Screencast.From.2026-07-15.20-07-19.webm
Case 3:
Screencast.From.2026-07-15.20-12-15.webm
Case 4:
Screencast.From.2026-07-15.20-32-33.webm