Skip to content

Redundant collection updates in markRead/setStarred onSuccess handlers #573

@brendanlong

Description

@brendanlong

This is related to #580 and a PR fixing this should target the tanstack-db branch.

Problem

The markRead and setStarred mutation onSuccess handlers in src/lib/hooks/useEntryMutations.ts perform two rounds of collection updates for the same data.

markRead (lines 321-347)

Round 1 (lines 323-337): Processes each entry through recordMutationResultapplyWinningStateToCache, which calls updateEntryReadInCollection and updateEntryScoreInCollection.

Round 2 (lines 339-343): Unconditionally calls updateEntryReadInCollection and updateEntryScoreInCollection again for every entry.

onSuccess: (data) => {
  // Round 1: timestamp-based merging (already updates collections)
  for (const entry of data.entries) {
    const { allComplete, winningState } = recordMutationResult(entry.id, result);
    if (allComplete && winningState) {
      applyWinningStateToCache(entry.id, winningState); // ← updates collections
    }
  }

  // Round 2: redundant updates
  for (const entry of data.entries) {
    updateEntryReadInCollection(collections, [entry.id], entry.read);    // ← duplicate
    updateEntryScoreInCollection(collections, entry.id, entry.score, entry.implicitScore); // ← duplicate
  }

  setBulkCounts(collections, data.counts);
},

setStarred (lines 418-443)

Same pattern — applyWinningStateToCache already updates starred + score, then lines 433-440 update them again unconditionally.

Impact

  • When allComplete is true: the second round writes identical values (wasted work, triggers unnecessary reactive updates in TanStack DB)
  • When allComplete is false (other mutations still in flight): the second round writes server state directly, bypassing the timestamp-based merging logic that was specifically designed to handle this case. This could cause flickering if a newer mutation's optimistic state gets overwritten by an older mutation's server response.

Fix

Remove the second loop in both markRead.onSuccess (lines 339-343) and setStarred.onSuccess (lines 433-440). The applyWinningStateToCache path handles the complete case, and the incomplete case should wait for all mutations to finish (the entire purpose of the tracking system).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions