diff --git a/.changeset/bright-rivers-listen.md b/.changeset/bright-rivers-listen.md new file mode 100644 index 0000000000..85d112f610 --- /dev/null +++ b/.changeset/bright-rivers-listen.md @@ -0,0 +1,5 @@ +--- +'urql': patch +--- + +Fix `useSubscription`'s `fetching` state to remain `true` while its source is active and become `false` when the source completes. Local React effect cleanup and stale sources no longer report a completed active subscription. diff --git a/packages/react-urql/src/hooks/useSubscription.test.tsx b/packages/react-urql/src/hooks/useSubscription.test.tsx index e7f75124f0..ae298866da 100644 --- a/packages/react-urql/src/hooks/useSubscription.test.tsx +++ b/packages/react-urql/src/hooks/useSubscription.test.tsx @@ -17,7 +17,9 @@ vi.mock('../context', async () => { import React from 'react'; import renderer, { act } from 'react-test-renderer'; -import { OperationContext } from '@urql/core'; +import { renderHook } from '@testing-library/react'; +import { CombinedError, OperationContext } from '@urql/core'; +import { fromValue, makeSubject, merge, never } from 'wonka'; import { useSubscription, UseSubscriptionState } from './useSubscription'; import { useClient } from '../context'; @@ -45,7 +47,10 @@ const SubscriptionUser = ({ }; beforeEach(() => { - client.executeSubscription.mockClear(); + client.executeSubscription.mockReset(); + client.executeSubscription.mockImplementation(() => + merge([fromValue({ data: 1234, error: 5678 }), never]) + ); state = undefined; }); @@ -95,6 +100,53 @@ describe('execute subscription', () => { }); }); +describe('fetching', () => { + it('remains active when subscription results arrive', () => { + const subject = makeSubject<{ data: number }>(); + client.executeSubscription.mockReturnValue(subject.source); + const { result } = renderHook(() => useSubscription({ query })); + + act(() => subject.next({ data: 1234 })); + + expect(result.current[0].fetching).toBe(true); + }); + + it('becomes inactive when the subscription completes', () => { + const subject = makeSubject<{ data: number }>(); + client.executeSubscription.mockReturnValue(subject.source); + const { result } = renderHook(() => useSubscription({ query })); + + expect(result.current[0].fetching).toBe(true); + act(() => subject.complete()); + expect(result.current[0].fetching).toBe(false); + }); + + it('preserves subscription errors on completion', () => { + const subject = makeSubject<{ + data: undefined; + error: CombinedError; + }>(); + const error = new CombinedError({ networkError: new Error('test') }); + client.executeSubscription.mockReturnValue(subject.source); + const { result } = renderHook(() => useSubscription({ query })); + + act(() => subject.next({ data: undefined, error })); + act(() => subject.complete()); + + expect(result.current[0]).toMatchObject({ fetching: false, error }); + }); + + it('remains active after local Strict Mode cleanup', () => { + const subject = makeSubject<{ data: number }>(); + client.executeSubscription.mockReturnValue(subject.source); + const { result } = renderHook(() => useSubscription({ query }), { + wrapper: React.StrictMode, + }); + + expect(result.current[0].fetching).toBe(true); + }); +}); + describe('pause', () => { const props = { q: query }; diff --git a/packages/react-urql/src/hooks/useSubscription.ts b/packages/react-urql/src/hooks/useSubscription.ts index 6e254e887f..8894e0f195 100644 --- a/packages/react-urql/src/hooks/useSubscription.ts +++ b/packages/react-urql/src/hooks/useSubscription.ts @@ -260,11 +260,19 @@ export function useSubscription< } React.useEffect(() => { + const source = state[0]; const updateResult = ( - result: Partial> + result: Partial>, + fetching?: boolean ) => { deferDispatch(setState, state => { - const nextResult = computeNextState(state[1], result); + if (state[0] !== source) return state; + + let nextResult = computeNextState(state[1], result); + if (fetching !== undefined && nextResult.fetching !== fetching) { + nextResult = { ...nextResult, fetching }; + } + if (state[1] === nextResult) return state; if ( handlerRef.current && @@ -281,14 +289,27 @@ export function useSubscription< }); }; - if (state[0]) { - return pipe( - state[0], + const updateFetching = (fetching: boolean) => { + deferDispatch(setState, state => { + if (state[0] !== source || state[1].fetching === fetching) return state; + return [state[0], { ...state[1], fetching }, state[2]] as const; + }); + }; + + if (source) { + let isSubscribed = true; + const subscription = pipe( + source, onEnd(() => { - updateResult({ fetching: !!source }); + if (isSubscribed) updateFetching(false); }), - subscribe(updateResult) - ).unsubscribe; + subscribe(result => updateResult(result, true)) + ); + + return () => { + isSubscribed = false; + subscription.unsubscribe(); + }; } else { updateResult({ fetching: false }); }