From ba64abb96813f469797869ade7d9f204b6907417 Mon Sep 17 00:00:00 2001 From: Christiaan Landman Date: Thu, 4 Sep 2025 11:08:21 +0200 Subject: [PATCH 1/2] Improved the abort handling for stale watched query results when the query/parameters change. This fixes the edge case where an already fetching query would handle a query change and briefly report `isFetching` being false before becoming true again. --- .changeset/real-dolls-peel.md | 5 +++++ .../src/client/watched/processors/AbstractQueryProcessor.ts | 4 ++-- .../client/watched/processors/DifferentialQueryProcessor.ts | 6 +++++- .../src/client/watched/processors/OnChangeQueryProcessor.ts | 6 +++++- 4 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 .changeset/real-dolls-peel.md diff --git a/.changeset/real-dolls-peel.md b/.changeset/real-dolls-peel.md new file mode 100644 index 000000000..ae860db50 --- /dev/null +++ b/.changeset/real-dolls-peel.md @@ -0,0 +1,5 @@ +--- +'@powersync/common': patch +--- + +Improved the abort handling for stale watched query results when the query/parameters change. This fixes the edge case where an already fetching query would handle a query change and briefly report `isFetching` being false before becoming true again. diff --git a/packages/common/src/client/watched/processors/AbstractQueryProcessor.ts b/packages/common/src/client/watched/processors/AbstractQueryProcessor.ts index 0e5c1ab8a..3f089a3bf 100644 --- a/packages/common/src/client/watched/processors/AbstractQueryProcessor.ts +++ b/packages/common/src/client/watched/processors/AbstractQueryProcessor.ts @@ -83,6 +83,7 @@ export abstract class AbstractQueryProcessor< * Updates the underlying query. */ async updateSettings(settings: Settings) { + this.abortController.abort(); await this.initialized; if (!this.state.isFetching && this.reportFetching) { @@ -92,7 +93,7 @@ export abstract class AbstractQueryProcessor< } this.options.watchOptions = settings; - this.abortController.abort(); + this.abortController = new AbortController(); await this.runWithReporting(() => this.linkQuery({ @@ -121,7 +122,6 @@ export abstract class AbstractQueryProcessor< if (typeof update.data !== 'undefined') { await this.iterateAsyncListenersWithError(async (l) => l.onData?.(this.state.data)); } - await this.iterateAsyncListenersWithError(async (l) => l.onStateChange?.(this.state)); } diff --git a/packages/common/src/client/watched/processors/DifferentialQueryProcessor.ts b/packages/common/src/client/watched/processors/DifferentialQueryProcessor.ts index 48367e552..9d7701bf0 100644 --- a/packages/common/src/client/watched/processors/DifferentialQueryProcessor.ts +++ b/packages/common/src/client/watched/processors/DifferentialQueryProcessor.ts @@ -236,7 +236,7 @@ export class DifferentialQueryProcessor db.onChangeWithCallback( { onChange: async () => { - if (this.closed) { + if (this.closed || abortSignal.aborted) { return; } // This fires for each change of the relevant tables @@ -256,6 +256,10 @@ export class DifferentialQueryProcessor db: this.options.db }); + if (abortSignal.aborted) { + return; + } + if (this.reportFetching) { partialStateUpdate.isFetching = false; } diff --git a/packages/common/src/client/watched/processors/OnChangeQueryProcessor.ts b/packages/common/src/client/watched/processors/OnChangeQueryProcessor.ts index 0763af4c6..bdf46a96a 100644 --- a/packages/common/src/client/watched/processors/OnChangeQueryProcessor.ts +++ b/packages/common/src/client/watched/processors/OnChangeQueryProcessor.ts @@ -57,7 +57,7 @@ export class OnChangeQueryProcessor extends AbstractQueryProcessor { - if (this.closed) { + if (this.closed || abortSignal.aborted) { return; } // This fires for each change of the relevant tables @@ -77,6 +77,10 @@ export class OnChangeQueryProcessor extends AbstractQueryProcessor Date: Thu, 4 Sep 2025 11:25:02 +0200 Subject: [PATCH 2/2] Added unit tests. --- packages/react/tests/useQuery.test.tsx | 85 ++++++++++++++++++++++++ packages/vue/tests/useQuery.test.ts | 92 +++++++++++++++++++++++++- 2 files changed, 176 insertions(+), 1 deletion(-) diff --git a/packages/react/tests/useQuery.test.tsx b/packages/react/tests/useQuery.test.tsx index 1b5ce36bf..d5619e224 100644 --- a/packages/react/tests/useQuery.test.tsx +++ b/packages/react/tests/useQuery.test.tsx @@ -542,6 +542,91 @@ describe('useQuery', () => { // It should be the same data array reference, no update should have happened expect(result.current.data == previousData).false; }); + + it('should handle dependent query parameter changes with correct state transitions', async () => { + const db = openPowerSync(); + + await db.execute(/* sql */ ` + INSERT INTO + lists (id, name) + VALUES + (uuid (), 'item1') + `); + + // Track state transitions + const stateTransitions: Array<{ + param: string | number; + dataLength: number; + isFetching: boolean; + isLoading: boolean; + }> = []; + + const testHook = () => { + // First query that provides the parameter - starts with 0, then returns 1 + const { data: paramData } = useQuery('SELECT 1 as result;', []); + const param = paramData?.[0]?.result ?? 0; + + // Second query that depends on the first query's result + const { data, isFetching, isLoading } = useQuery('SELECT * FROM lists LIMIT ?', [param]); + + const currentState = { + param: param, + dataLength: data?.length || 0, + isFetching: isFetching, + isLoading: isLoading + }; + + stateTransitions.push(currentState); + return currentState; + }; + + const { result } = renderHook(() => testHook(), { + wrapper: ({ children }) => testWrapper({ children, db }) + }); + + // Wait for final state + await waitFor( + () => { + const { current } = result; + expect(current.isLoading).toEqual(false); + expect(current.isFetching).toEqual(false); + expect(current.param).toEqual(1); + expect(current.dataLength).toEqual(1); + }, + { timeout: 500, interval: 100 } + ); + + // Find the index where param changes from 0 to 1 + let beforeParamChangeIndex = 0; + for (const transition of stateTransitions) { + if (transition.param === 1) { + beforeParamChangeIndex = stateTransitions.indexOf(transition) - 1; + break; + } + } + + const indexMultiplier = isStrictMode ? 2 : 1; // StrictMode causes 1 extra render per state + const initialState = stateTransitions[beforeParamChangeIndex]; + expect(initialState).toBeDefined(); + expect(initialState?.param).toEqual(0); + expect(initialState?.dataLength).toEqual(0); + expect(initialState?.isFetching).toEqual(true); + expect(initialState?.isLoading).toEqual(true); + + const paramChangedState = stateTransitions[beforeParamChangeIndex + 1 * indexMultiplier]; + expect(paramChangedState).toBeDefined(); + expect(paramChangedState?.param).toEqual(1); + expect(paramChangedState?.dataLength).toEqual(0); + expect(paramChangedState?.isFetching).toEqual(true); + expect(paramChangedState?.isLoading).toEqual(true); + + const finalState = stateTransitions[beforeParamChangeIndex + 2 * indexMultiplier]; + expect(finalState).toBeDefined(); + expect(finalState.param).toEqual(1); + expect(finalState.dataLength).toEqual(1); + expect(finalState.isFetching).toEqual(false); + expect(finalState.isLoading).toEqual(false); + }); }); }); diff --git a/packages/vue/tests/useQuery.test.ts b/packages/vue/tests/useQuery.test.ts index 648ddb6e8..bd35793aa 100644 --- a/packages/vue/tests/useQuery.test.ts +++ b/packages/vue/tests/useQuery.test.ts @@ -2,7 +2,7 @@ import * as commonSdk from '@powersync/common'; import { PowerSyncDatabase } from '@powersync/web'; import flushPromises from 'flush-promises'; import { describe, expect, it, onTestFinished, vi } from 'vitest'; -import { isProxy, isRef, ref } from 'vue'; +import { computed, isProxy, isRef, ref, watchEffect } from 'vue'; import { createPowerSyncPlugin } from '../src/composables/powerSync'; import { useQuery } from '../src/composables/useQuery'; import { useWatchedQuerySubscription } from '../src/composables/useWatchedQuerySubscription'; @@ -233,4 +233,94 @@ describe('useQuery', () => { 'PowerSync failed to fetch data: You cannot pass parameters to a compiled query.' ); }); + + it('should handle dependent query parameter changes with correct state transitions', async () => { + const db = openPowerSync(); + + await db.execute(/* sql */ ` + INSERT INTO + lists (id, name) + VALUES + (uuid (), 'item1') + `); + + // Track state transitions + const stateTransitions: Array<{ + param: string | number; + dataLength: number; + isFetching: boolean; + isLoading: boolean; + }> = []; + + const [state] = withPowerSyncSetup(() => { + // First query that provides the parameter - starts with 0, then returns 1 + const paramQuery = useQuery('SELECT 1 as result;', []); + const param = computed(() => paramQuery.data.value?.[0]?.result ?? 0); + + // Second query that depends on the first query's result + const dataQuery = useQuery( + computed(() => 'SELECT * FROM lists LIMIT ?'), + computed(() => [param.value]) + ); + + // Track state changes + watchEffect(() => { + const currentState = { + param: param.value, + dataLength: dataQuery.data.value?.length || 0, + isFetching: dataQuery.isFetching.value, + isLoading: dataQuery.isLoading.value + }; + stateTransitions.push(currentState); + }); + + return { + paramData: paramQuery.data, + data: dataQuery.data, + isFetching: dataQuery.isFetching, + isLoading: dataQuery.isLoading, + param + }; + }); + + // Wait for final state + await vi.waitFor( + () => { + expect(state.isLoading.value).toEqual(false); + expect(state.isFetching.value).toEqual(false); + expect(state.data.value?.length).toEqual(1); + expect(state.paramData.value[0].result).toEqual(1); + }, + { timeout: 1000 } + ); + + // Find the index where param changes from 0 to 1 + let beforeParamChangeIndex = 0; + for (const transition of stateTransitions) { + if (transition.param === 1) { + beforeParamChangeIndex = stateTransitions.indexOf(transition) - 1; + break; + } + } + const initialState = stateTransitions[beforeParamChangeIndex]; + expect(initialState).toBeDefined(); + expect(initialState?.param).toEqual(0); + expect(initialState?.dataLength).toEqual(0); + expect(initialState?.isFetching).toEqual(true); + expect(initialState?.isLoading).toEqual(true); + + const paramChangedState = stateTransitions[beforeParamChangeIndex + 1]; + expect(paramChangedState).toBeDefined(); + expect(paramChangedState?.param).toEqual(1); + expect(paramChangedState?.dataLength).toEqual(0); + expect(paramChangedState?.isFetching).toEqual(true); + expect(paramChangedState?.isLoading).toEqual(true); + + const finalState = stateTransitions[beforeParamChangeIndex + 2]; + expect(finalState).toBeDefined(); + expect(finalState.param).toEqual(1); + expect(finalState.dataLength).toEqual(1); + expect(finalState.isFetching).toEqual(false); + expect(finalState.isLoading).toEqual(false); + }); });