From ea136c9d730df24c1ea6b0fa8dae00435da5ef28 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Tue, 29 Sep 2020 13:38:10 +0100 Subject: [PATCH] (preact) - Update @urql/preact implementation to match React bindings --- .changeset/famous-penguins-tickle.md | 5 + packages/preact-urql/src/hooks/constants.ts | 8 ++ .../src/hooks/useImmediateEffect.test.ts | 50 ------- .../src/hooks/useImmediateEffect.ts | 29 ---- .../src/hooks/useImmediateState.test.ts | 54 -------- .../src/hooks/useImmediateState.ts | 45 ------- packages/preact-urql/src/hooks/useMutation.ts | 51 +++---- .../preact-urql/src/hooks/useQuery.test.tsx | 27 ++-- packages/preact-urql/src/hooks/useQuery.ts | 125 +++++++++--------- packages/preact-urql/src/hooks/useSource.ts | 88 ++++++++++++ .../src/hooks/useSubscription.test.tsx | 2 +- .../preact-urql/src/hooks/useSubscription.ts | 112 +++++++++------- .../react-urql/src/hooks/useSubscription.ts | 2 +- 13 files changed, 266 insertions(+), 332 deletions(-) create mode 100644 .changeset/famous-penguins-tickle.md create mode 100644 packages/preact-urql/src/hooks/constants.ts delete mode 100644 packages/preact-urql/src/hooks/useImmediateEffect.test.ts delete mode 100644 packages/preact-urql/src/hooks/useImmediateEffect.ts delete mode 100644 packages/preact-urql/src/hooks/useImmediateState.test.ts delete mode 100644 packages/preact-urql/src/hooks/useImmediateState.ts create mode 100644 packages/preact-urql/src/hooks/useSource.ts diff --git a/.changeset/famous-penguins-tickle.md b/.changeset/famous-penguins-tickle.md new file mode 100644 index 0000000000..283db1436f --- /dev/null +++ b/.changeset/famous-penguins-tickle.md @@ -0,0 +1,5 @@ +--- +'@urql/preact': minor +--- + +Update `@urql/preact` implementation to match `urql` React implementation. Internally these changes should align behaviour and updates slightly, but outwardly no changes should be apparent apart from how some updates are scheduled. diff --git a/packages/preact-urql/src/hooks/constants.ts b/packages/preact-urql/src/hooks/constants.ts new file mode 100644 index 0000000000..1852afcda4 --- /dev/null +++ b/packages/preact-urql/src/hooks/constants.ts @@ -0,0 +1,8 @@ +export const initialState = { + fetching: false, + stale: false, + error: undefined, + data: undefined, + extensions: undefined, + operation: undefined, +}; diff --git a/packages/preact-urql/src/hooks/useImmediateEffect.test.ts b/packages/preact-urql/src/hooks/useImmediateEffect.test.ts deleted file mode 100644 index ccf3dac637..0000000000 --- a/packages/preact-urql/src/hooks/useImmediateEffect.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { h } from 'preact'; -import { render } from '@testing-library/preact'; -import { useImmediateEffect } from './useImmediateEffect'; -import { act } from 'preact/test-utils'; - -const Component = ({ assertion, effect }) => { - useImmediateEffect(effect, [effect]); - if (assertion) assertion(); - return null; -}; - -it('calls effects immediately on mount', () => { - const effect = jest.fn(); - - act(() => { - render( - h(Component, { - assertion: () => expect(effect).toHaveBeenCalledTimes(1), - effect, - }) - ); - }); - - expect(effect).toHaveBeenCalledTimes(1); -}); - -it('behaves like useEffect otherwise', () => { - const effect = jest.fn(); - const effectRerender = jest.fn(); - let rerender; - - act(() => { - ({ rerender } = render( - h(Component, { - assertion: () => expect(effect).toHaveBeenCalledTimes(1), - effect, - }) - )); - }); - - act(() => { - rerender( - h(Component, { - assertion: () => expect(effectRerender).toHaveBeenCalledTimes(0), - effect: effectRerender, - }) - ); - }); - expect(effectRerender).toHaveBeenCalledTimes(1); -}); diff --git a/packages/preact-urql/src/hooks/useImmediateEffect.ts b/packages/preact-urql/src/hooks/useImmediateEffect.ts deleted file mode 100644 index d2a24d55a6..0000000000 --- a/packages/preact-urql/src/hooks/useImmediateEffect.ts +++ /dev/null @@ -1,29 +0,0 @@ -/* eslint-disable react-hooks/exhaustive-deps */ - -import { useRef, useEffect, EffectCallback } from 'preact/hooks'; -import { noop } from './useQuery'; - -export const useImmediateEffect = ( - effect: EffectCallback, - changes: ReadonlyArray -) => { - const teardown = useRef<() => void>(noop); - const isMounted = useRef(false); - - // On initial render we just execute the effect - if (!isMounted.current) { - // There's the slight possibility that we had an interrupt due to - // conccurrent mode after running the effect. - // This could result in memory leaks. - teardown.current(); - teardown.current = effect() || noop; - } - - useEffect(() => { - // Initially we skip executing the effect since we've already done so on - // initial render, then we execute it as usual - return isMounted.current - ? effect() - : ((isMounted.current = true), teardown.current); - }, changes); -}; diff --git a/packages/preact-urql/src/hooks/useImmediateState.test.ts b/packages/preact-urql/src/hooks/useImmediateState.test.ts deleted file mode 100644 index 3b832234c5..0000000000 --- a/packages/preact-urql/src/hooks/useImmediateState.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { h } from 'preact'; -import { render } from '@testing-library/preact'; -import { useImmediateState } from './useImmediateState'; - -const initialState = { someObject: 1234 }; -const updateState = { someObject: 5678 }; -let state; -let setState; -let returnVal; - -const Fixture = ({ update }: { update?: boolean }) => { - const [a, set] = useImmediateState(initialState); - - if (update) { - returnVal = set(updateState); - } - - state = a; - setState = set; - - return null; -}; - -beforeEach(jest.clearAllMocks); - -describe('on initial mount', () => { - it('sets initial state', () => { - render( - // @ts-ignore - h(Fixture, {}) - ); - expect(state).toEqual(initialState); - }); - - it('only mutates on setState call', () => { - render( - // @ts-ignore - h(Fixture, { update: true }) - ); - expect(state).toEqual(updateState); - expect(returnVal).toBe(undefined); - }); -}); - -describe('on later mounts', () => { - it('sets state via setState', () => { - render( - // @ts-ignore - h(Fixture, {}) - ); - setState(updateState); - expect(returnVal).toBe(undefined); - }); -}); diff --git a/packages/preact-urql/src/hooks/useImmediateState.ts b/packages/preact-urql/src/hooks/useImmediateState.ts deleted file mode 100644 index 2d242bce3b..0000000000 --- a/packages/preact-urql/src/hooks/useImmediateState.ts +++ /dev/null @@ -1,45 +0,0 @@ -/* eslint-disable react-hooks/exhaustive-deps */ - -import { - useRef, - useState, - useCallback, - useEffect, - useLayoutEffect, -} from 'preact/hooks'; - -type SetStateAction = S | ((prevState: S) => S); -type SetState = (action: SetStateAction) => void; - -const useIsomorphicLayoutEffect = - typeof window !== 'undefined' ? useLayoutEffect : useEffect; - -export const useImmediateState = (init: S): [S, SetState] => { - const isMounted = useRef(false); - const [state, setState] = useState(init); - - // This wraps setState and updates the state mutably on initial mount - const updateState: SetState = useCallback( - (action: SetStateAction): void => { - if (!isMounted.current) { - const newState = - typeof action === 'function' - ? (action as (arg: S) => S)(state) - : action; - Object.assign(state, newState); - } else { - setState(action); - } - }, - [] - ); - - useIsomorphicLayoutEffect(() => { - isMounted.current = true; - return () => { - isMounted.current = false; - }; - }, []); - - return [state, updateState]; -}; diff --git a/packages/preact-urql/src/hooks/useMutation.ts b/packages/preact-urql/src/hooks/useMutation.ts index 4ed703675e..fbf452da73 100644 --- a/packages/preact-urql/src/hooks/useMutation.ts +++ b/packages/preact-urql/src/hooks/useMutation.ts @@ -1,16 +1,17 @@ -import { useCallback } from 'preact/hooks'; import { DocumentNode } from 'graphql'; +import { useState, useCallback, useRef, useEffect } from 'preact/hooks'; import { pipe, toPromise } from 'wonka'; + import { - Operation, + OperationResult, OperationContext, CombinedError, createRequest, - OperationResult, + Operation, } from '@urql/core'; + import { useClient } from '../context'; -import { useImmediateState } from './useImmediateState'; -import { initialState } from './useQuery'; +import { initialState } from './constants'; export interface UseMutationState { fetching: boolean; @@ -29,21 +30,17 @@ export type UseMutationResponse = [ ) => Promise> ]; -export const useMutation = ( +export function useMutation( query: DocumentNode | string -): UseMutationResponse => { +): UseMutationResponse { + const isMounted = useRef(true); const client = useClient(); - const [state, setState] = useImmediateState>({ - ...initialState, - }); + const [state, setState] = useState>(initialState); const executeMutation = useCallback( (variables?: V, context?: Partial) => { - setState({ - ...initialState, - fetching: true, - }); + setState({ ...initialState, fetching: true }); return pipe( client.executeMutation( @@ -52,14 +49,16 @@ export const useMutation = ( ), toPromise ).then(result => { - setState({ - fetching: false, - stale: !!result.stale, - data: result.data, - error: result.error, - extensions: result.extensions, - operation: result.operation, - }); + if (isMounted.current) { + setState({ + fetching: false, + stale: !!result.stale, + data: result.data, + error: result.error, + extensions: result.extensions, + operation: result.operation, + }); + } return result; }); }, @@ -67,5 +66,11 @@ export const useMutation = ( [client, query, setState] ); + useEffect(() => { + return () => { + isMounted.current = false; + }; + }, []); + return [state, executeMutation]; -}; +} diff --git a/packages/preact-urql/src/hooks/useQuery.test.tsx b/packages/preact-urql/src/hooks/useQuery.test.tsx index 6a9c87980b..4ae0b1517a 100644 --- a/packages/preact-urql/src/hooks/useQuery.test.tsx +++ b/packages/preact-urql/src/hooks/useQuery.test.tsx @@ -262,20 +262,22 @@ describe('useQuery', () => { unmount(); }); - expect(start).toBeCalledTimes(1); - expect(unsubscribe).toBeCalledTimes(1); + expect(start).toBeCalledTimes(2); + expect(unsubscribe).toBeCalledTimes(2); }); }); describe('active teardown', () => { it('sets fetching to false when the source ends', () => { client.executeQuery.mockReturnValueOnce(empty); - render( - h(Provider, { - value: client as any, - children: [h(QueryUser, { ...props })], - }) - ); + act(() => { + render( + h(Provider, { + value: client as any, + children: [h(QueryUser, { ...props })], + }) + ); + }); expect(client.executeQuery).toHaveBeenCalled(); expect(state).toMatchObject({ fetching: false }); }); @@ -313,15 +315,6 @@ describe('useQuery', () => { }) ); - /** - * Call update twice for the change to be detected. - */ - rerender( - h(Provider, { - value: client as any, - children: [h(QueryUser, { ...props, pause: true })], - }) - ); rerender( h(Provider, { value: client as any, diff --git a/packages/preact-urql/src/hooks/useQuery.ts b/packages/preact-urql/src/hooks/useQuery.ts index 4d1c1a343a..99b4476ca2 100644 --- a/packages/preact-urql/src/hooks/useQuery.ts +++ b/packages/preact-urql/src/hooks/useQuery.ts @@ -1,26 +1,17 @@ import { DocumentNode } from 'graphql'; -import { pipe, subscribe, onEnd } from 'wonka'; -import { useRef, useCallback } from 'preact/hooks'; +import { useCallback, useMemo } from 'preact/hooks'; +import { pipe, concat, fromValue, switchMap, map, scan } from 'wonka'; import { + CombinedError, OperationContext, RequestPolicy, - CombinedError, Operation, } from '@urql/core'; import { useClient } from '../context'; +import { useSource, useBehaviourSubject } from './useSource'; import { useRequest } from './useRequest'; -import { useImmediateState } from './useImmediateState'; -import { useImmediateEffect } from './useImmediateEffect'; - -export const initialState: UseQueryState = { - fetching: false, - stale: false, - data: undefined, - error: undefined, - operation: undefined, - extensions: undefined, -}; +import { initialState } from './constants'; export interface UseQueryArgs { query: string | DocumentNode; @@ -45,69 +36,75 @@ export type UseQueryResponse = [ (opts?: Partial) => void ]; -// eslint-disable-next-line -export const noop = () => {}; - -export const useQuery = ( +export function useQuery( args: UseQueryArgs -): UseQueryResponse => { - const unsubscribe = useRef<(_1?: any) => void>(noop); +): UseQueryResponse { const client = useClient(); - const [state, setState] = useImmediateState>({ - ...initialState, - }); // This creates a request which will keep a stable reference // if request.key doesn't change const request = useRequest(args.query, args.variables); - const executeQuery = useCallback( + // Create a new query-source from client.executeQuery + const makeQuery$ = useCallback( (opts?: Partial) => { - unsubscribe.current(); + return client.executeQuery(request, { + requestPolicy: args.requestPolicy, + pollInterval: args.pollInterval, + ...args.context, + ...opts, + }); + }, + [client, request, args.requestPolicy, args.pollInterval, args.context] + ); - setState(s => ({ ...s, fetching: true })); + const [query$$, update] = useBehaviourSubject( + useMemo(() => (args.pause ? null : makeQuery$()), [args.pause, makeQuery$]) + ); + + const state = useSource( + useMemo(() => { + return pipe( + query$$, + switchMap(query$ => { + if (!query$) return fromValue({ fetching: false, stale: false }); - const result = pipe( - client.executeQuery(request, { - requestPolicy: args.requestPolicy, - pollInterval: args.pollInterval, - ...args.context, - ...opts, + return concat([ + // Initially set fetching to true + fromValue({ fetching: true, stale: false }), + pipe( + query$, + map(({ stale, data, error, extensions, operation }) => ({ + fetching: false, + stale: !!stale, + data, + error, + operation, + extensions, + })) + ), + // When the source proactively closes, fetching is set to false + fromValue({ fetching: false, stale: false }), + ]); }), - onEnd(() => setState(s => ({ ...s, fetching: false }))), - subscribe(result => { - setState({ - fetching: false, - data: result.data, - error: result.error, - extensions: result.extensions, - stale: !!result.stale, - operation: result.operation, - }); - }) + // The individual partial results are merged into each previous result + scan( + (result, partial) => ({ + ...result, + ...partial, + }), + initialState + ) ); - unsubscribe.current = result.unsubscribe; - }, - [ - setState, - client, - request, - args.requestPolicy, - args.pollInterval, - args.context, - ] + }, [query$$]), + initialState ); - useImmediateEffect(() => { - if (args.pause) { - unsubscribe.current(); - setState(s => ({ ...s, fetching: false })); - return noop; - } - - executeQuery(); - return unsubscribe.current; // eslint-disable-line - }, [executeQuery, args.pause, setState]); + // This is the imperative execute function passed to the user + const executeQuery = useCallback( + (opts?: Partial) => update(makeQuery$(opts)), + [update, makeQuery$] + ); return [state, executeQuery]; -}; +} diff --git a/packages/preact-urql/src/hooks/useSource.ts b/packages/preact-urql/src/hooks/useSource.ts new file mode 100644 index 0000000000..7d93a9c388 --- /dev/null +++ b/packages/preact-urql/src/hooks/useSource.ts @@ -0,0 +1,88 @@ +/* eslint-disable react-hooks/exhaustive-deps */ + +import { useMemo, useEffect, useState } from 'preact/hooks'; + +import { + Source, + fromValue, + makeSubject, + pipe, + map, + concat, + onPush, + publish, + subscribe, +} from 'wonka'; + +import { useClient } from '../context'; + +let currentInit = false; + +export function useSource(source: Source, init: T): T { + const [state, setState] = useState(() => { + currentInit = true; + let initialValue = init; + + pipe( + source, + onPush(value => { + initialValue = value; + }), + publish + ).unsubscribe(); + + currentInit = false; + return initialValue; + }); + + useEffect(() => { + return pipe( + source, + subscribe(value => { + if (!currentInit) { + setState(value); + } + }) + ).unsubscribe as () => void; + }, [source]); + + return state; +} + +export function useBehaviourSubject(value: T) { + const client = useClient(); + + const state = useMemo((): [Source, (value: T) => void] => { + let prevValue = value; + + const subject = makeSubject(); + const prevValue$ = pipe( + fromValue(value), + map(() => prevValue) + ); + + // This turns the subject into a behaviour subject that returns + // the last known value (or the initial value) synchronously + const source = concat([prevValue$, subject.source]); + + const next = (value: T) => { + // We can use the latest known value to also deduplicate next calls. + if (value !== prevValue) { + subject.next((prevValue = value)); + } + }; + + return [source, next]; + }, []); + + // NOTE: This is a special case for client-side suspense. + // We can't trigger suspense inside an effect but only in the render function. + // So we "deopt" to not using an effect if the client is in suspense-mode. + useEffect(() => { + if (!client.suspense) state[1](value); + }, [state, value]); + + if (client.suspense) state[1](value); + + return state; +} diff --git a/packages/preact-urql/src/hooks/useSubscription.test.tsx b/packages/preact-urql/src/hooks/useSubscription.test.tsx index c087b1863c..5e87b9457b 100644 --- a/packages/preact-urql/src/hooks/useSubscription.test.tsx +++ b/packages/preact-urql/src/hooks/useSubscription.test.tsx @@ -112,7 +112,7 @@ describe('useSubscription', () => { children: [h(SubscriptionUser, { ...props })], }) ); - expect(handler).toBeCalledTimes(1); + expect(handler).toBeCalledTimes(2); expect(handler).toBeCalledWith(undefined, 1234); }); diff --git a/packages/preact-urql/src/hooks/useSubscription.ts b/packages/preact-urql/src/hooks/useSubscription.ts index 4a6d1fef12..e241a2638d 100644 --- a/packages/preact-urql/src/hooks/useSubscription.ts +++ b/packages/preact-urql/src/hooks/useSubscription.ts @@ -1,12 +1,12 @@ import { DocumentNode } from 'graphql'; -import { useCallback, useRef } from 'preact/hooks'; -import { pipe, onEnd, subscribe } from 'wonka'; +import { useCallback, useRef, useMemo } from 'preact/hooks'; +import { pipe, concat, fromValue, switchMap, map, scan } from 'wonka'; import { CombinedError, OperationContext, Operation } from '@urql/core'; + import { useClient } from '../context'; +import { useSource, useBehaviourSubject } from './useSource'; import { useRequest } from './useRequest'; -import { noop, initialState } from './useQuery'; -import { useImmediateEffect } from './useImmediateEffect'; -import { useImmediateState } from './useImmediateState'; +import { initialState } from './constants'; export interface UseSubscriptionArgs { query: DocumentNode | string; @@ -31,67 +31,83 @@ export type UseSubscriptionResponse = [ (opts?: Partial) => void ]; -export const useSubscription = ( +export function useSubscription( args: UseSubscriptionArgs, handler?: SubscriptionHandler -): UseSubscriptionResponse => { - const unsubscribe = useRef<(_1?: any) => void>(noop); - const handlerRef = useRef(handler); +): UseSubscriptionResponse { const client = useClient(); - const [state, setState] = useImmediateState>({ - ...initialState, - }); - // Update handler on constant ref, since handler changes shouldn't // trigger a new subscription run + const handlerRef = useRef(handler); handlerRef.current = handler!; // This creates a request which will keep a stable reference // if request.key doesn't change const request = useRequest(args.query, args.variables); - const executeSubscription = useCallback( + // Create a new subscription-source from client.executeSubscription + const makeSubscription$ = useCallback( (opts?: Partial) => { - unsubscribe.current(); + return client.executeSubscription(request, { ...args.context, ...opts }); + }, + [client, request, args.context] + ); - setState(s => ({ ...s, fetching: true })); + const [subscription$$, update] = useBehaviourSubject( + useMemo(() => (args.pause ? null : makeSubscription$()), [ + args.pause, + makeSubscription$, + ]) + ); + + const state = useSource( + useMemo(() => { + return pipe( + subscription$$, + switchMap(subscription$ => { + if (!subscription$) return fromValue({ fetching: false }); - const result = pipe( - client.executeSubscription(request, { - ...args.context, - ...opts, + return concat([ + // Initially set fetching to true + fromValue({ fetching: true, stale: false }), + pipe( + subscription$, + map(({ stale, data, error, extensions, operation }) => ({ + fetching: true, + stale: !!stale, + data, + error, + extensions, + operation, + })) + ), + // When the source proactively closes, fetching is set to false + fromValue({ fetching: false, stale: false }), + ]); }), - onEnd(() => setState(s => ({ ...s, fetching: false }))), - subscribe(result => { - setState(s => ({ - fetching: true, - data: - typeof handlerRef.current === 'function' - ? handlerRef.current(s.data, result.data) - : result.data, - error: result.error, - extensions: result.extensions, - stale: !!result.stale, - operation: result.operation, - })); - }) + // The individual partial results are merged into each previous result + scan((result, partial: any) => { + const { current: handler } = handlerRef; + // If a handler has been passed, it's used to merge new data in + const data = + partial.data !== undefined + ? typeof handler === 'function' + ? handler(result.data, partial.data) + : partial.data + : result.data; + return { ...result, ...partial, data }; + }, initialState) ); - unsubscribe.current = result.unsubscribe; - }, - [client, request, setState, args.context] + }, [subscription$$]), + initialState ); - useImmediateEffect(() => { - if (args.pause) { - unsubscribe.current(); - setState(s => ({ ...s, fetching: false })); - return noop; - } - - executeSubscription(); - return unsubscribe.current; // eslint-disable-line - }, [executeSubscription, args.pause, setState]); + // This is the imperative execute function passed to the user + const executeSubscription = useCallback( + (opts?: Partial) => update(makeSubscription$(opts)), + [update, makeSubscription$] + ); return [state, executeSubscription]; -}; +} diff --git a/packages/react-urql/src/hooks/useSubscription.ts b/packages/react-urql/src/hooks/useSubscription.ts index 20726b4b95..d50e7f820d 100644 --- a/packages/react-urql/src/hooks/useSubscription.ts +++ b/packages/react-urql/src/hooks/useSubscription.ts @@ -40,7 +40,7 @@ export function useSubscription( // Update handler on constant ref, since handler changes shouldn't // trigger a new subscription run const handlerRef = useRef(handler); - handlerRef.current = handler; + handlerRef.current = handler!; // This creates a request which will keep a stable reference // if request.key doesn't change