-
Notifications
You must be signed in to change notification settings - Fork 106
fix change events #206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix change events #206
Conversation
🦋 Changeset detectedLatest commit: 6172059 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@tanstack/db-example-react-todo commit: |
|
Size Change: +841 B (+3.19%) Total Size: 27.2 kB
ℹ️ View Unchanged
|
|
Size Change: 0 B Total Size: 822 B ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love it!
Claude made me a nice mermaid diagram to analyze the flow:
flowchart TD
A[User Makes Optimistic Mutation] --> B[Emit Optimistic Change Event]
B --> C[Transaction State: persisting]
%% NEW: Pre-sync state capture
C --> D[🆕 capturePreSyncVisibleState]
D --> E[🆕 Mark recentlySyncedKeys]
E --> F[🆕 Capture preSyncVisibleState for affected keys]
%% Server responds
F --> G[Server Response Received]
G --> H[onTransactionStateChange]
%% First: recomputeOptimisticState (with filtering)
H --> I{🆕 isCommittingSyncTransactions?}
I -->|Yes| J[🆕 Skip Redundant Recalculation]
I -->|No| K[recomputeOptimisticState]
%% IMPROVED: Better event filtering in recompute
K --> L[🆕 Filter recentlySyncedKeys]
L --> M[🆕 Filter redundant delete events]
M --> N[Emit filtered events]
%% Then: commitPendingTransactions (if any pending)
J --> O[commitPendingTransactions]
N --> O
O --> P[🆕 Set isCommittingSyncTransactions = true]
P --> Q[Apply sync operations to syncedData]
Q --> R[🆕 Clear optimistic state]
R --> S[🆕 Recompute remaining active transactions]
S --> T[🆕 Set isCommittingSyncTransactions = false]
%% NEW: Smart change detection
T --> U[🆕 Compare preSyncVisibleState vs current state]
U --> V{🆕 Sync matches optimistic via deepEqual?}
V -->|Yes| W[🆕 No events emitted - redundant sync]
V -->|No| X[🆕 Emit actual change events only]
%% Cleanup
W --> Y[🆕 Clear preSyncVisibleState]
X --> Y
Y --> Z[🆕 Microtask: Clear recentlySyncedKeys]
%% Before vs After
AA[❌ BEFORE: Duplicate Events] --> BB[Delete optimistic state]
BB --> CC[Insert sync state]
CC --> DD[User sees confusing duplicate events]
EE[✅ AFTER: Clean Events] --> FF[Smart state comparison]
FF --> GG[Only emit real changes]
GG --> HH[User sees smooth updates]
%% Styling
classDef new fill:#e8f5e8,stroke:#4caf50,stroke-width:3px
classDef old fill:#ffebee,stroke:#f44336,stroke-width:2px
classDef good fill:#e3f2fd,stroke:#2196f3,stroke-width:2px
class D,E,F,I,J,L,M,P,R,S,T,U,V,W,X,Y,Z new
class AA,BB,CC,DD old
class EE,FF,GG,HH good
| const newVisibleValue = this.get(key) // This returns the new derived state | ||
|
|
||
| // Check if this sync operation is redundant with a completed optimistic operation | ||
| const completedOp = completedOptimisticOps.get(key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice optimization! If the synced data is identical, no new renders 🙏
We have experienced various issues with the change events where they could be doubled up or missed. I think this fixes it all...