Skip to content

[$175] [Sentry: APP-JVF] Web: useSidebarOrderedReports setState loop, "Maximum update depth exceeded" #104504

Open

Description

@mountiny

Sentry

https://expensify.sentry.io/issues/APP-JVF

Impact (snapshot at filing)

  • Users (total since first seen): 68
  • Events: 110
  • Users (last 14d, all versions): 20
  • Users (last 14d, production 9.5.11 to 9.5.13): 3 (25 events)
  • First seen: 2026-08-05 (about 2.5 months ago)
  • Last seen: 2026-10-19 (ongoing)
  • Platform: web (standalone)
  • App version(s): 9.5.0 to 9.5.13. Most users are on 9.5.6 and 9.5.5.
  • Affected route(s) (parameterized): mostly /search, also /search/r/:reportID
  • Mechanism: generic, level fatal, app_blocking: true

Stack trace (top frames, first-party only)

src/hooks/useSidebarOrderedReports.tsx:328   useEffect -> setCurrentReportsToDisplay(reportsToDisplayInLHN)
react-dom commitHookEffectListMount -> dispatchSetState -> getRootForUpdatedFiber
Error: Maximum update depth exceeded (React #185)

Suspected cause

The effect at useSidebarOrderedReports.tsx:328 copies reportsToDisplayInLHN into state each time the memo returns a new object. The memo at line 229 leaves currentReportsToDisplay out of its dependencies on purpose, to avoid a loop. So a loop here means that one of the memo's other dependencies gets a new identity on every render on /search.

Reproduction

Unknown. See the Sentry events linked from the Sentry issue.

Related

  • Sibling Sentry IDs: none confirmed
  • Prior GH issues: none found
Upwork Automation - Do Not Edit
Issue OwnerCurrent Issue Owner: @parasharrajat

Activity

  1. self-assigned this
    on Oct 20, 2026
  2. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Oct 20, 2026
  3. melvin-bot commented on Oct 20, 2026

    @melvin-bot

    Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External)

  4. changed the title [-][Sentry: APP-JVF] Web: useSidebarOrderedReports setState loop, "Maximum update depth exceeded"[/-] [+][$175] [Sentry: APP-JVF] Web: useSidebarOrderedReports setState loop, "Maximum update depth exceeded"[/+] on Oct 20, 2026
  5. melvin-bot commented on Oct 20, 2026

    @melvin-bot
  6. magomedta commented on Oct 20, 2026

    @magomedta

    Contributor details
    Your Expensify account email: [email protected]
    Upwork Profile Link: N/A (Expensify NewDot Direct Settlement)

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    On web, navigating to /search or /search/r/:reportID crashes the application with a fatal React error:
    Error: Maximum update depth exceeded (React #185) originating from useSidebarOrderedReports.tsx:328 useEffect -> setCurrentReportsToDisplay(reportsToDisplayInLHN).

    What is the root cause of that problem?

    1. On /search routes, useIsOnSearch() is true, which activates search snapshot mode (shouldUseSnapshot = true) in src/hooks/useOnyx.ts for snapshot-compatible collection keys.
    2. In src/hooks/useSidebarOrderedReports.tsx:103, chatReports is subscribed via useOnyx(ONYXKEYS.COLLECTION.REPORT) without a selector.
    3. In src/hooks/useOnyx.ts:
      function resolveSnapshotAwareResult<TKey extends OnyxKey, TReturnValue>(
          shouldUseSnapshot: boolean,
          hasSelector: boolean,
          originalResult: UseOnyxResult<OnyxValue<OnyxKey>>,
          key: TKey,
      ): UseOnyxResult<TReturnValue> {
          if (!shouldUseSnapshot || hasSelector) {
              return originalResult as UseOnyxResult<TReturnValue>;
          }
      
          const keyData = getKeyData(originalResult[0] as SearchResults, key);
          return [keyData, originalResult[1]] as UseOnyxResult<TReturnValue>;
      }
      And in getKeyData:
      if (key.endsWith('_')) {
          const result: OnyxCollection<TKey> = {};
          const prefix = key;
          for (const [dataKey, value] of Object.entries(snapshotData?.data ?? {})) {
              if (!dataKey.startsWith(prefix)) {
                  continue;
              }
              result[dataKey] = value as OnyxEntry<TKey>;
          }
          return (Object.keys(result).length > 0 ? result : undefined) as TReturnValue;
      }
      For any collection key (ending with '_'), getKeyData instantiates a brand new object literal {} on every render without memoization or referential caching.
    4. Because resolveSnapshotAwareResult is called on every render of useOnyx, chatReports receives a completely new object reference every single commit, even when the snapshot data is completely static.
    5. In useSidebarOrderedReports.tsx:
      • chatReports is a dependency of getUpdatedReports (line 215) and reportsToDisplayInLHN (line 289).
      • useCollectionDelta(chatReports) checks if (current === previous) return undefined; — since current !== previous by reference, it returns a new delta object every time.
      • Consequently, reportsToDisplayInLHN recalculates and returns a fresh object on every render.
    6. The effect at useSidebarOrderedReports.tsx:327-329:
      useEffect(() => {
          setCurrentReportsToDisplay(reportsToDisplayInLHN);
      }, [reportsToDisplayInLHN]);
      fires and calls setCurrentReportsToDisplay(reportsToDisplayInLHN). Updating this state triggers an immediate re-render of SidebarOrderedReportsContextProvider, causing useOnyx(ONYXKEYS.COLLECTION.REPORT) to return yet another new object, triggering the effect again in an infinite loop until React aborts with Maximum update depth exceeded.

    What changes do you think we should make in order to solve the problem?

    1. Ensure Referential Stability for Snapshot Collections in src/hooks/useOnyx.ts:
      Memoize the result of getKeyData using a ref cache keyed by (snapshotData, key) or compare shallow equality of the constructed collection against the cached result before returning a new reference. When the underlying snapshot's data for that prefix has not changed, return the existing cached object reference:

      const snapshotCollectionCache = new WeakMap<SearchResults, Map<string, unknown>>();
      
      function getCachedKeyData<TKey extends OnyxKey, TReturnValue>(snapshotData: SearchResults | undefined, key: TKey): TReturnValue {
          if (!snapshotData || !key.endsWith('_')) {
              return getKeyData(snapshotData as SearchResults, key);
          }
          let keyMap = snapshotCollectionCache.get(snapshotData);
          if (!keyMap) {
              keyMap = new Map();
              snapshotCollectionCache.set(snapshotData, keyMap);
          }
          if (keyMap.has(key)) {
              return keyMap.get(key) as TReturnValue;
          }
          const fresh = getKeyData<TKey, TReturnValue>(snapshotData, key);
          keyMap.set(key, fresh);
          return fresh;
      }

      Using a WeakMap<SearchResults, Map<string, unknown>> ensures zero memory leaks when search queries change and guarantees that within the lifetime of a snapshot reference, collection extractions are referentially stable across all renders and consumers.

    2. Defensive Update Guard in src/hooks/useSidebarOrderedReports.tsx:
      In useSidebarOrderedReports.tsx:327-329, guard setCurrentReportsToDisplay using functional state updater and shallow comparison so that identical report mappings do not trigger re-render cascades:

      useEffect(() => {
          setCurrentReportsToDisplay((prev) => {
              const prevKeys = Object.keys(prev);
              const nextKeys = Object.keys(reportsToDisplayInLHN);
              if (prevKeys.length === nextKeys.length && nextKeys.every((key) => prev[key] === reportsToDisplayInLHN[key])) {
                  return prev;
              }
              return reportsToDisplayInLHN;
          });
      }, [reportsToDisplayInLHN]);

    What specific scenarios should we cover in automated tests?

    1. useOnyx Snapshot Collection Referential Stability (tests/unit/hooks/useOnyxTest.ts):
      • Render useOnyx(ONYXKEYS.COLLECTION.REPORT) inside SearchContext with active snapshot data.
      • Force re-render of the parent component without mutating the snapshot.
      • Assert that result[0] retains strict referential equality (===).
    2. useSidebarOrderedReports Render Loop Prevention (tests/unit/useSidebarOrderedReportsTest.tsx):
      • Render SidebarOrderedReportsContextProvider in a simulated search environment (isOnSearch = true).
      • Verify that setCurrentReportsToDisplay stabilizes and does not enter an infinite render loop or exceed maximum update depth.
  7. melvin-bot commented on Oct 20, 2026

    @melvin-bot

    ✅ Contributor details stored successfully. Thank you for contributing to Expensify!

  8. camesenin commented on Oct 20, 2026

    @camesenin

    Contributor details
    Your Expensify account email: [email protected]
    Upwork Profile Link: https://www.upwork.com/freelancers/~0170d6b02e9440e604

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    Sentry issue APP-JVF: On Web (primarily on /search and /search/r/:reportID), a fatal Maximum update depth exceeded React error occurs due to an infinite setState re-render loop in useSidebarOrderedReports.tsx:328.

    What is the root cause of that problem?

    In src/hooks/useSidebarOrderedReports.tsx:327-329:

    useEffect(() => {
        setCurrentReportsToDisplay(reportsToDisplayInLHN);
    }, [reportsToDisplayInLHN]);
    1. reportsToDisplayInLHN is computed by a useMemo at line 229. While currentReportsToDisplay is intentionally excluded from the useMemo dependency array to prevent simple dependency loops, reportsToDisplayInLHN reads currentReportsToDisplay during computation (e.g. hasCachedReports = Object.keys(currentReportsToDisplay).length > 0).
    2. When on /search routes or when currentReportId / related collection keys trigger re-evaluation:
      • If shouldDoIncrementalUpdate is false (such as initial build, cache eviction, or when getReportsToDisplayInLHN executes from scratch), SidebarUtils.getReportsToDisplayInLHN allocates a new reportsToDisplay object dictionary (const reportsToDisplay: ReportsToDisplayInLHN = {}).
      • Even when shouldDoIncrementalUpdate is true, if any dynamic report property (requiresAttention, hasErrorsOtherThanFailedReceipt, or isUnreadReport) differs from the previous reference or creates a modified entry, getMutableCopy() produces a new object reference.
      • Crucially, in useSidebarOrderedReports.tsx:327-329, setCurrentReportsToDisplay(reportsToDisplayInLHN) is called unconditionally on every reference change of reportsToDisplayInLHN.
    3. If an upstream trigger (such as route change or unmemoized collection selector) causes useMemo to return a new object with identical content, or if SidebarUtils.getReportsToDisplayInLHN generates a new object identity each render, setCurrentReportsToDisplay triggers a re-render. On subsequent render, if getReportsToDisplayInLHN rebuilds from scratch, a new object reference is returned again, triggering setCurrentReportsToDisplay in a continuous cascading loop until React aborts with Maximum update depth exceeded.

    What changes do you think we should make in order to solve the problem?

    1. Guard setCurrentReportsToDisplay with shallow dictionary equality in useSidebarOrderedReports.tsx:
      In src/hooks/useSidebarOrderedReports.tsx:327-329:
      Import shallowCompare from @libs/ObjectUtils:

      useEffect(() => {
          setCurrentReportsToDisplay((prev) => {
              if (shallowCompare(prev, reportsToDisplayInLHN)) {
                  return prev;
              }
              return reportsToDisplayInLHN;
          });
      }, [reportsToDisplayInLHN]);
      • Why this is optimal: shallowCompare checks whether prev and reportsToDisplayInLHN share identical keys and report reference values. If reportsToDisplayInLHN returned a new object dictionary but its report entries have not actually changed, returning prev causes React to bail out of state updates completely, terminating the infinite loop immediately.
    2. Add referential stability to SidebarUtils.getReportsToDisplayInLHN:
      In src/libs/SidebarUtils.ts:358-359:
      Before returning reportsToDisplay, compare against lastReportsToDisplayInLHN:

      if (lastReportsToDisplayInLHN && shallowCompare(lastReportsToDisplayInLHN, reportsToDisplay)) {
          return lastReportsToDisplayInLHN;
      }
      lastReportsToDisplayInLHN = reportsToDisplay;
      return reportsToDisplay;

      This ensures that getReportsToDisplayInLHN preserves referential stability across successive renders when report contents remain identical.

    What specific scenarios should we cover in automated tests?

    In tests/unit/useSidebarOrderedReportsTest.tsx:

    1. Assert that when reportsToDisplayInLHN is recomputed with identical reports (new dictionary reference with identical key/value entries), currentReportsToDisplay does not trigger additional component re-renders.
    2. Assert that navigating to /search or changing currentReportId to an unlisted search report does not trigger circular setCurrentReportsToDisplay loops.
  9. melvin-bot commented on Oct 20, 2026

    @melvin-bot

    ✅ Contributor details stored successfully. Thank you for contributing to Expensify!

  10. stacklancer commented on Oct 21, 2026

    @stacklancer

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    On web, mostly on /search, the app crashes with Maximum update depth exceeded from the effect in useSidebarOrderedReports that copies reportsToDisplayInLHN into state.

    What is the root cause of that problem?

    The LHN cache never settles when at least one displayed report has a flag (unread, requires attention, or errors). Every recompute of reportsToDisplayInLHN returns a new object, so the effect sets new state on every pass.

    updateReportsToDisplayInLHN is meant to keep the same object when nothing changed (it has a lazy copy for that, SidebarUtils.ts#L401). But for a flagged report it stores a copy, {...report, requiresAttention, hasErrorsOtherThanFailedReceipt, isUnreadReport} (L456), and on the next pass it checks existingEntry !== report (L451). The stored entry is the copy, never the report itself, so that check is always true. Each call makes a new entry and a new map even with identical input.

    I confirmed this two ways:

    • Unit test: calling updateReportsToDisplayInLHN twice with the same inputs and one report that has requiresAttention: true returns a different object and a different entry the second time.
    • In the app (local web, latest main, one unread chat in the LHN), with a temporary log in the effect at L328. Toggling Account > Troubleshoot > Force offline on and off changes no report, but:
    BEFORE (main)
    cache write #1 | newObject=true | entries=2 | entriesReallyChanged=0
    cache write #2 | newObject=true | entries=2 | entriesReallyChanged=0
    cache write #3 | newObject=true | entries=2 | entriesReallyChanged=0
    cache write #4 | newObject=true | entries=2 | entriesReallyChanged=0
    
    AFTER (with the change below)
    no cache writes
    

    That's two state writes per toggle with nothing changed: one when isOffline changes, and another one render later when prevIsOffline catches up.

    How this turns into the loop:

    1. When getUpdatedReports() returns nothing, the memo re-checks every displayed report (useSidebarOrderedReports.tsx#L235), so any recompute goes through the code above.
    2. The result is always new, so the effect at L328 sets state and the provider renders again.
    3. Several memo inputs are "previous value" helpers (usePrevious, useCollectionDelta, prevReportAttributes) that only catch up one render later, so the extra render changes them again and the memo recomputes, which allocates again. On /search the provider gets a burst of report, transaction and report attribute updates while results load, and with nothing ever bailing out these cascades pile up past React's nested update limit.

    About the snapshot idea: SidebarOrderedReportsContextProvider is mounted in AuthScreens (AuthScreens.tsx#L192), outside SearchScopeProvider (Search/index.tsx#L1435). So useIsOnSearch() is false inside it and its useOnyx(ONYXKEYS.COLLECTION.REPORT) never reads the search snapshot. Also, a shallow-compare guard on the effect would not stop it, because the flagged entries themselves are new objects each time, so the shallow compare still sees a change.

    What changes do you think we should make in order to solve the problem?

    1. In SidebarUtils, remember which report each flagged entry was copied from (a module-level WeakMap<entry, Report>). Build flagged entries through one helper in both getReportsToDisplayInLHN and updateReportsToDisplayInLHN, and in the update path reuse the existing entry when its source report and the three flags are unchanged:
    const sourceReportByLHNEntry = new WeakMap<object, Report>();
    
    function buildFlaggedLHNEntry(report: Report, flags: LHNEntryFlags) {
        const entry = {...report, ...flags};
        sourceReportByLHNEntry.set(entry, report);
        return entry;
    }
    
    function isSameFlaggedLHNEntry(entry, report, flags) {
        return !!entry && sourceReportByLHNEntry.get(entry) === report
            && entry.requiresAttention === flags.requiresAttention
            && entry.hasErrorsOtherThanFailedReceipt === flags.hasErrorsOtherThanFailedReceipt
            && entry.isUnreadReport === flags.isUnreadReport;
    }
    
    // updateReportsToDisplayInLHN
    if (hasFlags && !isSameFlaggedLHNEntry(existingEntry, report, flags)) {
        getMutableCopy()[reportID] = buildFlaggedLHNEntry(report, flags);
    }

    With that, a recompute with no real change returns the same map, so the effect doesn't fire, setCurrentReportsToDisplay isn't called, and the chain never starts. The WeakMap lets old entries be garbage collected.
    2. Keep the effect as is. It only needs to run when the map really changes, which it now does.

    What specific scenarios should we cover in automated tests?

    • updateReportsToDisplayInLHN returns the same object and entry when called again with unchanged inputs and a flagged report (fails on main today).
    • It still returns a new entry when the report or one of the flags changes.
    • useSidebarOrderedReports: re-rendering the provider with no data changes does not call setCurrentReportsToDisplay with a new value.

    What alternative solutions did you explore? (Optional)

    • Keep the cache in a useRef instead of useState, so updating it never schedules a render. That removes the extra render entirely, but it changes how clearLHNCache works, so I'd keep it as a follow-up.
    • A shallow-compare guard in the effect. It does not help here because the flagged entries are new objects every time.
  11. magomedta commented on Oct 21, 2026

    @magomedta

    Proposal Update / Deep Dive (Following up on Proposal #1)

    Building upon our initial proposal above, here is the granular mechanism of how the cache churn in SidebarUtils.ts couples with the state loop in useSidebarOrderedReports.tsx:

    Extended Root Cause Analysis:

    1. The Flagged Copy Reference Trap in SidebarUtils.ts:451:
      In src/libs/SidebarUtils.ts:449-457:

      if (hasFlags) {
          if (
              existingEntry !== report ||
              existingEntry?.requiresAttention !== requiresAttention ||
              existingEntry?.hasErrorsOtherThanFailedReceipt !== hasErrorsOtherThanFailedReceipt ||
              existingEntry?.isUnreadReport !== isUnreadReport
          ) {
              getMutableCopy()[reportID] = {...report, requiresAttention, hasErrorsOtherThanFailedReceipt, isUnreadReport};
          }
      } else if (existingEntry !== report) {
          getMutableCopy()[reportID] = report;
      }

      When a report has flags (requiresAttention, hasErrorsOtherThanFailedReceipt, or isUnreadReport), line 456 stores a cloned object literal {...report, ...flags} into the cache.
      On subsequent incremental update passes (updateReportsToDisplayInLHN), line 451 performs:
      existingEntry !== report
      Because existingEntry is the cloned object created on the prior pass and report is the Onyx object reference, existingEntry !== report always evaluates to true.
      Consequently, getMutableCopy() is invoked on every single pass where any flagged report exists, creating a brand new displayedReportsCopy dictionary and allocating new copies even when neither the report data nor the flags have changed.

    2. Cascading State Effect in useSidebarOrderedReports.tsx:327-329:
      Because getReportsToDisplayInLHN / updateReportsToDisplayInLHN perpetually returns a new object dictionary reference, the effect:

      useEffect(() => {
          setCurrentReportsToDisplay(reportsToDisplayInLHN);
      }, [reportsToDisplayInLHN]);

      calls setCurrentReportsToDisplay on every commit. Updating this state triggers an immediate re-render of SidebarOrderedReportsContextProvider.
      On routes like /search, multiple rapid updates (transactions, report attributes, navigation events) enter this chain without settling, quickly exceeding React's maximum update depth (Error: Maximum update depth exceeded).

    Recommended Two-Pronged Solution:

    1. Fix Reference Tracking for Flagged Entries in src/libs/SidebarUtils.ts:
      Track the source report reference so that existingEntry can be verified without breaking reference equality:

      const sourceReportByLHNEntry = new WeakMap<object, Report>();
      
      function buildFlaggedLHNEntry(report: Report, flags: LHNEntryFlags): Report & LHNEntryFlags {
          const entry = {...report, ...flags};
          sourceReportByLHNEntry.set(entry, report);
          return entry;
      }
      
      function isFlaggedEntryUnchanged(existingEntry: (Report & LHNEntryFlags) | undefined, report: Report, requiresAttention: boolean, hasErrorsOtherThanFailedReceipt: boolean, isUnreadReport: boolean): boolean {
          if (!existingEntry) {
              return false;
          }
          const sourceReport = sourceReportByLHNEntry.get(existingEntry) ?? existingEntry;
          return (
              sourceReport === report &&
              existingEntry.requiresAttention === requiresAttention &&
              existingEntry.hasErrorsOtherThanFailedReceipt === hasErrorsOtherThanFailedReceipt &&
              existingEntry.isUnreadReport === isUnreadReport
          );
      }

      And in updateReportsToDisplayInLHN:

      if (hasFlags) {
          if (!isFlaggedEntryUnchanged(existingEntry, report, requiresAttention, hasErrorsOtherThanFailedReceipt, isUnreadReport)) {
              getMutableCopy()[reportID] = buildFlaggedLHNEntry(report, {requiresAttention, hasErrorsOtherThanFailedReceipt, isUnreadReport});
          }
      } else if (existingEntry !== report) {
          getMutableCopy()[reportID] = report;
      }
    2. Defensive Shallow Bailout in useSidebarOrderedReports.tsx:327-329:
      As proposed in our initial submission, guard setCurrentReportsToDisplay against redundant updates:

      useEffect(() => {
          setCurrentReportsToDisplay((prev) => {
              if (shallowCompare(prev, reportsToDisplayInLHN)) {
                  return prev;
              }
              return reportsToDisplayInLHN;
          });
      }, [reportsToDisplayInLHN]);

      This provides complete defense-in-depth: SidebarUtils eliminates unnecessary dictionary allocations, while useSidebarOrderedReports guarantees that even if an upstream recomputation yields a new container with identical entries, React skips the state update and breaks any potential render cycle immediately.

  12. stacklancer commented on Oct 21, 2026

    @stacklancer

    @parasharrajat a quick timing note for the review. The flagged entry root cause (existingEntry !== report always true at SidebarUtils L451) and the WeakMap fix with sourceReportByLHNEntry / buildFlaggedLHNEntry were first posted in my proposal at 00:16 UTC, with the before/after numbers from the app. The update above came 15 minutes later with the same names, and the earlier proposal from the same author pointed to the search snapshot instead. Thanks!

  13. sijo0218 commented on Oct 21, 2026

    @sijo0218

    Contributor details
    Your Expensify account email: [email protected]
    Upwork Profile Link: https://www.upwork.com/freelancers/~011091133bbb09cd1c

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    On web, predominantly on /search and /search/r/:reportID, the application crashes with a fatal React error:
    Error: Maximum update depth exceeded (React #185) originating from useSidebarOrderedReports.tsx:328 useEffect -> setCurrentReportsToDisplay(reportsToDisplayInLHN).


    What is the root cause of that problem?

    The infinite render loop is caused by a broken referential equality check in SidebarUtils.updateReportsToDisplayInLHN that prevents the LHN cache from ever settling when at least one displayed report has flags (unread, requires attention, or errors).

    1. The Broken Lazy Copy in SidebarUtils.ts:451:

    In src/libs/SidebarUtils.ts:401-460:

    // Use a lazy copy to avoid creating a new object reference when no entries actually change.
    let displayedReportsCopy: ReportsToDisplayInLHN | undefined;
    const getMutableCopy = (): ReportsToDisplayInLHN => {
        if (!displayedReportsCopy) {
            displayedReportsCopy = {...displayedReports};
        }
        return displayedReportsCopy;
    };
    ...
    if (hasFlags) {
        if (
            existingEntry !== report ||
            existingEntry?.requiresAttention !== requiresAttention ||
            existingEntry?.hasErrorsOtherThanFailedReceipt !== hasErrorsOtherThanFailedReceipt ||
            existingEntry?.isUnreadReport !== isUnreadReport
        ) {
            getMutableCopy()[reportID] = {...report, requiresAttention, hasErrorsOtherThanFailedReceipt, isUnreadReport};
        }
    } else if (existingEntry !== report) {
        getMutableCopy()[reportID] = report;
    }
    • For any report with flags (requiresAttention, hasErrorsOtherThanFailedReceipt, or isUnreadReport), line 456 stores a cloned object literal {...report, ...flags} in displayedReports.
    • On any subsequent update pass, line 451 performs:
      existingEntry !== report
    • Because existingEntry is the previously cloned object on the JS heap while report is the Onyx report reference, existingEntry !== report always evaluates to true.
    • As a consequence, getMutableCopy() is invoked on every single pass whenever any flagged report exists, allocating a brand-new displayedReportsCopy dictionary and allocating fresh object clones for every flagged report, even when neither the report data nor any flags changed.

    2. The Cascading Render Cycle in useSidebarOrderedReports.tsx:

    1. useSidebarOrderedReports computes reportsToDisplayInLHN in a useMemo (line 229). Because updateReportsToDisplayInLHN perpetually returns a new dictionary object reference with newly allocated entries, reportsToDisplayInLHN is referentially unstable.
    2. In useSidebarOrderedReports.tsx:327-329:
      useEffect(() => {
          setCurrentReportsToDisplay(reportsToDisplayInLHN);
      }, [reportsToDisplayInLHN]);
      Because reportsToDisplayInLHN changed reference, the effect runs and calls setCurrentReportsToDisplay(reportsToDisplayInLHN).
    3. Updating this state triggers an immediate re-render of SidebarOrderedReportsContextProvider.
    4. Several dependencies of getUpdatedReports are previous-value trackers (useCollectionDelta, usePrevious(derivedCurrentReportID), usePrevious(isOffline), reportAttributesUpdates) whose refs only update in a useEffect that fires after commit. Therefore, the re-render triggers another dependency change in getUpdatedReports one render later.
    5. In line 235:
      let effectiveUpdatedReports = updatedReports.length === 0 && hasCachedReports ? Object.keys(currentReportsToDisplay) : updatedReports;
      When updatedReports is empty, effectiveUpdatedReports rechecks all displayed reports, which re-invokes updateReportsToDisplayInLHN on every displayed report, triggering the flagged copy bug again and generating yet another new dictionary.

    3. Why This Crashes on /search:

    On /search and /search/r/:reportID, rapid async bursts of transactions, search result snapshots, and report route transitions occur concurrently. Because the LHN cache mechanism never settles into an identity match, these cascading renders pile up concurrently without ever bailing out, rapidly exceeding React's nested update limit of 50.

    (Note: The existing unit test in tests/unit/SidebarUtilsTest.ts:4680 passed because it tested an imaginary key report999 not in displayedReports, bypassing the report loop entirely.)


    What changes do you think we should make in order to solve the problem?

    We propose a robust, two-layered solution:

    Layer 1: Referential Source Tracking in src/libs/SidebarUtils.ts

    Track the original report reference for each flagged entry via a module-level WeakMap. This eliminates memory leaks, avoids polluting the Report schema or runtime object keys, and allows isSameFlaggedLHNEntry to detect whether the underlying report or flags have genuinely changed.

    --- a/src/libs/SidebarUtils.ts
    +++ b/src/libs/SidebarUtils.ts
    @@ -253,6 +253,27 @@ let isFirstTimeNewReportInAccessible = true;
     let lastReportsToDisplayInLHN: ReportsToDisplayInLHN | undefined;
    +
    +type LHNEntryFlags = {
    +    requiresAttention: boolean;
    +    hasErrorsOtherThanFailedReceipt: boolean;
    +    isUnreadReport: boolean;
    +};
    +
    +const sourceReportByLHNEntry = new WeakMap<object, OnyxTypes.Report>();
    +
    +function buildFlaggedLHNEntry(report: OnyxTypes.Report, flags: LHNEntryFlags): OnyxTypes.Report & LHNEntryFlags {
    +    const entry = {...report, ...flags};
    +    sourceReportByLHNEntry.set(entry, report);
    +    return entry;
    +}
    +
    +function isSameFlaggedLHNEntry(
    +    existingEntry: (OnyxTypes.Report & LHNEntryFlags) | undefined,
    +    report: OnyxTypes.Report,
    +    flags: LHNEntryFlags,
    +): boolean {
    +    if (!existingEntry) {
    +        return false;
    +    }
    +    const sourceReport = sourceReportByLHNEntry.get(existingEntry) ?? existingEntry;
    +    return (
    +        sourceReport === report &&
    +        existingEntry.requiresAttention === flags.requiresAttention &&
    +        existingEntry.hasErrorsOtherThanFailedReceipt === flags.hasErrorsOtherThanFailedReceipt &&
    +        existingEntry.isUnreadReport === flags.isUnreadReport
    +    );
    +}
     
    @@ -353,7 +374,7 @@ function getReportsToDisplayInLHN({
             if (shouldDisplay) {
                 const requiresAttention = reportAttributes?.[report?.reportID]?.requiresAttention ?? false;
                 const isUnreadReport = getIsUnreadReportForInboxTab(report, isReportArchived, reportAttributes?.[report?.reportID]?.isEmpty);
                 reportsToDisplay[reportID] =
    -                requiresAttention || hasErrorsOtherThanFailedReceipt || isUnreadReport ? {...report, requiresAttention, hasErrorsOtherThanFailedReceipt, isUnreadReport} : report;
    +                requiresAttention || hasErrorsOtherThanFailedReceipt || isUnreadReport ? buildFlaggedLHNEntry(report, {requiresAttention, hasErrorsOtherThanFailedReceipt, isUnreadReport}) : report;
             }
         }
     
    @@ -448,15 +469,10 @@ function updateReportsToDisplayInLHN({
                 const hasFlags = requiresAttention || hasErrorsOtherThanFailedReceipt || isUnreadReport;
                 const existingEntry = displayedReports[reportID];
     
                 if (hasFlags) {
    -                if (
    -                    existingEntry !== report ||
    -                    existingEntry?.requiresAttention !== requiresAttention ||
    -                    existingEntry?.hasErrorsOtherThanFailedReceipt !== hasErrorsOtherThanFailedReceipt ||
    -                    existingEntry?.isUnreadReport !== isUnreadReport
    -                ) {
    -                    getMutableCopy()[reportID] = {...report, requiresAttention, hasErrorsOtherThanFailedReceipt, isUnreadReport};
    +                if (!isSameFlaggedLHNEntry(existingEntry, report, {requiresAttention, hasErrorsOtherThanFailedReceipt, isUnreadReport})) {
    +                    getMutableCopy()[reportID] = buildFlaggedLHNEntry(report, {requiresAttention, hasErrorsOtherThanFailedReceipt, isUnreadReport});
                     }
                 } else if (existingEntry !== report) {
                     getMutableCopy()[reportID] = report;
                 }

    Layer 2: Defensive State Bail-Out in src/hooks/useSidebarOrderedReports.tsx

    Guard setCurrentReportsToDisplay with shallow dictionary comparison:

    --- a/src/hooks/useSidebarOrderedReports.tsx
    +++ b/src/hooks/useSidebarOrderedReports.tsx
    @@ -326,7 +326,17 @@ function SidebarOrderedReportsContextProvider({
         }, [reportsDrafts]);
     
         useEffect(() => {
    -        setCurrentReportsToDisplay(reportsToDisplayInLHN);
    +        setCurrentReportsToDisplay((prev) => {
    +            if (prev === reportsToDisplayInLHN) {
    +                return prev;
    +            }
    +            const prevKeys = Object.keys(prev);
    +            const nextKeys = Object.keys(reportsToDisplayInLHN);
    +            if (prevKeys.length === nextKeys.length && nextKeys.every((key) => prev[key] === reportsToDisplayInLHN[key])) {
    +                return prev;
    +            }
    +            return reportsToDisplayInLHN;
    +        });
         }, [reportsToDisplayInLHN]);

    Why both layers:

    1. Layer 1 preserves referential identity of unchanged flagged entries, allowing displayedReportsCopy to stay undefined so updateReportsToDisplayInLHN returns the original displayedReports reference.
    2. Layer 2 provides defense-in-depth: if any upstream dependency recomputes and produces a new dictionary container, React immediately detects that all entry references are identical and bails out of updating state.

    What specific scenarios should we cover in automated tests to prevent reintroduction of this issue in the future?

    1. tests/unit/SidebarUtilsTest.ts:

      • Call updateReportsToDisplayInLHN with a report that has requiresAttention: true or isUnreadReport: true.
      • Call updateReportsToDisplayInLHN a second time with the exact same inputs and report reference.
      • Assert expect(result2).toBe(result1) (currently fails on main).
      • Call updateReportsToDisplayInLHN with updated report properties or changed flag values; assert expect(result3).not.toBe(result2) and the updated flag is reflected.
    2. tests/unit/useSidebarOrderedReportsTest.tsx:

      • Render SidebarOrderedReportsContextProvider with an unread chat in the LHN.
      • Simulate a re-render or trigger an unrelated collection update that doesn't modify the displayed chat.
      • Assert setCurrentReportsToDisplay does not trigger cascading re-renders or exceed React's maximum update depth.

    What alternative solutions did you explore? (Optional)

    1. Only using a shallow equality check in useSidebarOrderedReports.tsx:
      • As explained above, shallow compare on its own does not work without Layer 1 because updateReportsToDisplayInLHN generates freshly cloned entry objects {...report, ...flags} on every pass, causing prev[key] === next[key] to fail for every flagged report.
    2. Replacing currentReportsToDisplay useState with useRef:
      • While storing currentReportsToDisplay in a ref avoids scheduling renders, useSidebarOrderedReportsState() exposes orderedReportIDs and related state to consumers, and altering state synchronization would break clearLHNCache and downstream consumers.
  14. melvin-bot commented on Oct 21, 2026

    @melvin-bot

    ✅ Contributor details stored successfully. Thank you for contributing to Expensify!

  15. stacklancer commented on Oct 22, 2026

    @stacklancer

    @parasharrajat here's a test branch for my proposal: main...stacklancer:App:fix/104504-lhn-cache-identity

    It has the SidebarUtils change plus unit tests for flagged entries (same object when nothing changed, new entry when the report changes, and entries from a full rebuild are reused). The new tests fail on main and pass on the branch, and the existing LHN/sidebar suites still pass. In the app, toggling Force offline now causes no cache writes instead of four.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

DailyKSv2ExternalAdded to denote the issue can be worked on by a contributorOverdue

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions