fix: status is infinity loading for the none-exist key#709
Conversation
|
@bernhardoj can you review please? I believe we don't need a review checklist here |
|
Let's hold the review for this one. I approved the other solution, so we still need to discuss this solution on the App Issue. |
| const shouldUpdateResult = | ||
| !areValuesEqual || | ||
| (previousValueRef.current === null && (hasCacheForKey || OnyxCache.hasPendingTask(TASK.CLEAR))) || | ||
| (previousValueRef.current === null && !isFirstConnectionRef.current); |
There was a problem hiding this comment.
I think we can just combine the !isFirstConnectionRef.current to the previous condition.
(previousValueRef.current === null && (hasCacheForKey || OnyxCache.hasPendingTask(TASK.CLEAR) || !isFirstConnectionRef.current))There was a problem hiding this comment.
Also, please update the comment to include this new case
|
@nkdengineer Hmm, I can't seem to reproduce the issue anymore. (this is before applying this PR) web.mp4 |
|
@bernhardoj This PR changed the |
|
Let me know when this is ready for review |
bernhardoj
left a comment
There was a problem hiding this comment.
I reverted Expensify/App#76602 locally to able to reproduce the issue again and verified the issue is fixed with this PR.
web.mp4
|
@arosiclair ready for your review |
Details
For the key that doesn't exist in the storage,
useOnyxalways returns theloadingstatus. So when the subscribe is triggered,shouldUpdateResultshould be updated totrueto update the status toloaded.Related Issues
Expensify/App#75794
Automated Tests
Manual Tests
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
Screen.Recording.2025-12-06.at.21.54.14.mov
Android: mWeb Chrome
Screen.Recording.2025-12-06.at.21.51.26.mov
iOS: Native
Screen.Recording.2025-12-06.at.21.50.30.mov
iOS: mWeb Safari
Screen.Recording.2025-12-06.at.21.48.20.mov
MacOS: Chrome / Safari
Screen.Recording.2025-12-06.at.21.45.49.mov