Remove initWithStoredValues from Onyx#782
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ 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". |
|
Ready to review @roryabraham @Krishna2323 @Julesssss |
|
I'll review tomorrow morning. |
|
Running QA testing on the App AdHoc build |
| // Create options hash without expensive JSON.stringify | ||
| const initWithStoredValues = options?.initWithStoredValues ?? true; | ||
| const cacheKey = `${selectorID}_${initWithStoredValues}`; | ||
| const cacheKey = `${selectorID}`; |
There was a problem hiding this comment.
Should this keep the _ underscore?
There was a problem hiding this comment.
No need anymore as it was only necessary to concat the initWithStoredValues option, which was necessary to differentiate cache keys. Since we don't have it anymore we can just use String(selectorID) that I adjusted in d83e8b7
There was a problem hiding this comment.
OK, thank you for confirming!
|
I don't know why but Typecheck and Jest reported strange failures that I don't have locally, for example TS one: This doesn't make any sense as we don't even have |
|
Restarted them 👍 |
|
Oh it was because the branch was behind |
Details
Removes
initWithStoredValuesfrom Onyx as the final task of Expensify/App#80091.E/App PR: Expensify/App#89488
Related Issues
Expensify/App#80091
Automated Tests
Unit tests removed.
Manual Tests
We just need to test the removal of the last
initWithStoredValuesthat was introduced in Expensify/App#81973. Use Expensify/App#89488 for testing:Author Checklist
### Related Issuessection aboveTestssectiontoggleReportand notonIconClick)myBool && <MyComponent />.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Avataris modified, I verified thatAvataris working as expected in all cases)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
N/A, emulator crashing for me
Android: mWeb Chrome
Screen.Recording.2026-05-06.at.13.41.05.mov
iOS: Native
Screen.Recording.2026-05-06.at.13.50.44.mov
iOS: mWeb Safari
Screen.Recording.2026-05-06.at.13.54.43.mov
MacOS: Chrome / Safari
Screen.Recording.2026-05-06.at.13.56.24.mov
Screen.Recording.2026-05-06.at.13.57.26.mov