From 699e8029ade8c4ef152d80a061dd1f80cc53f928 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 4 Nov 2020 23:09:05 +0000 Subject: [PATCH 01/11] Add context.suspense flag consistently --- packages/core/src/client.test.ts | 1 + packages/core/src/client.ts | 34 +++++++++++-------- .../react-urql/src/test-utils/ssr.test.tsx | 1 + 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/packages/core/src/client.test.ts b/packages/core/src/client.test.ts index 71128ef25c..125c3b944c 100755 --- a/packages/core/src/client.test.ts +++ b/packages/core/src/client.test.ts @@ -122,6 +122,7 @@ describe('promisified methods', () => { requestPolicy: 'cache-and-network', fetchOptions: undefined, fetch: undefined, + suspense: false, preferGetMethod: false, }); expect(mutationResult).toHaveProperty('then'); diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 42fe9df2d9..94933765ff 100755 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -173,14 +173,19 @@ export class Client { createOperationContext = ( opts?: Partial - ): OperationContext => ({ - url: this.url, - fetchOptions: this.fetchOptions, - fetch: this.fetch, - preferGetMethod: this.preferGetMethod, - ...opts, - requestPolicy: (opts || {}).requestPolicy || this.requestPolicy, - }); + ): OperationContext => { + if (!opts) opts = {}; + + return { + url: this.url, + fetchOptions: this.fetchOptions, + fetch: this.fetch, + preferGetMethod: this.preferGetMethod, + ...opts, + suspense: opts.suspense || (opts.suspense !== false && this.suspense), + requestPolicy: opts.requestPolicy || this.requestPolicy, + }; + }; createRequestOperation = ( kind: OperationType, @@ -222,10 +227,9 @@ export class Client { executeRequestOperation( operation: Operation ): Source> { - const { key, kind } = operation; let operationResults$ = pipe( this.results$, - filter((res: OperationResult) => res.operation.key === key) + filter((res: OperationResult) => res.operation.key === operation.key) ) as Source>; if (this.maskTypename) { @@ -238,7 +242,7 @@ export class Client { ); } - if (kind === 'mutation') { + if (operation.kind === 'mutation') { // A mutation is always limited to just a single result and is never shared return pipe( operationResults$, @@ -249,7 +253,9 @@ export class Client { const teardown$ = pipe( this.operations$, - filter((op: Operation) => op.kind === 'teardown' && op.key === key) + filter( + (op: Operation) => op.kind === 'teardown' && op.key === operation.key + ) ); const result$ = pipe( @@ -263,9 +269,7 @@ export class Client { }) ); - return operation.context.suspense !== false && - this.suspense && - kind === 'query' + return operation.kind === 'query' && operation.context.suspense ? toSuspenseSource(result$ as Source) : (result$ as Source); } diff --git a/packages/react-urql/src/test-utils/ssr.test.tsx b/packages/react-urql/src/test-utils/ssr.test.tsx index 872ef64a1f..8bdca172df 100644 --- a/packages/react-urql/src/test-utils/ssr.test.tsx +++ b/packages/react-urql/src/test-utils/ssr.test.tsx @@ -25,6 +25,7 @@ const context: OperationContext = { }, requestPolicy: 'cache-first', url: 'http://localhost:3000/graphql', + suspense: true, }; export const queryGql: GraphQLRequest = { From 074d58e93b32fcc122d54d30c3c54702eb6f8e3c Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 4 Nov 2020 23:20:39 +0000 Subject: [PATCH 02/11] Move suspense-related effects to useQuery/useSubscription --- packages/react-urql/src/hooks/useQuery.ts | 14 ++++++++++++-- packages/react-urql/src/hooks/useSource.ts | 9 --------- packages/react-urql/src/hooks/useSubscription.ts | 15 ++++++++++----- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/packages/react-urql/src/hooks/useQuery.ts b/packages/react-urql/src/hooks/useQuery.ts index 1d1aedba52..6afb58f324 100644 --- a/packages/react-urql/src/hooks/useQuery.ts +++ b/packages/react-urql/src/hooks/useQuery.ts @@ -1,5 +1,5 @@ import { DocumentNode } from 'graphql'; -import { useCallback, useMemo } from 'react'; +import { useEffect, useCallback, useMemo } from 'react'; import { pipe, concat, fromValue, switchMap, map, scan } from 'wonka'; import { @@ -60,8 +60,12 @@ export function useQuery( [client, request, args.requestPolicy, args.pollInterval, args.context] ); + const query$ = useMemo(() => { + return args.pause ? null : makeQuery$(); + }, [args.pause, makeQuery$]); + const [state, update] = useSource( - useMemo(() => (args.pause ? null : makeQuery$()), [args.pause, makeQuery$]), + query$, useCallback( (query$$, prevState: UseQueryState | undefined) => { return pipe( @@ -107,5 +111,11 @@ export function useQuery( [update, makeQuery$] ); + useEffect(() => { + if (!client.suspense) update(query$); + }, [update, query$]); + + if (client.suspense) update(query$); + return [state, executeQuery]; } diff --git a/packages/react-urql/src/hooks/useSource.ts b/packages/react-urql/src/hooks/useSource.ts index 676934599f..eac962157a 100644 --- a/packages/react-urql/src/hooks/useSource.ts +++ b/packages/react-urql/src/hooks/useSource.ts @@ -4,8 +4,6 @@ import { useMemo, useEffect, useState, useRef } from 'react'; import { Source, fromValue, makeSubject, pipe, concat, subscribe } from 'wonka'; -import { useClient } from '../context'; - type Updater = (input: T) => void; let currentInit = false; @@ -21,7 +19,6 @@ export function useSource( input: T, transform: (input$: Source, initial: R | undefined) => Source ): [R, Updater] { - const client = useClient(); const prev = useRef(); const [input$, updateInput] = useMemo((): [Source, (value: T) => void] => { @@ -65,11 +62,5 @@ export function useSource( ).unsubscribe; }, [input$]); - useEffect(() => { - if (!client.suspense) updateInput(input); - }, [updateInput, input]); - - if (client.suspense) updateInput(input); - return [state, updateInput]; } diff --git a/packages/react-urql/src/hooks/useSubscription.ts b/packages/react-urql/src/hooks/useSubscription.ts index bd93b040b4..db6fdafb28 100644 --- a/packages/react-urql/src/hooks/useSubscription.ts +++ b/packages/react-urql/src/hooks/useSubscription.ts @@ -1,5 +1,5 @@ import { DocumentNode } from 'graphql'; -import { useCallback, useRef, useMemo } from 'react'; +import { useEffect, useCallback, useRef, useMemo } from 'react'; import { pipe, concat, fromValue, switchMap, map, scan } from 'wonka'; import { @@ -63,11 +63,12 @@ export function useSubscription( [client, request, args.context] ); + const subscription$ = useMemo(() => { + return args.pause ? null : makeSubscription$(); + }, [args.pause, makeSubscription$]); + const [state, update] = useSource( - useMemo(() => (args.pause ? null : makeSubscription$()), [ - args.pause, - makeSubscription$, - ]), + subscription$, useCallback( ( subscription$$, @@ -123,5 +124,9 @@ export function useSubscription( [update, makeSubscription$] ); + useEffect(() => { + update(subscription$); + }, [update, subscription$]); + return [state, executeSubscription]; } From 84b32acdae50f07a8b01180e58c7640fc0bd189f Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 4 Nov 2020 23:35:49 +0000 Subject: [PATCH 03/11] Switch useQuery to effect if args.context.suspense is false --- packages/react-urql/src/hooks/useQuery.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/react-urql/src/hooks/useQuery.ts b/packages/react-urql/src/hooks/useQuery.ts index 6afb58f324..7e39163a32 100644 --- a/packages/react-urql/src/hooks/useQuery.ts +++ b/packages/react-urql/src/hooks/useQuery.ts @@ -42,7 +42,6 @@ export function useQuery( args: UseQueryArgs ): UseQueryResponse { const client = useClient(); - // This creates a request which will keep a stable reference // if request.key doesn't change const request = useRequest(args.query, args.variables); @@ -112,10 +111,14 @@ export function useQuery( ); useEffect(() => { - if (!client.suspense) update(query$); - }, [update, query$]); + if (!client.suspense || (args.context && args.context.suspense === false)) { + update(query$); + } + }, [update, query$, args.context]); - if (client.suspense) update(query$); + if (client.suspense && (!args.context || args.context.suspense !== false)) { + update(query$); + } return [state, executeQuery]; } From e10953e28df129f6333a1c8e793087a6ddb42063 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 4 Nov 2020 23:39:53 +0000 Subject: [PATCH 04/11] Replace unnecessary prevInput variable --- packages/react-urql/src/hooks/useSource.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-urql/src/hooks/useSource.ts b/packages/react-urql/src/hooks/useSource.ts index eac962157a..00bbf7c2e0 100644 --- a/packages/react-urql/src/hooks/useSource.ts +++ b/packages/react-urql/src/hooks/useSource.ts @@ -25,9 +25,8 @@ export function useSource( const subject = makeSubject(); const source = concat([fromValue(input), subject.source]); - let prevInput = input; - const updateInput = (input: T) => { - if (input !== prevInput) subject.next((prevInput = input)); + const updateInput = (nextInput: T) => { + if (nextInput !== input) subject.next((input = nextInput)); }; return [source, updateInput]; From f333604871f7ad6fbdeca0a59fa649d179820463 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 4 Nov 2020 23:46:23 +0000 Subject: [PATCH 05/11] Golf useSource implementation by removing ref --- packages/react-urql/src/hooks/useQuery.ts | 67 +++++++++---------- packages/react-urql/src/hooks/useSource.ts | 23 +++---- .../react-urql/src/hooks/useSubscription.ts | 5 +- 3 files changed, 42 insertions(+), 53 deletions(-) diff --git a/packages/react-urql/src/hooks/useQuery.ts b/packages/react-urql/src/hooks/useQuery.ts index 7e39163a32..6e4bf10bb1 100644 --- a/packages/react-urql/src/hooks/useQuery.ts +++ b/packages/react-urql/src/hooks/useQuery.ts @@ -65,43 +65,40 @@ export function useQuery( const [state, update] = useSource( query$, - useCallback( - (query$$, prevState: UseQueryState | undefined) => { - return pipe( - query$$, - switchMap(query$ => { - if (!query$) return fromValue({ fetching: false, stale: false }); + useCallback((query$$, prevState?: UseQueryState) => { + return pipe( + query$$, + switchMap(query$ => { + if (!query$) return fromValue({ fetching: false, stale: false }); - 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 }), - ]); + 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 }), + ]); + }), + // The individual partial results are merged into each previous result + scan( + (result: UseQueryState, partial) => ({ + ...result, + ...partial, }), - // The individual partial results are merged into each previous result - scan( - (result: UseQueryState, partial) => ({ - ...result, - ...partial, - }), - prevState || initialState - ) - ); - }, - [] - ) + prevState || initialState + ) + ); + }, []) ); // This is the imperative execute function passed to the user diff --git a/packages/react-urql/src/hooks/useSource.ts b/packages/react-urql/src/hooks/useSource.ts index 00bbf7c2e0..2ec592b2cc 100644 --- a/packages/react-urql/src/hooks/useSource.ts +++ b/packages/react-urql/src/hooks/useSource.ts @@ -1,6 +1,6 @@ /* eslint-disable react-hooks/exhaustive-deps */ -import { useMemo, useEffect, useState, useRef } from 'react'; +import { useMemo, useEffect, useState } from 'react'; import { Source, fromValue, makeSubject, pipe, concat, subscribe } from 'wonka'; @@ -17,10 +17,8 @@ const isShallowDifferent = (a: any, b: any) => { export function useSource( input: T, - transform: (input$: Source, initial: R | undefined) => Source + transform: (input$: Source, initial?: R) => Source ): [R, Updater] { - const prev = useRef(); - const [input$, updateInput] = useMemo((): [Source, (value: T) => void] => { const subject = makeSubject(); const source = concat([fromValue(input), subject.source]); @@ -34,32 +32,29 @@ export function useSource( const [state, setState] = useState(() => { currentInit = true; - + let state: R; pipe( - transform(fromValue(input), prev.current), + transform(fromValue(input)), subscribe(value => { - prev.current = value; + state = value; }) ).unsubscribe(); - currentInit = false; - return prev.current!; + return state!; }); useEffect(() => { return pipe( - transform(input$, prev.current), + transform(input$, state), subscribe(value => { if (!currentInit) { setState(prevValue => { - return (prev.current = isShallowDifferent(prevValue, value) - ? value - : prevValue); + return isShallowDifferent(prevValue, value) ? value : prevValue; }); } }) ).unsubscribe; - }, [input$]); + }, [input$ /* `state` is only an initialiser */]); return [state, updateInput]; } diff --git a/packages/react-urql/src/hooks/useSubscription.ts b/packages/react-urql/src/hooks/useSubscription.ts index db6fdafb28..3f1e2676fe 100644 --- a/packages/react-urql/src/hooks/useSubscription.ts +++ b/packages/react-urql/src/hooks/useSubscription.ts @@ -70,10 +70,7 @@ export function useSubscription( const [state, update] = useSource( subscription$, useCallback( - ( - subscription$$, - prevState: UseSubscriptionState | undefined - ) => { + (subscription$$, prevState?: UseSubscriptionState) => { return pipe( subscription$$, switchMap(subscription$ => { From b23cdea41bb0a62e8523aabb37c6726053e5ab04 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Fri, 6 Nov 2020 11:00:08 +0000 Subject: [PATCH 06/11] Move suspense logic and micro-cache to useQuery implementation --- packages/core/src/client.ts | 7 +- packages/core/src/utils/index.ts | 1 - .../core/src/utils/toSuspenseSource.test.ts | 111 ------------------ packages/core/src/utils/toSuspenseSource.ts | 20 ---- packages/react-urql/src/hooks/useQuery.ts | 82 +++++++++++-- .../react-urql/src/test-utils/ssr.test.tsx | 17 --- 6 files changed, 75 insertions(+), 163 deletions(-) delete mode 100644 packages/core/src/utils/toSuspenseSource.test.ts delete mode 100644 packages/core/src/utils/toSuspenseSource.ts diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 94933765ff..353afba42b 100755 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -41,7 +41,6 @@ import { import { createRequest, - toSuspenseSource, withPromise, maskTypename, noop, @@ -258,7 +257,7 @@ export class Client { ) ); - const result$ = pipe( + return pipe( operationResults$, takeUntil(teardown$), onStart(() => { @@ -268,10 +267,6 @@ export class Client { this.onOperationEnd(operation); }) ); - - return operation.kind === 'query' && operation.context.suspense - ? toSuspenseSource(result$ as Source) - : (result$ as Source); } query( diff --git a/packages/core/src/utils/index.ts b/packages/core/src/utils/index.ts index 3ab0237f5c..ee2437c463 100644 --- a/packages/core/src/utils/index.ts +++ b/packages/core/src/utils/index.ts @@ -2,7 +2,6 @@ export * from './error'; export * from './request'; export * from './result'; export * from './typenames'; -export * from './toSuspenseSource'; export * from './stringifyVariables'; export * from './maskTypename'; export * from './withPromise'; diff --git a/packages/core/src/utils/toSuspenseSource.test.ts b/packages/core/src/utils/toSuspenseSource.test.ts deleted file mode 100644 index b1c52ea93d..0000000000 --- a/packages/core/src/utils/toSuspenseSource.test.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { - pipe, - onStart, - onPush, - onEnd, - fromValue, - fromArray, - makeSubject, - never, - publish, - subscribe, -} from 'wonka'; - -import { toSuspenseSource } from './toSuspenseSource'; - -it('does nothing when not subscribed to', () => { - const start = jest.fn(); - - pipe(fromValue('test'), onStart(start), toSuspenseSource); - - expect(start).not.toHaveBeenCalled(); -}); - -it('resolves synchronously when the source resolves synchronously', () => { - const start = jest.fn(); - const push = jest.fn(); - let result; - - pipe( - fromValue('test'), - onStart(start), - onPush(push), - toSuspenseSource, - subscribe(value => { - result = value; - }) - ); - - expect(result).toBe('test'); - expect(start).toHaveBeenCalledTimes(1); - expect(push).toHaveBeenCalledTimes(1); -}); - -it('throws a promise when the source is not resolving immediately', () => { - expect(() => { - pipe(never, toSuspenseSource as any, publish); - }).toThrow(expect.any(Promise)); -}); - -it('throws a promise that resolves when the source emits a value', () => { - const { source, next } = makeSubject(); - const end = jest.fn(); - - let promise; - let result; - - try { - pipe( - source, - toSuspenseSource, - onEnd(end), - subscribe(value => { - expect(value).toBe('test'); - result = value; - }) - ); - } catch (error) { - promise = error; - } - - // Expect it to have thrown - expect(promise).toBeInstanceOf(Promise); - - next('test'); - - // The result came in asynchronously and the original source has ended - expect(result).toBe(undefined); - - return promise.then(resolved => { - expect(resolved).toBe('test'); - expect(end).toHaveBeenCalled(); - }); -}); - -it('behaves like a normal source when the first result was synchronous', async () => { - const push = jest.fn(); - await new Promise(resolve => { - pipe(fromArray([1, 2]), toSuspenseSource, onEnd(resolve), subscribe(push)); - }); - - expect(push).toHaveBeenCalledTimes(2); -}); - -it('still supports cancellation', async () => { - let unsubscribe; - const end = jest.fn(); - - try { - ({ unsubscribe } = pipe( - fromArray([1, 2]), - toSuspenseSource, - onEnd(end), - publish - )); - } catch (promise) { - expect(promise).toBe(expect.any(Promise)); - } - - unsubscribe(); - expect(end).toHaveBeenCalledTimes(1); -}); diff --git a/packages/core/src/utils/toSuspenseSource.ts b/packages/core/src/utils/toSuspenseSource.ts deleted file mode 100644 index b7c6265acb..0000000000 --- a/packages/core/src/utils/toSuspenseSource.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { pipe, share, onPush, toPromise, takeWhile, take, Source } from 'wonka'; - -/** This converts a Source to a suspense Source; It will forward the first result synchronously or throw a promise that resolves when the result becomes available */ -export const toSuspenseSource = (source: Source): Source => sink => { - const shared = share(source); - let hasResult = false; - let hasSuspended = false; - - pipe( - shared, - takeWhile(() => !hasSuspended), - onPush(() => (hasResult = true)) - )(sink); - - if (!hasResult) { - hasSuspended = true; - sink(0); /* End */ - throw pipe(shared, take(1), toPromise); - } -}; diff --git a/packages/react-urql/src/hooks/useQuery.ts b/packages/react-urql/src/hooks/useQuery.ts index 6e4bf10bb1..78aab534db 100644 --- a/packages/react-urql/src/hooks/useQuery.ts +++ b/packages/react-urql/src/hooks/useQuery.ts @@ -1,12 +1,24 @@ import { DocumentNode } from 'graphql'; import { useEffect, useCallback, useMemo } from 'react'; -import { pipe, concat, fromValue, switchMap, map, scan } from 'wonka'; + +import { + Source, + pipe, + share, + takeWhile, + concat, + fromValue, + switchMap, + map, + scan, +} from 'wonka'; import { TypedDocumentNode, CombinedError, OperationContext, RequestPolicy, + OperationResult, Operation, } from '@urql/core'; @@ -38,6 +50,44 @@ export type UseQueryResponse = [ (opts?: Partial) => void ]; +/** Convert the Source to a React Suspense source on demand */ +function toSuspenseSource(source: Source): Source { + const shared = share(source); + let cache: T | void; + let resolve: (value: T) => void; + + return sink => { + let hasSuspended = false; + + pipe( + shared, + takeWhile(result => { + // The first result that is received will resolve the suspense + // promise after waiting for a microtick + if (cache === undefined) Promise.resolve(result).then(resolve); + cache = result; + return !hasSuspended; + }) + )(sink); + + // If we haven't got a previous result then start suspending + // otherwise issue the last known result immediately + if (cache !== undefined) { + const signal = [cache] as [T] & { tag: 1 }; + signal.tag = 1; + sink(signal); + } else { + hasSuspended = true; + sink(0 /* End */); + throw new Promise(_resolve => { + resolve = _resolve; + }); + } + }; +} + +const sources = new Map>(); + export function useQuery( args: UseQueryArgs ): UseQueryResponse { @@ -49,12 +99,27 @@ export function useQuery( // Create a new query-source from client.executeQuery const makeQuery$ = useCallback( (opts?: Partial) => { - return client.executeQuery(request, { - requestPolicy: args.requestPolicy, - pollInterval: args.pollInterval, - ...args.context, - ...opts, - }); + // Determine whether suspense is enabled for the given operation + const isSuspense = + client.suspense && (!args.context || args.context.suspense !== false); + + let source: Source | void = isSuspense + ? sources.get(request.key) + : undefined; + if (!source) { + source = client.executeQuery(request, { + requestPolicy: args.requestPolicy, + pollInterval: args.pollInterval, + ...args.context, + ...opts, + }); + + // Create a suspense source and cache it for the given request + if (isSuspense) + sources.set(request.key, (source = toSuspenseSource(source))); + } + + return source; }, [client, request, args.requestPolicy, args.pollInterval, args.context] ); @@ -108,10 +173,11 @@ export function useQuery( ); useEffect(() => { + sources.delete(request.key); // Delete any cached suspense source if (!client.suspense || (args.context && args.context.suspense === false)) { update(query$); } - }, [update, query$, args.context]); + }, [update, query$, request, args.context]); if (client.suspense && (!args.context || args.context.suspense !== false)) { update(query$); diff --git a/packages/react-urql/src/test-utils/ssr.test.tsx b/packages/react-urql/src/test-utils/ssr.test.tsx index 8bdca172df..42746e7661 100644 --- a/packages/react-urql/src/test-utils/ssr.test.tsx +++ b/packages/react-urql/src/test-utils/ssr.test.tsx @@ -98,23 +98,6 @@ describe('server-side rendering', () => { }); }); - it('correctly executes suspense and populates the SSR cache', async () => { - let promise; - - try { - pipe(client.executeRequestOperation(queryOperation), publish); - } catch (error) { - promise = error; - } - - expect(promise).toBeInstanceOf(Promise); - const result = await promise; - expect(result.data).not.toBe(undefined); - - const data = ssr.extractData(); - expect(Object.keys(data).length).toBe(1); - }); - it('works for an actual component tree', async () => { const Query = () => { useQuery({ From f38f167427737ee3314b8611fc792e0374b1e5b4 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Fri, 6 Nov 2020 11:04:12 +0000 Subject: [PATCH 07/11] Update preact-urql implementation with react-urql changes --- packages/preact-urql/src/hooks/useQuery.ts | 168 +++++++++++++----- packages/preact-urql/src/hooks/useSource.ts | 37 ++-- .../preact-urql/src/hooks/useSubscription.ts | 21 ++- 3 files changed, 145 insertions(+), 81 deletions(-) diff --git a/packages/preact-urql/src/hooks/useQuery.ts b/packages/preact-urql/src/hooks/useQuery.ts index fabc7e3c1e..f34447c3f0 100644 --- a/packages/preact-urql/src/hooks/useQuery.ts +++ b/packages/preact-urql/src/hooks/useQuery.ts @@ -1,12 +1,24 @@ import { DocumentNode } from 'graphql'; -import { useCallback, useMemo } from 'preact/hooks'; -import { pipe, concat, fromValue, switchMap, map, scan } from 'wonka'; +import { useEffect, useCallback, useMemo } from 'preact/hooks'; + +import { + Source, + pipe, + share, + takeWhile, + concat, + fromValue, + switchMap, + map, + scan, +} from 'wonka'; import { TypedDocumentNode, CombinedError, OperationContext, RequestPolicy, + OperationResult, Operation, } from '@urql/core'; @@ -38,11 +50,48 @@ export type UseQueryResponse = [ (opts?: Partial) => void ]; +/** Convert the Source to a React Suspense source on demand */ +function toSuspenseSource(source: Source): Source { + const shared = share(source); + let cache: T | void; + let resolve: (value: T) => void; + + return sink => { + let hasSuspended = false; + + pipe( + shared, + takeWhile(result => { + // The first result that is received will resolve the suspense + // promise after waiting for a microtick + if (cache === undefined) Promise.resolve(result).then(resolve); + cache = result; + return !hasSuspended; + }) + )(sink); + + // If we haven't got a previous result then start suspending + // otherwise issue the last known result immediately + if (cache !== undefined) { + const signal = [cache] as [T] & { tag: 1 }; + signal.tag = 1; + sink(signal); + } else { + hasSuspended = true; + sink(0 /* End */); + throw new Promise(_resolve => { + resolve = _resolve; + }); + } + }; +} + +const sources = new Map>(); + export function useQuery( args: UseQueryArgs ): UseQueryResponse { const client = useClient(); - // This creates a request which will keep a stable reference // if request.key doesn't change const request = useRequest(args.query, args.variables); @@ -50,55 +99,71 @@ export function useQuery( // Create a new query-source from client.executeQuery const makeQuery$ = useCallback( (opts?: Partial) => { - return client.executeQuery(request, { - requestPolicy: args.requestPolicy, - pollInterval: args.pollInterval, - ...args.context, - ...opts, - }); + // Determine whether suspense is enabled for the given operation + const isSuspense = + client.suspense && (!args.context || args.context.suspense !== false); + + let source: Source | void = isSuspense + ? sources.get(request.key) + : undefined; + if (!source) { + source = client.executeQuery(request, { + requestPolicy: args.requestPolicy, + pollInterval: args.pollInterval, + ...args.context, + ...opts, + }); + + // Create a suspense source and cache it for the given request + if (isSuspense) + sources.set(request.key, (source = toSuspenseSource(source))); + } + + return source; }, [client, request, args.requestPolicy, args.pollInterval, args.context] ); + const query$ = useMemo(() => { + return args.pause ? null : makeQuery$(); + }, [args.pause, makeQuery$]); + const [state, update] = useSource( - useMemo(() => (args.pause ? null : makeQuery$()), [args.pause, makeQuery$]), - useCallback( - (query$$, prevState: UseQueryState | undefined) => { - return pipe( - query$$, - switchMap(query$ => { - if (!query$) return fromValue({ fetching: false, stale: false }); - - 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 }), - ]); + query$, + useCallback((query$$, prevState?: UseQueryState) => { + return pipe( + query$$, + switchMap(query$ => { + if (!query$) return fromValue({ fetching: false, stale: false }); + + 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 }), + ]); + }), + // The individual partial results are merged into each previous result + scan( + (result: UseQueryState, partial) => ({ + ...result, + ...partial, }), - // The individual partial results are merged into each previous result - scan( - (result: UseQueryState, partial) => ({ - ...result, - ...partial, - }), - prevState || initialState - ) - ); - }, - [] - ) + prevState || initialState + ) + ); + }, []) ); // This is the imperative execute function passed to the user @@ -107,5 +172,16 @@ export function useQuery( [update, makeQuery$] ); + useEffect(() => { + sources.delete(request.key); // Delete any cached suspense source + if (!client.suspense || (args.context && args.context.suspense === false)) { + update(query$); + } + }, [update, query$, request, args.context]); + + if (client.suspense && (!args.context || args.context.suspense !== false)) { + update(query$); + } + return [state, executeQuery]; } diff --git a/packages/preact-urql/src/hooks/useSource.ts b/packages/preact-urql/src/hooks/useSource.ts index 1498f09904..84dc52561c 100644 --- a/packages/preact-urql/src/hooks/useSource.ts +++ b/packages/preact-urql/src/hooks/useSource.ts @@ -1,11 +1,9 @@ /* eslint-disable react-hooks/exhaustive-deps */ -import { useMemo, useEffect, useState, useRef } from 'preact/hooks'; +import { useMemo, useEffect, useState } from 'preact/hooks'; import { Source, fromValue, makeSubject, pipe, concat, subscribe } from 'wonka'; -import { useClient } from '../context'; - type Updater = (input: T) => void; let currentInit = false; @@ -19,18 +17,14 @@ const isShallowDifferent = (a: any, b: any) => { export function useSource( input: T, - transform: (input$: Source, initial: R | undefined) => Source + transform: (input$: Source, initial?: R) => Source ): [R, Updater] { - const client = useClient(); - const prev = useRef(); - const [input$, updateInput] = useMemo((): [Source, (value: T) => void] => { const subject = makeSubject(); const source = concat([fromValue(input), subject.source]); - let prevInput = input; - const updateInput = (input: T) => { - if (input !== prevInput) subject.next((prevInput = input)); + const updateInput = (nextInput: T) => { + if (nextInput !== input) subject.next((input = nextInput)); }; return [source, updateInput]; @@ -38,38 +32,29 @@ export function useSource( const [state, setState] = useState(() => { currentInit = true; - + let state: R; pipe( - transform(fromValue(input), prev.current), + transform(fromValue(input)), subscribe(value => { - prev.current = value; + state = value; }) ).unsubscribe(); - currentInit = false; - return prev.current!; + return state!; }); useEffect(() => { return pipe( - transform(input$, prev.current), + transform(input$, state), subscribe(value => { if (!currentInit) { setState(prevValue => { - return (prev.current = isShallowDifferent(prevValue, value) - ? value - : prevValue); + return isShallowDifferent(prevValue, value) ? value : prevValue; }); } }) ).unsubscribe; - }, [input$]); - - useEffect(() => { - if (!client.suspense) updateInput(input); - }, [updateInput, input]); - - if (client.suspense) updateInput(input); + }, [input$ /* `state` is only an initialiser */]); return [state, updateInput]; } diff --git a/packages/preact-urql/src/hooks/useSubscription.ts b/packages/preact-urql/src/hooks/useSubscription.ts index 405daead52..d1b0253df1 100644 --- a/packages/preact-urql/src/hooks/useSubscription.ts +++ b/packages/preact-urql/src/hooks/useSubscription.ts @@ -1,6 +1,7 @@ import { DocumentNode } from 'graphql'; -import { useCallback, useRef, useMemo } from 'preact/hooks'; +import { useEffect, useCallback, useRef, useMemo } from 'preact/hooks'; import { pipe, concat, fromValue, switchMap, map, scan } from 'wonka'; + import { TypedDocumentNode, CombinedError, @@ -62,16 +63,14 @@ export function useSubscription( [client, request, args.context] ); + const subscription$ = useMemo(() => { + return args.pause ? null : makeSubscription$(); + }, [args.pause, makeSubscription$]); + const [state, update] = useSource( - useMemo(() => (args.pause ? null : makeSubscription$()), [ - args.pause, - makeSubscription$, - ]), + subscription$, useCallback( - ( - subscription$$, - prevState: UseSubscriptionState | undefined - ) => { + (subscription$$, prevState?: UseSubscriptionState) => { return pipe( subscription$$, switchMap(subscription$ => { @@ -122,5 +121,9 @@ export function useSubscription( [update, makeSubscription$] ); + useEffect(() => { + update(subscription$); + }, [update, subscription$]); + return [state, executeSubscription]; } From e46f9ca8853a97dfdca34c031dcd52c955f47ac4 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Fri, 6 Nov 2020 11:07:49 +0000 Subject: [PATCH 08/11] Move client pollInternal logic to executeRequestOperation --- packages/core/src/client.ts | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 353afba42b..e66000a7b7 100755 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -257,7 +257,7 @@ export class Client { ) ); - return pipe( + const result$ = pipe( operationResults$, takeUntil(teardown$), onStart(() => { @@ -267,6 +267,15 @@ export class Client { this.onOperationEnd(operation); }) ); + + if (operation.kind === 'query' && operation.context.pollInterval) { + return pipe( + merge([fromValue(0), interval(operation.context.pollInterval)]), + switchMap(() => result$) + ); + } + + return result$; } query( @@ -308,17 +317,7 @@ export class Client { opts?: Partial ): Source> => { const operation = this.createRequestOperation('query', query, opts); - const response$ = this.executeRequestOperation(operation); - const { pollInterval } = operation.context; - - if (pollInterval) { - return pipe( - merge([fromValue(0), interval(pollInterval)]), - switchMap(() => response$) - ); - } - - return response$; + return this.executeRequestOperation(operation); }; subscription( From 120331e29db17cb05403ac5be3a41ee721381247 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Fri, 6 Nov 2020 11:27:29 +0000 Subject: [PATCH 09/11] Add changeset --- .changeset/strange-beans-sort.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/strange-beans-sort.md diff --git a/.changeset/strange-beans-sort.md b/.changeset/strange-beans-sort.md new file mode 100644 index 0000000000..c605c9bf09 --- /dev/null +++ b/.changeset/strange-beans-sort.md @@ -0,0 +1,7 @@ +--- +'@urql/core': minor +'@urql/preact': minor +'urql': minor +--- + +Improve the Suspense implementation, which fixes edge-cases when Suspense is used with subscriptions, partially disabled, or _used on the client-side_. It has now been ensured that client-side suspense functions without the deprecated `suspenseExchange` and uncached results are loaded consistently. As part of this work, the `Client` itself does now never throw Suspense promises anymore, which is functionality that either way has no place outside of the React/Preact bindings. From 3e355a60866dcb1f2e9bc36adc69c0fe039a2376 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Fri, 6 Nov 2020 11:47:02 +0000 Subject: [PATCH 10/11] Update @urql/exchange-suspense deprecation notice --- exchanges/suspense/README.md | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/exchanges/suspense/README.md b/exchanges/suspense/README.md index 95a35e450a..f0a59933f7 100644 --- a/exchanges/suspense/README.md +++ b/exchanges/suspense/README.md @@ -13,13 +13,11 @@ suspense on the server. But since `` is mainly intended for client-side use it made sense to build and publish this exchange, which allows you to try out `urql` and suspense in your React app! -> ⚠️ **\*Deprecated**: -> This package is deprecated! Usage of client-side suspense with `urql` isn't recommended anymore -> and this packages has been marked as _deprecated_ after being _experimental_, since all it allows -> for is to use Suspense as a fancier loading boundary, which isn't its intended use. -> This exchange may still be useful when used with care, but it's worth keeping in mind that the -> suspense patterns in `urql` for the client-side may change. -> Suspense-mode usage for SSR remains unchanged and undeprecated however. +## ⚠️ Deprecated + +Starting from `urql@1.11.0` / `@urql/preact@1.4.0` this exchange isn't required anymore to enable +client-side suspense support. Instead the `useQuery` hook internally handles React Suspense and +caches a result between Suspense and the subsequent re-mount of your component automatically. ## Quick Start Guide From 973469a84cd7d9f21426da7c1b05b24006b61be1 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Fri, 6 Nov 2020 12:49:27 +0000 Subject: [PATCH 11/11] Add predicate for isSuspense check --- packages/preact-urql/src/hooks/useQuery.ts | 18 ++++++++++-------- packages/react-urql/src/hooks/useQuery.ts | 18 ++++++++++-------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/preact-urql/src/hooks/useQuery.ts b/packages/preact-urql/src/hooks/useQuery.ts index f34447c3f0..538c2bb63c 100644 --- a/packages/preact-urql/src/hooks/useQuery.ts +++ b/packages/preact-urql/src/hooks/useQuery.ts @@ -14,6 +14,7 @@ import { } from 'wonka'; import { + Client, TypedDocumentNode, CombinedError, OperationContext, @@ -86,6 +87,9 @@ function toSuspenseSource(source: Source): Source { }; } +const isSuspense = (client: Client, context?: Partial) => + client.suspense && (!context || context.suspense !== false); + const sources = new Map>(); export function useQuery( @@ -100,10 +104,8 @@ export function useQuery( const makeQuery$ = useCallback( (opts?: Partial) => { // Determine whether suspense is enabled for the given operation - const isSuspense = - client.suspense && (!args.context || args.context.suspense !== false); - - let source: Source | void = isSuspense + const suspense = isSuspense(client, args.context); + let source: Source | void = suspense ? sources.get(request.key) : undefined; if (!source) { @@ -115,7 +117,7 @@ export function useQuery( }); // Create a suspense source and cache it for the given request - if (isSuspense) + if (suspense) sources.set(request.key, (source = toSuspenseSource(source))); } @@ -174,12 +176,12 @@ export function useQuery( useEffect(() => { sources.delete(request.key); // Delete any cached suspense source - if (!client.suspense || (args.context && args.context.suspense === false)) { + if (!isSuspense(client, args.context)) { update(query$); } - }, [update, query$, request, args.context]); + }, [update, client, query$, request, args.context]); - if (client.suspense && (!args.context || args.context.suspense !== false)) { + if (isSuspense(client, args.context)) { update(query$); } diff --git a/packages/react-urql/src/hooks/useQuery.ts b/packages/react-urql/src/hooks/useQuery.ts index 78aab534db..35804c2202 100644 --- a/packages/react-urql/src/hooks/useQuery.ts +++ b/packages/react-urql/src/hooks/useQuery.ts @@ -14,6 +14,7 @@ import { } from 'wonka'; import { + Client, TypedDocumentNode, CombinedError, OperationContext, @@ -86,6 +87,9 @@ function toSuspenseSource(source: Source): Source { }; } +const isSuspense = (client: Client, context?: Partial) => + client.suspense && (!context || context.suspense !== false); + const sources = new Map>(); export function useQuery( @@ -100,10 +104,8 @@ export function useQuery( const makeQuery$ = useCallback( (opts?: Partial) => { // Determine whether suspense is enabled for the given operation - const isSuspense = - client.suspense && (!args.context || args.context.suspense !== false); - - let source: Source | void = isSuspense + const suspense = isSuspense(client, args.context); + let source: Source | void = suspense ? sources.get(request.key) : undefined; if (!source) { @@ -115,7 +117,7 @@ export function useQuery( }); // Create a suspense source and cache it for the given request - if (isSuspense) + if (suspense) sources.set(request.key, (source = toSuspenseSource(source))); } @@ -174,12 +176,12 @@ export function useQuery( useEffect(() => { sources.delete(request.key); // Delete any cached suspense source - if (!client.suspense || (args.context && args.context.suspense === false)) { + if (!isSuspense(client, args.context)) { update(query$); } - }, [update, query$, request, args.context]); + }, [update, client, query$, request, args.context]); - if (client.suspense && (!args.context || args.context.suspense !== false)) { + if (isSuspense(client, args.context)) { update(query$); }