fix repeated renders when markReady called when collection already ready (#604)

* fix repeated renders when markReady called when already ready

* changeset

* tidy

* fix the regression

* address review

Author: samwillis

.changeset/polite-eels-laugh.md

@@ -0,0 +1,6 @@
+---
+"@tanstack/electric-db-collection": patch
+"@tanstack/db": patch
+---
+
+Fix repeated renders when markReady called when the collection was already ready. This would occur after each long poll on an Electric collection.

packages/db/src/collection/lifecycle.ts

@@ -1,5 +1,6 @@
 import {
   CollectionInErrorStateError,
+  CollectionStateError,
   InvalidCollectionStatusTransitionError,
 } from "../errors"
 import {
@@ -90,7 +91,18 @@ export class CollectionLifecycleManager<
    * Safely update the collection status with validation
    * @private
    */
-  public setStatus(newStatus: CollectionStatus): void {
+  public setStatus(
+    newStatus: CollectionStatus,
+    allowReady: boolean = false
+  ): void {
+    if (newStatus === `ready` && !allowReady) {
+      // setStatus('ready') is an internal method that should not be called directly
+      // Instead, use markReady to transition to ready triggering the necessary events
+      // and side effects.
+      throw new CollectionStateError(
+        `You can't directly call "setStatus('ready'). You must use markReady instead.`
+      )
+    }
     this.validateStatusTransition(this.status, newStatus)
     const previousStatus = this.status
     this.status = newStatus
@@ -129,9 +141,10 @@ export class CollectionLifecycleManager<
    * @private - Should only be called by sync implementations
    */
   public markReady(): void {
+    this.validateStatusTransition(this.status, `ready`)
     // Can transition to ready from loading or initialCommit states
     if (this.status === `loading` || this.status === `initialCommit`) {
-      this.setStatus(`ready`)
+      this.setStatus(`ready`, true)
 
       // Call any registered first ready callbacks (only on first time becoming ready)
       if (!this.hasBeenReady) {
@@ -146,12 +159,11 @@ export class CollectionLifecycleManager<
         this.onFirstReadyCallbacks = []
         callbacks.forEach((callback) => callback())
       }
-    }
-
-    // Always notify dependents when markReady is called, after status is set
-    // This ensures live queries get notified when their dependencies become ready
-    if (this.changes.changeSubscriptions.size > 0) {
-      this.changes.emitEmptyReadyEvent()
+      // Notify dependents when markReady is called, after status is set
+      // This ensures live queries get notified when their dependencies become ready
+      if (this.changes.changeSubscriptions.size > 0) {
+        this.changes.emitEmptyReadyEvent()
+      }
     }
   }
 

packages/db/src/collection/state.ts

@@ -644,7 +644,7 @@ export class CollectionStateManager<
 
         // Ensure listeners are active before emitting this critical batch
         if (this.lifecycle.status !== `ready`) {
-          this.lifecycle.setStatus(`ready`)
+          this.lifecycle.markReady()
         }
       }
 

packages/electric-db-collection/tests/electric-live-query.test.ts

@@ -144,6 +144,11 @@ describe.each([
     subscriber(messages)
   }
 
+  function simulateUpToDateOnly() {
+    // Send only an up-to-date message with no data changes
+    subscriber([{ headers: { control: `up-to-date` } }])
+  }
+
   beforeEach(() => {
     electricCollection = createElectricUsersCollection()
   })
@@ -338,4 +343,98 @@ describe.each([
     expect(testElectricCollection.status).toBe(`ready`)
     expect(liveQuery.status).toBe(`ready`)
   })
+
+  it(`should not emit changes on up-to-date messages with no data changes`, async () => {
+    // Test to verify that up-to-date messages without actual data changes
+    // don't trigger unnecessary renders in live query collections
+
+    // Create a live query collection
+    const liveQuery = createLiveQueryCollection({
+      startSync: true,
+      query: (q) =>
+        q
+          .from({ user: electricCollection })
+          .where(({ user }) => eq(user.active, true))
+          .select(({ user }) => ({
+            id: user.id,
+            name: user.name,
+            active: user.active,
+          })),
+    })
+
+    // Track changes emitted by the live query
+    const changeNotifications: Array<any> = []
+    const subscription = liveQuery.subscribeChanges((changes) => {
+      changeNotifications.push(changes)
+    })
+
+    // Initial sync with data
+    simulateInitialSync()
+    expect(liveQuery.status).toBe(`ready`)
+    expect(liveQuery.size).toBe(3) // Only active users
+
+    // Clear any initial change notifications
+    changeNotifications.length = 0
+
+    // Send an up-to-date message with no data changes
+    // This simulates the scenario where Electric sends up-to-date
+    // but there are no actual changes to the data
+    simulateUpToDateOnly()
+
+    // Wait a tick to ensure any async operations complete
+    await new Promise((resolve) => setTimeout(resolve, 0))
+
+    // The live query should not have emitted any changes
+    // because there were no actual data changes
+    expect(changeNotifications).toHaveLength(0)
+
+    // Verify the collection is still in ready state
+    expect(liveQuery.status).toBe(`ready`)
+    expect(liveQuery.size).toBe(3)
+
+    // Clean up
+    subscription.unsubscribe()
+  })
+
+  it(`should not emit changes on multiple consecutive up-to-date messages with no data changes`, async () => {
+    // Test to verify that multiple consecutive up-to-date messages
+    // without data changes don't accumulate unnecessary renders
+
+    const liveQuery = createLiveQueryCollection({
+      startSync: true,
+      query: (q) => q.from({ user: electricCollection }),
+    })
+
+    // Track changes emitted by the live query
+    const changeNotifications: Array<any> = []
+    const subscription = liveQuery.subscribeChanges((changes) => {
+      changeNotifications.push(changes)
+    })
+
+    // Initial sync
+    simulateInitialSync()
+    expect(liveQuery.status).toBe(`ready`)
+    expect(liveQuery.size).toBe(4)
+
+    // Clear initial change notifications
+    changeNotifications.length = 0
+
+    // Send multiple up-to-date messages with no data changes
+    simulateUpToDateOnly()
+    simulateUpToDateOnly()
+    simulateUpToDateOnly()
+
+    // Wait for any async operations
+    await new Promise((resolve) => setTimeout(resolve, 0))
+
+    // Should not have emitted any changes despite multiple up-to-date messages
+    expect(changeNotifications).toHaveLength(0)
+
+    // Verify collection state is still correct
+    expect(liveQuery.status).toBe(`ready`)
+    expect(liveQuery.size).toBe(4)
+
+    // Clean up
+    subscription.unsubscribe()
+  })
 })