fix: Ensuring that useQuery's isFetching becomes true after query…

… changes (#331)

.changeset/quick-cows-design.md

@@ -0,0 +1,5 @@
+---
+'@powersync/react': patch
+---
+
+Ensuring that `useQuery`'s `isFetching` becomes true immediately after the query changes.

packages/react/src/hooks/useQuery.ts

@@ -66,20 +66,33 @@ export const useQuery = <T = any>(
   const memoizedOptions = React.useMemo(() => options, [JSON.stringify(options)]);
   const abortController = React.useRef(new AbortController());
 
+  const previousQueryRef = React.useRef({ sqlStatement, memoizedParams });
+
+  // Indicates that the query will be re-fetched due to a change in the query.
+  // Used when `isFetching` hasn't been set to true yet due to React execution.
+  const shouldFetch = React.useMemo(
+    () =>
+      previousQueryRef.current.sqlStatement !== sqlStatement ||
+      JSON.stringify(previousQueryRef.current.memoizedParams) != JSON.stringify(memoizedParams),
+    [powerSync, sqlStatement, memoizedParams, isFetching]
+  );
+
   const handleResult = (result: T[]) => {
+    setData(result);
     setIsLoading(false);
     setIsFetching(false);
-    setData(result);
     setError(undefined);
+    previousQueryRef.current = { sqlStatement, memoizedParams };
   };
 
   const handleError = (e: Error) => {
+    setData([]);
     setIsLoading(false);
     setIsFetching(false);
-    setData([]);
     const wrappedError = new Error('PowerSync failed to fetch data: ' + e.message);
     wrappedError.cause = e;
     setError(wrappedError);
+    previousQueryRef.current = { sqlStatement, memoizedParams };
   };
 
   const fetchData = async () => {
@@ -139,5 +152,5 @@ export const useQuery = <T = any>(
     };
   }, [powerSync, sqlStatement, memoizedParams, memoizedOptions, tables]);
 
-  return { isLoading, isFetching, data, error, refresh: fetchData };
+  return { isLoading, isFetching: isFetching || shouldFetch, data, error, refresh: fetchData };
 };

packages/react/tests/useQuery.test.tsx

@@ -1,7 +1,7 @@
 import * as commonSdk from '@powersync/common';
-import { renderHook, waitFor } from '@testing-library/react';
+import { cleanup, renderHook, waitFor } from '@testing-library/react';
 import React from 'react';
-import { afterEach, describe, expect, it, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { PowerSyncContext } from '../src/hooks/PowerSyncContext';
 import { useQuery } from '../src/hooks/useQuery';
 
@@ -12,16 +12,17 @@ const mockPowerSync = {
   })),
   resolveTables: vi.fn(() => ['table1', 'table2']),
   onChangeWithCallback: vi.fn(),
-  getAll: vi.fn(() => ['list1', 'list2'])
+  getAll: vi.fn(() => Promise.resolve(['list1', 'list2']))
 };
 
 vi.mock('./PowerSyncContext', () => ({
   useContext: vi.fn(() => mockPowerSync)
 }));
 
 describe('useQuery', () => {
-  afterEach(() => {
-    vi.resetAllMocks();
+  beforeEach(() => {
+    vi.clearAllMocks();
+    cleanup(); // Cleanup the DOM after each test
   });
 
   it('should error when PowerSync is not set', async () => {
@@ -48,14 +49,14 @@ describe('useQuery', () => {
     );
 
     const { result } = renderHook(() => useQuery('SELECT * from lists', [], { runQueryOnce: true }), { wrapper });
-    const currentResult = result.current;
-    expect(currentResult.isLoading).toEqual(true);
+    expect(result.current.isLoading).toEqual(true);
 
-    waitFor(
+    await waitFor(
       async () => {
-        expect(currentResult.isLoading).toEqual(false);
+        const currentResult = result.current;
         expect(currentResult.data).toEqual(['list1', 'list2']);
         expect(currentResult.isLoading).toEqual(false);
+        expect(currentResult.isFetching).toEqual(false);
         expect(mockPowerSync.onChangeWithCallback).not.toHaveBeenCalled();
         expect(mockPowerSync.getAll).toHaveBeenCalledTimes(1);
       },
@@ -69,13 +70,14 @@ describe('useQuery', () => {
     );
 
     const { result } = renderHook(() => useQuery('SELECT * from lists', [], { runQueryOnce: true }), { wrapper });
-    const currentResult = result.current;
-    expect(currentResult.isLoading).toEqual(true);
+
+    expect(result.current.isLoading).toEqual(true);
 
     let refresh;
 
-    waitFor(
+    await waitFor(
       async () => {
+        const currentResult = result.current;
         refresh = currentResult.refresh;
         expect(currentResult.isLoading).toEqual(false);
         expect(mockPowerSync.getAll).toHaveBeenCalledTimes(1);
@@ -105,11 +107,10 @@ describe('useQuery', () => {
     );
 
     const { result } = renderHook(() => useQuery('SELECT * from lists', [], { runQueryOnce: true }), { wrapper });
-    const currentResult = result.current;
 
-    waitFor(
+    await waitFor(
       async () => {
-        expect(currentResult.error).toEqual(Error('PowerSync failed to fetch data: some error'));
+        expect(result.current.error).toEqual(Error('PowerSync failed to fetch data: some error'));
       },
       { timeout: 100 }
     );
@@ -133,11 +134,10 @@ describe('useQuery', () => {
     );
 
     const { result } = renderHook(() => useQuery('SELECT * from lists', []), { wrapper });
-    const currentResult = result.current;
 
-    waitFor(
+    await waitFor(
       async () => {
-        expect(currentResult.error).toEqual(Error('PowerSync failed to fetch data: some error'));
+        expect(result.current.error).toEqual(Error('PowerSync failed to fetch data: some error'));
       },
       { timeout: 100 }
     );
@@ -173,13 +173,13 @@ describe('useQuery', () => {
     });
   });
 
-  // The test returns unhandled errors when run with all the others.
-  // TODO: Fix the test so that there are no unhandled errors (this may be a vitest or @testing-library/react issue)
-  it.skip('should show an error if parsing the query results in an error', async () => {
+  it('should show an error if parsing the query results in an error', async () => {
     const wrapper = ({ children }) => (
       <PowerSyncContext.Provider value={mockPowerSync as any}>{children}</PowerSyncContext.Provider>
     );
-    vi.spyOn(commonSdk, 'parseQuery').mockReturnValue(Error('error') as any);
+    vi.spyOn(commonSdk, 'parseQuery').mockImplementation(() => {
+      throw new Error('error');
+    });
 
     const { result } = renderHook(
       () =>
@@ -189,10 +189,10 @@ describe('useQuery', () => {
         }),
       { wrapper }
     );
-    const currentResult = result.current;
 
-    waitFor(
+    await waitFor(
       async () => {
+        const currentResult = result.current;
         expect(currentResult.isLoading).toEqual(false);
         expect(currentResult.isFetching).toEqual(false);
         expect(currentResult.data).toEqual([]);