Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/real-dolls-peel.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -92,7 +93,7 @@ export abstract class AbstractQueryProcessor<
}

this.options.watchOptions = settings;
this.abortController.abort();

this.abortController = new AbortController();
await this.runWithReporting(() =>
this.linkQuery({
Expand Down Expand Up @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export class DifferentialQueryProcessor<RowType>
db.onChangeWithCallback(
{
onChange: async () => {
if (this.closed) {
if (this.closed || abortSignal.aborted) {
return;
}
// This fires for each change of the relevant tables
Expand All @@ -256,6 +256,10 @@ export class DifferentialQueryProcessor<RowType>
db: this.options.db
});

if (abortSignal.aborted) {
return;
}

if (this.reportFetching) {
partialStateUpdate.isFetching = false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class OnChangeQueryProcessor<Data> extends AbstractQueryProcessor<Data, W
db.onChangeWithCallback(
{
onChange: async () => {
if (this.closed) {
if (this.closed || abortSignal.aborted) {
return;
}
// This fires for each change of the relevant tables
Expand All @@ -77,6 +77,10 @@ export class OnChangeQueryProcessor<Data> extends AbstractQueryProcessor<Data, W
db: this.options.db
});

if (abortSignal.aborted) {
return;
}

if (this.reportFetching) {
partialStateUpdate.isFetching = false;
}
Expand Down
85 changes: 85 additions & 0 deletions packages/react/tests/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});

Expand Down
92 changes: 91 additions & 1 deletion packages/vue/tests/useQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
});
});