Add deepEqual check for getSnapshot collections#717
Add deepEqual check for getSnapshot collections#717LukasMod wants to merge 1 commit intoExpensify:mainfrom
Conversation
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
| if (memoizedSelector) { | ||
| if (isCollection) { | ||
| // Collections always need deepEqual, even with selectors, because selector outputs are new object references | ||
| areValuesEqual = deepEqual(previousValueRef.current ?? undefined, newValueRef.current); |
There was a problem hiding this comment.
Do we do deepEqual of the whole collection? But the collections can be HUGE, right? Will it be performant enough?
| const cachedValue = cachedResult[0]; | ||
| const oldValue = oldResultRef[0]; |
There was a problem hiding this comment.
Hey, can you clarify what is the cachedResult[0] and oldResultRef[0] in this case?
There was a problem hiding this comment.
[0] is data, [1] is metadata. I changed it to whole array in my current experiments. I opened new PR
|
Close this in favor of #718 |
Details
Related Issues
GH_LINK
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop