feat: expose callback trigger value for collections#622
feat: expose callback trigger value for collections#622MonilBhavsar merged 9 commits intoExpensify:mainfrom
Conversation
| /** | ||
| * The value that triggered the last update | ||
| */ | ||
| sourceValue?: OnyxValue<OnyxKey>; |
There was a problem hiding this comment.
Is it the value or an object of onyx key and value?
There was a problem hiding this comment.
OnyxValue represents a value that can be either a single entry or a collection of entries
|
@MonilBhavsar friendly bump, could you take a look when you have a moment? |
MonilBhavsar
left a comment
There was a problem hiding this comment.
Looks great! Sorry for delay. Could you please provide a usage for this in App repo, or where we're going to utilize this?
|
My idea is to create a new derived value that stores certain report attributes that does not change often. The goal is to avoid unnecessary recomputations and significantly reduce processing overhead. In this draft PR, I moved the Initial measurements look very promising—this simple optimization already brings noticeable performance improvements. We can expand this further by moving more static or infrequently changing attributes into derived values to scale the gains even more. |
|
Thanks for explaining. Makes sense 👍 |
Details
This PR introduces exposing the value that triggered the connection callback. It's helpful when callback is doing some operations for each collection item - e.g. when a collection key is a dependency of derived value in E/App and it loops through all items to compute things, having this trigger value allows to make an update for a single item rather than recompute everything from scratch.
Related Issues
GH_LINK
https://expensify.slack.com/archives/C05LX9D6E07/p1741698890119029
Automated Tests
Added unit tests that check if:
sourceValuewhenwaitForCollectionCallback: truesourceValueis not available forwaitForCollectionCallback: falseManual Tests
Opened a draft PR where
getReportNameis moved to derived value and it updates a single item: Expensify/App#58476If you want to make sure we just did a single-item computation, open chrome dev tools -> Performance and start recording before step no 7, download the trace, upload it to https://www.speedscope.app/ and search for
recomputeDerivedValue. You should findcomputefunction forreportAttributeshere, which should be a very tiny update probably with <1ms of execution time.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