From 74fb1ade04b2a35a11b49b1967d7e0bccce021cf Mon Sep 17 00:00:00 2001 From: Sam Willis Date: Mon, 29 Sep 2025 19:08:56 +0100 Subject: [PATCH 1/3] fix repeated renders when markReady called when already ready --- packages/db/src/collection/lifecycle.ts | 10 +- .../tests/electric-live-query.test.ts | 113 ++++++++++++++++++ 2 files changed, 118 insertions(+), 5 deletions(-) diff --git a/packages/db/src/collection/lifecycle.ts b/packages/db/src/collection/lifecycle.ts index bdc207670..3ebceedf3 100644 --- a/packages/db/src/collection/lifecycle.ts +++ b/packages/db/src/collection/lifecycle.ts @@ -146,12 +146,12 @@ 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/electric-db-collection/tests/electric-live-query.test.ts b/packages/electric-db-collection/tests/electric-live-query.test.ts index 253a8fb12..3b3ff4f66 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,112 @@ describe.each([ expect(testElectricCollection.status).toBe(`ready`) expect(liveQuery.status).toBe(`ready`) }) + + it.only(`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 + // BUG: Currently emits empty change arrays on up-to-date messages + if (changeNotifications.length > 0) { + console.log( + `BUG DETECTED: Live query emitted ${changeNotifications.length} change notifications on up-to-date with no data changes:`, + changeNotifications + ) + } + 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 + // BUG: Currently emits empty change arrays for each up-to-date message + if (changeNotifications.length > 0) { + console.log( + `BUG DETECTED: Live query emitted ${changeNotifications.length} change notifications on multiple up-to-date messages with no data changes:`, + changeNotifications + ) + } + expect(changeNotifications).toHaveLength(0) + + // Verify collection state is still correct + expect(liveQuery.status).toBe(`ready`) + expect(liveQuery.size).toBe(4) + + // Clean up + subscription.unsubscribe() + }) }) From b5754456ef406ccdceca34dc44741f82748e8e22 Mon Sep 17 00:00:00 2001 From: Sam Willis Date: Mon, 29 Sep 2025 19:10:31 +0100 Subject: [PATCH 2/3] changeset --- .changeset/polite-eels-laugh.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/polite-eels-laugh.md diff --git a/.changeset/polite-eels-laugh.md b/.changeset/polite-eels-laugh.md new file mode 100644 index 000000000..381432686 --- /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 a Electric collection. From d854f2b69a3f68dc5f4d696ecf7de8fffa47155e Mon Sep 17 00:00:00 2001 From: Sam Willis Date: Mon, 29 Sep 2025 19:18:06 +0100 Subject: [PATCH 3/3] tidy --- .../tests/electric-live-query.test.ts | 14 -------------- 1 file changed, 14 deletions(-) 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 3b3ff4f66..e5ccd490d 100644 --- a/packages/electric-db-collection/tests/electric-live-query.test.ts +++ b/packages/electric-db-collection/tests/electric-live-query.test.ts @@ -386,13 +386,6 @@ describe.each([ // The live query should not have emitted any changes // because there were no actual data changes - // BUG: Currently emits empty change arrays on up-to-date messages - if (changeNotifications.length > 0) { - console.log( - `BUG DETECTED: Live query emitted ${changeNotifications.length} change notifications on up-to-date with no data changes:`, - changeNotifications - ) - } expect(changeNotifications).toHaveLength(0) // Verify the collection is still in ready state @@ -435,13 +428,6 @@ describe.each([ await new Promise((resolve) => setTimeout(resolve, 0)) // Should not have emitted any changes despite multiple up-to-date messages - // BUG: Currently emits empty change arrays for each up-to-date message - if (changeNotifications.length > 0) { - console.log( - `BUG DETECTED: Live query emitted ${changeNotifications.length} change notifications on multiple up-to-date messages with no data changes:`, - changeNotifications - ) - } expect(changeNotifications).toHaveLength(0) // Verify collection state is still correct