Extract shared Intl.Collator into CollatorUtils - #100944
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai. |
|
@thelullabyy 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] |
Explanation of Change
Prep slice 1 of 4 decomposing #99062 (Route Search previews through the shared LHN alternate text pipeline). This slice only extracts the shared
Intl.Collatorconstruction into a newCollatorUtils.getCollator(locale)helper, ahead of the actual pipeline change.Behavioral note: in
LocaleContextProvider, this replaces a per-rendernew Intl.Collator(...)with a shared cached instance.Intl.Collator#compareis stateless, so sharing the instance across renders/locales is safe, and the now-stable identity helps downstream memoization.No behavior change otherwise — all three call sites keep the same
COLLATOR_OPTIONS, just sourced from one place.Fixed Issues
$ #98958
PROPOSAL:
Tests
Offline tests
N/A — pure refactor of locale-aware string comparison, no network interaction.
QA Steps
Same as tests.
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