diff --git a/.changeset/polite-eels-laugh.md b/.changeset/polite-eels-laugh.md new file mode 100644 index 000000000..472390b3b --- /dev/null +++ b/.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. diff --git a/packages/db/src/collection/lifecycle.ts b/packages/db/src/collection/lifecycle.ts index bdc207670..f425f4d68 100644 --- a/packages/db/src/collection/lifecycle.ts +++ b/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() + } } } diff --git a/packages/db/src/collection/state.ts b/packages/db/src/collection/state.ts index 10a7c0491..a077c0a4a 100644 --- a/packages/db/src/collection/state.ts +++ b/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() } } diff --git a/packages/electric-db-collection/tests/electric-live-query.test.ts b/packages/electric-db-collection/tests/electric-live-query.test.ts index 253a8fb12..b387f1756 100644 --- a/packages/electric-db-collection/tests/electric-live-query.test.ts +++ b/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 = [] + 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 = [] + 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() + }) })