(react) - leverage the new use-sync-external-store package

.changeset/wicked-terms-sneeze.md

@@ -0,0 +1,5 @@
+---
+'urql': minor
+---
+
+Leverage the new `use-sync-external-store` package and `useSyncExternalStore` hook in `useQuery` implementation to bring the state synchronisation in React in line with React v18. While the current implementation works already with React Suspense and React Concurrent this will reduce the maintenance burden of our implementation and ensure certain guarantees so that React doesn't break us.

packages/react-urql/package.json

@@ -47,6 +47,7 @@
     "@testing-library/react-hooks": "^5.1.2",
     "@types/react": "^17.0.4",
     "@types/react-test-renderer": "^17.0.1",
+    "@types/use-sync-external-store": "^0.0.3",
     "graphql": "^16.0.0",
     "react": "^17.0.1",
     "react-dom": "^17.0.1",
@@ -60,6 +61,7 @@
   },
   "dependencies": {
     "@urql/core": "^2.3.6",
+    "use-sync-external-store": "^1.0.0-rc.0 || ^1.0.0",
     "wonka": "^4.0.14"
   }
 }

packages/react-urql/src/hooks/useQuery.ts

@@ -1,9 +1,8 @@
 /* eslint-disable react-hooks/exhaustive-deps */
-
 import { DocumentNode } from 'graphql';
-import { Source, pipe, subscribe, onEnd, onPush, takeWhile } from 'wonka';
-import { useState, useEffect, useCallback, useMemo } from 'react';
-
+import { Source, pipe, subscribe, takeWhile } from 'wonka';
+import { useCallback, useMemo, useState } from 'react';
+import { useSyncExternalStore } from 'use-sync-external-store/shim';
 import {
   Client,
   TypedDocumentNode,
@@ -16,8 +15,8 @@ import {
 
 import { useClient } from '../context';
 import { useRequest } from './useRequest';
+import { hasDepsChanged, computeNextState, initialState } from './state';
 import { getCacheForClient } from './cache';
-import { initialState, computeNextState, hasDepsChanged } from './state';
 
 export interface UseQueryArgs<Variables = object, Data = any> {
   query: string | DocumentNode | TypedDocumentNode<Data, Variables>;
@@ -41,70 +40,67 @@ export type UseQueryResponse<Data = any, Variables = object> = [
   (opts?: Partial<OperationContext>) => void
 ];
 
+const notFetching = initialState;
+const fetching = { ...initialState, fetching: true };
+
 const isSuspense = (client: Client, context?: Partial<OperationContext>) =>
   client.suspense && (!context || context.suspense !== false);
 
-let currentInit = false;
-
 export function useQuery<Data = any, Variables = object>(
   args: UseQueryArgs<Variables, Data>
 ): UseQueryResponse<Data, Variables> {
   const client = useClient();
-  const cache = getCacheForClient(client);
-  const suspense = isSuspense(client, args.context);
   const request = useRequest<Data, Variables>(args.query, args.variables);
+  const cache = getCacheForClient(client);
 
-  const source = useMemo(() => {
-    if (args.pause) return null;
-
-    const source = client.executeQuery(request, {
-      requestPolicy: args.requestPolicy,
-      ...args.context,
-    });
+  const [meta, setMeta] = useState<{
+    source: Source<OperationResult<Data, Variables>> | null;
+    prevValue: UseQueryState<Data, Variables>;
+    deps: Array<any>;
+    suspense: boolean;
+  }>({
+    source: null,
+    prevValue: notFetching,
+    deps: [],
+    suspense: isSuspense(client, args.context),
+  });
 
-    return suspense
-      ? pipe(
-          source,
-          onPush(result => {
-            cache.set(request.key, result);
-          })
-        )
-      : source;
-  }, [
-    cache,
-    client,
-    request,
-    suspense,
-    args.pause,
-    args.requestPolicy,
-    args.context,
-  ]);
+  const { source, deps, suspense } = meta;
 
-  const getSnapshot = useCallback(
-    (
-      source: Source<OperationResult<Data, Variables>> | null,
-      suspense: boolean
-    ): Partial<UseQueryState<Data, Variables>> => {
-      if (!source) return { fetching: false };
+  const [getSnapshot, sub] = useMemo(() => {
+    let result = cache.get(request.key);
 
-      let result = cache.get(request.key);
-      if (!result) {
-        let resolve: (value: unknown) => void;
+    const getSnapshot = (): Partial<UseQueryState<Data, Variables>> => {
+      if (!source) {
+        return notFetching;
+      } else if (!result) {
+        let resolve:
+          | ((result: OperationResult<Data, Variables>) => void)
+          | undefined;
 
         const subscription = pipe(
           source,
-          takeWhile(() => (suspense && !resolve) || !result),
+          takeWhile(
+            () =>
+              (suspense && (!resolve || (result && (result as any).then))) ||
+              !result
+          ),
           subscribe(_result => {
+            if (suspense) {
+              cache.set(request.key, result);
+            }
             result = _result;
-            if (resolve) resolve(result);
+            if (resolve) {
+              resolve(result);
+              resolve = undefined;
+            }
           })
         );
 
         if (result == null && suspense) {
-          const promise = new Promise(_resolve => {
+          const promise = (result = new Promise(_resolve => {
             resolve = _resolve;
-          });
-
+          }));
           cache.set(request.key, promise);
           throw promise;
         } else {
@@ -114,81 +110,35 @@ export function useQuery<Data = any, Variables = object>(
         throw result;
       }
 
-      return (result as OperationResult<Data, Variables>) || { fetching: true };
-    },
-    [cache, request]
-  );
-
-  const deps = [
-    client,
-    request,
-    args.requestPolicy,
-    args.context,
-    args.pause,
-  ] as const;
-
-  const [state, setState] = useState(() => {
-    currentInit = true;
-    try {
-      return [
-        source,
-        computeNextState(initialState, getSnapshot(source, suspense)),
-        deps,
-      ] as const;
-    } finally {
-      currentInit = false;
-    }
-  });
-
-  let currentResult = state[1];
-  if (source !== state[0] && hasDepsChanged(state[2], deps)) {
-    setState([
-      source,
-      (currentResult = computeNextState(
-        state[1],
-        getSnapshot(source, suspense)
-      )),
-      deps,
-    ]);
-  }
+      return (result as OperationResult<Data, Variables>) || fetching;
+    };
 
-  useEffect(() => {
-    const source = state[0];
-    const request = state[2][1];
-
-    let hasResult = false;
-
-    const updateResult = (result: Partial<UseQueryState<Data, Variables>>) => {
-      hasResult = true;
-      if (!currentInit) {
-        setState(state => {
-          const nextResult = computeNextState(state[1], result);
-          return state[1] !== nextResult
-            ? [state[0], nextResult, state[2]]
-            : state;
-        });
+    const sub = (notify: () => void) => {
+      if (!source) {
+        return () => {
+          /*noop*/
+        };
       }
-    };
 
-    if (source) {
-      const subscription = pipe(
+      const unsub = pipe(
         source,
-        onEnd(() => {
-          updateResult({ fetching: false });
-        }),
-        subscribe(updateResult)
-      );
-
-      if (!hasResult) updateResult({ fetching: true });
+        subscribe(_result => {
+          if (suspense) {
+            cache.set(request.key, result);
+          }
+          result = _result;
+          notify();
+        })
+      ).unsubscribe;
 
       return () => {
         cache.dispose(request.key);
-        subscription.unsubscribe();
+        unsub();
       };
-    } else {
-      updateResult({ fetching: false });
-    }
-  }, [cache, state[0], state[2][1]]);
+    };
+
+    return [getSnapshot, sub];
+  }, [source, args.pause]);
 
   const executeQuery = useCallback(
     (opts?: Partial<OperationContext>) => {
@@ -198,28 +148,46 @@ export function useQuery<Data = any, Variables = object>(
         ...opts,
       };
 
-      setState(state => {
-        const source = suspense
-          ? pipe(
-              client.executeQuery(request, context),
-              onPush(result => {
-                cache.set(request.key, result);
-              })
-            )
-          : client.executeQuery(request, context);
-        return [source, state[1], deps];
-      });
+      setMeta(prev => ({
+        prevValue: prev.prevValue,
+        deps: prev.deps,
+        source: client.executeQuery(request, context),
+        suspense: isSuspense(client, context),
+      }));
     },
-    [
-      client,
-      cache,
-      request,
-      suspense,
-      getSnapshot,
-      args.requestPolicy,
-      args.context,
-    ]
+    [client, request, args.requestPolicy, args.context]
   );
 
-  return [currentResult, executeQuery];
+  const result = (meta.prevValue = computeNextState(
+    meta.prevValue,
+    useSyncExternalStore<Partial<UseQueryState<Data, Variables>>>(
+      sub,
+      getSnapshot,
+      getSnapshot
+    )
+  ));
+
+  const currDeps = [
+    client,
+    request,
+    args.pause,
+    args.requestPolicy,
+    args.context,
+  ];
+
+  if (hasDepsChanged(deps, currDeps) && !args.pause) {
+    const fetchSource = client.executeQuery(request, {
+      requestPolicy: args.requestPolicy,
+      ...args.context,
+    });
+
+    setMeta({
+      prevValue: result,
+      source: args.pause ? null : fetchSource,
+      deps: currDeps,
+      suspense: isSuspense(client, args.context),
+    });
+  }
+
+  return [result, executeQuery];
 }

yarn.lock

@@ -2977,6 +2977,11 @@
   resolved "https://registry.yarnpkg.com/@types/unist/-/unist-2.0.3.tgz#9c088679876f374eb5983f150d4787aa6fb32d7e"
   integrity sha512-FvUupuM3rlRsRtCN+fDudtmytGO6iHJuuRKS1Ss0pG5z8oX0diNEw94UEL7hgDbpN94rgaK5R7sWm6RrSkZuAQ==
 
+"@types/use-sync-external-store@^0.0.3":
+  version "0.0.3"
+  resolved "https://registry.yarnpkg.com/@types/use-sync-external-store/-/use-sync-external-store-0.0.3.tgz#b6725d5f4af24ace33b36fafd295136e75509f43"
+  integrity sha512-EwmlvuaxPNej9+T4v5AuBPJa2x2UOJVdjCtDHgcDqitUeOtjnJKJ+apYjVcAoBEMjKW1VVFGZLUb5+qqa09XFA==
+
 "@types/webpack-env@^1.15.3", "@types/webpack-env@^1.16.0":
   version "1.16.0"
   resolved "https://registry.yarnpkg.com/@types/webpack-env/-/webpack-env-1.16.0.tgz#8c0a9435dfa7b3b1be76562f3070efb3f92637b4"
@@ -16350,6 +16355,11 @@ [email protected]:
   dependencies:
     object-assign "^4.1.1"
 
+"use-sync-external-store@^1.0.0-rc.0 || ^1.0.0":
+  version "1.0.0-rc.0-next-fe905f152-20220107"
+  resolved "https://registry.yarnpkg.com/use-sync-external-store/-/use-sync-external-store-1.0.0-rc.0-next-fe905f152-20220107.tgz#1a244bd9c97f6853552df1f5a29fca726e7a3e96"
+  integrity sha512-1kK4h4Qubjnkmk6UObQKkYv+XwGTd5SuH6/hx1dfzEEvIWLkVd326Lg7Y1EOxfMUxJ5binWXndYX8ZrXFKHGuw==
+
 use@^3.1.0:
   version "3.1.1"
   resolved "https://registry.yarnpkg.com/use/-/use-3.1.1.tgz#d50c8cac79a19fbc20f2911f56eb973f4e10070f"