Repository navigation
Keep cached Onyx arrays when re-sent data is unchanged to avoid recalculations and re-renders #104725
Description
Problem
After every OpenApp or ReconnectApp, the app sends SearchForTodos. Its response merges records into several collections: transaction violations, transactions, report actions, and policies. Transaction violations are stored as arrays.
Since Expensify/react-native-onyx#765 and Expensify/react-native-onyx#768, a merge that changes nothing keeps the original object, and the cache skips that key.
Arrays are not covered. An incoming array always replaces the cached one, so the array and its parent objects get new references even when the content is identical. Derived values (OnyxDerived) and useOnyx subscribers without a selector detect changes by reference.
As a result, when the server re-sends unchanged records, the app recalculates and re-renders data that did not change. On Android, the Spend tab takes about 0.7 s (30%) longer to load.
Unchanged records re-sent by SearchForTodos on every cold start:
1,600 reports 3,900 reports
violations 1,355 212
transactions 201 106
report actions 31 165
policies 6 26
total 1,593 509
On the 1,600-report account, OpenSearchPage also re-sends 256 unchanged policies when the Spend tab opens.
Solution
In react-native-onyx, keep the cached array when the incoming one is deep-equal to it, for cache merges only. This is the same rule Expensify/react-native-onyx#765 already applies to objects.
Unchanged records then keep their references, so derived values skip them and subscribers do not re-render for them.
Draft PR: Deep compare arrays on merge and return old array if unchanged
Impact
Measured on a working Onyx branch against the same build without the change (medians, dev builds, account with about 1,600 reports):
before after change
Android
ManualNavigateToReportsContentLoad 3.07 s 2.35 s -24%
ManualNavigateToReports 3.06 s 2.35 s -23%
OnyxDerivedCompute_reportAttributes 471 ms 28 ms -94%
Web
ManualNavigateToInboxTab 216 ms 176 ms -19%
OnyxDerivedCompute_reportAttributes 28 ms 6 ms -78%
OnyxDerivedCompute, all values 103 ms 60 ms -44%
No measured span got slower on either platform. Other spans (opening a report, the search router, the Inbox on Android) did not change.
The size of the gain depends on how much unchanged data the server re-sends for the account. The change is general: every key and every command that re-sends identical arrays stops triggering work.
Notes
- Coordinate with the changes in Expensify/react-native-onyx#856 in case they overlap.
Reported in Slack.
Issue Owner
Current Issue Owner: @jmusialMetadata
Metadata
Labels
Type
Projects
- Status
Activity
zhumeiyan commentedon Oct 23, 2026
Proposal
Please re-state the problem that we are trying to solve in this issue.
On every
OpenApp/ReconnectApp,SearchForTodosre-sends records that have not changed. PR #765 and #768 made object merges keep the original object when nothing changed, so the cache skips those keys. Arrays were not covered. An incoming array always replaces the cached one, so the array and its parent objects get new references even when the contents are byte-identical.OnyxDerivedanduseOnyxsubscribers without a selector compare by reference, so they see a change that did not happen and recalculate / re-render. On a 1,600-report account that is ~1,593 unchanged records on cold start, and the Android Spend tab takes ~0.7 s (30%) longer.What is the root cause of that problem?
Reference identity is being used as a proxy for value equality, and for arrays that proxy is broken. After #765/#768 an unchanged object survives a merge with its reference intact, so its subscribers correctly stay quiet. An array carrying the same unchanged objects still gets rebuilt, so it hands out a fresh reference and every subscriber downstream treats it as new data. The upstream data is fine — the identity contract is what breaks, and it breaks specifically on the array branch of the merge path.
What changes do you think we should make in order to solve the problem?
Follow the issue's stated solution in
react-native-onyx: on cache merges, keep the cached array when the incoming one is deep-equal to it. This is the array counterpart of what #765/#768 already do for objects. Draft PR #860 exists.What I would add on top of the draft, and why it matters:
The issue reports that "no measured span got slower," and I believe that measurement — but it does not cover the cost that the fix itself introduces. A deep compare now runs on every merge of every array, which is a hot path that did not previously do this work. The reported spans measure end-to-end navigation; they would only surface a deep-compare regression if it were large enough to move a whole navigation span. A per-merge cost increase that is real but smaller than span noise would not appear.
So I propose measuring the new cost directly and separately, stratified by array size:
The 1,600-report account in the issue has 1,355 violations in one array. If the curve is superlinear in that range, the fix trades a render cost for a compare cost on large accounts, and that trade should be visible in the PR rather than discovered later. If the curve is flat and negligible, the fix is unambiguously good and the table says so. Either result is a useful outcome — I am not proposing this because I expect to find a problem, but because a cost added to a hot path should be measured on its own rather than inferred from a span that got faster.
If the curve does turn out to be a problem at the large end, the natural mitigation is to compare cheap first (length, then element-wise
Object.is, then fall back to deep compare only when the shallow pass is inconclusive) so the common case stays O(n) pointer comparisons.What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
Four tests, and the fourth one is the one I care most about:
Merge an array whose contents are identical → the cached reference is preserved.
This is the fix itself. Assert
Onyx.get()/cache returns the same array object before and after.Merge an array whose contents differ → the reference does change.
If the comparison is written too eagerly (for instance by short-circuiting on length alone, or by comparing only the first element), test 1 passes and the store silently stops propagating real updates. Test 2 is the guard against that. A fix that keeps references too aggressively is worse than the bug, because lost updates are silent and re-renders are not.
An element that is itself an unchanged object survives inside the array.
Per the root cause, arrays are rebuilt even when their elements are the objects Update version to 1.0.1-171 #765/[HOLD] Add Automated Tests for Onyx / withOnyx #768 already preserved. This test pins the interaction between the two fixes rather than assuming it composes.
A negative control: the same assertions run against a deliberately broken implementation must fail.
This is the one that catches a suite that passes because it is not actually exercising the path — for example if the test fixture never puts an array in the cache, or if the assertion compares with
toEqualinstead oftoBe.toEqualpasses on a deep-equal copy and would make every test above green while the bug is still live. I have been bitten by exactly this shape of false green, so I would rather assert on a known-bad implementation once than trust three green checks.What I cannot do, stated up front
Why this proposal is shaped this way
The fix is already specified in the issue and largely drafted. The remaining risk is not "will it work" but "what does it cost, and will it stay fixed." Those are measurement and test questions, which is where I would put the effort. I would rather deliver a table that says "compare cost is flat, no concern" than deliver a fix with an unmeasured cost on a hot path.
melvin-bot commentedon Oct 23, 2026
📣 @zhumeiyan! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:
Format:
zhumeiyan commentedon Oct 23, 2026
Follow-up: I measured the deep-compare cost, and it disproves my own concern
In my proposal above I raised the possibility that adding a deep compare to every array merge could become a new hot-path cost, and suggested a cheap-first tiered comparison as mitigation. I ran the measurement, and the concern does not hold. I'm posting it rather than quietly dropping it.
Method: synthetic arrays, three scenarios, 200 samples after 20 warmup iterations, median. Node on my machine — not device numbers, not extrapolable to Android/iOS.
What this says
1. The mitigation I proposed is not worth adding. In the scenario that actually occurs — unchanged re-sent data, where #765/#768 have already preserved element object references — 5,000 elements cost 0.0079 ms. The 1,355-violation array from the issue is well below that. The tiered comparison is not faster (ratio 0.8, slightly slower, since the tier checks themselves cost something). Please ignore that part of my proposal.
2. The cost lives almost entirely in "elements were rebuilt." Same-ref 0.0079 ms vs all-new 0.80 ms at length 5,000 — roughly 100x. Deep compare returns nearly free when
Object.isholds element-wise; it only recurses when elements are genuinely new objects.3. This makes the cost curve useful as a diagnostic, which I did not expect. If #765/#768 are working, merged element objects keep their references and the cost lands in the 0.0079 ms bucket. If a real account measures near 0.8 ms, that is not "deep compare is slow" — it would mean element-level reference preservation is not taking effect on the array path. The measurement doubles as a probe for whether #765/#768 actually hold here. That seems more valuable than the performance number itself, and it's the version I'd want in CI: not a timing assertion, but a reference-identity assertion.
Caveats
{id, type, name, amount, meta}). Real violation objects may nest deeper, which would widen the all-new scenario specifically.Net: the issue's approach looks unconditionally fine on cost. The thing worth testing in CI is reference identity, not timing.
zhumeiyan commentedon Oct 23, 2026
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/z4570285
jmusial commentedon Oct 27, 2026
Update: the react-native-onyx PR has been merged. The App PR is holding on removal of Onyx.disconnects
melvin-bot commentedon Oct 30, 2026
@jmusial Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!