Internalize useSyncExternalStore shim, for more control than use-sync-external-store provides

CHANGELOG.md

@@ -1,3 +1,10 @@
+## Apollo Client 3.6.4 (unreleased)
+
+### Improvements
+
+- Internalize `useSyncExternalStore` shim, for more control than `use-sync-external-store` provides, fixing some React Native issues. <br/>
+  [@benjamn](https://github.com/benjamn) in [#9675](https://github.com/apollographql/apollo-client/pull/9675)
+
 ## Apollo Client 3.6.3 (unreleased)
 
 ### Bug Fixes

package.json

@@ -56,7 +56,7 @@
     {
       "name": "apollo-client",
       "path": "./dist/apollo-client.min.cjs",
-      "maxSize": "29.4kB"
+      "maxSize": "29.45kB"
     }
   ],
   "engines": {
@@ -81,6 +81,7 @@
   },
   "dependencies": {
     "@graphql-typed-document-node/core": "^3.1.1",
+    "@types/use-sync-external-store": "^0.0.3",
     "@wry/context": "^0.6.0",
     "@wry/equality": "^0.5.0",
     "@wry/trie": "^0.3.0",
@@ -91,7 +92,6 @@
     "symbol-observable": "^4.0.0",
     "ts-invariant": "^0.10.2",
     "tslib": "^2.3.0",
-    "use-sync-external-store": "^1.0.0",
     "zen-observable-ts": "^1.2.0"
   },
   "devDependencies": {
@@ -109,7 +109,6 @@
     "@types/node": "16.11.33",
     "@types/react": "17.0.45",
     "@types/react-dom": "17.0.16",
-    "@types/use-sync-external-store": "0.0.3",
     "acorn": "8.7.1",
     "bundlesize": "0.18.1",
     "cross-fetch": "3.1.5",

src/__tests__/__snapshots__/exports.ts.snap

@@ -347,6 +347,8 @@ Array [
   "argumentsObjectFromField",
   "asyncMap",
   "buildQueryFromSelectionSet",
+  "canUseDOM",
+  "canUseLayoutEffect",
   "canUseSymbol",
   "canUseWeakMap",
   "canUseWeakSet",

src/react/hooks/useQuery.ts

@@ -7,7 +7,7 @@ import {
   useRef,
   useState,
 } from 'react';
-import { useSyncExternalStore } from 'use-sync-external-store/shim/index.js';
+import { useSyncExternalStore } from './useSyncExternalStore';
 import { equal } from '@wry/equality';
 
 import { mergeOptions, OperationVariables, WatchQueryFetchPolicy } from '../../core';

src/react/hooks/useSyncExternalStore.ts

@@ -0,0 +1,129 @@
+import { invariant } from '../../utilities/globals';
+import * as React from 'react';
+
+import { canUseLayoutEffect } from '../../utilities';
+
+let didWarnUncachedGetSnapshot = false;
+
+type RealUseSESHookType =
+  // This import depends only on the @types/use-sync-external-store package, not
+  // the actual use-sync-external-store package, which is not installed. It
+  // might be nice to get this type from React 18, but it still needs to work
+  // when only React 17 or earlier is installed.
+  typeof import("use-sync-external-store").useSyncExternalStore;
+
+// Adapted from https://www.npmjs.com/package/use-sync-external-store, with
+// Apollo Client deviations called out by "// DEVIATION ..." comments.
+
+export const useSyncExternalStore: RealUseSESHookType = (
+  subscribe,
+  getSnapshot,
+  getServerSnapshot,
+) => {
+  // When/if React.useSyncExternalStore is defined, delegate fully to it.
+  const realHook = (React as {
+    useSyncExternalStore?: RealUseSESHookType;
+  }).useSyncExternalStore;
+  if (realHook) {
+    return realHook(subscribe, getSnapshot, getServerSnapshot);
+  }
+
+  // Read the current snapshot from the store on every render. Again, this
+  // breaks the rules of React, and only works here because of specific
+  // implementation details, most importantly that updates are
+  // always synchronous.
+  const value = getSnapshot();
+  if (
+    // DEVIATION: Using our own __DEV__ polyfill (from ../../utilities/globals).
+    __DEV__ &&
+    !didWarnUncachedGetSnapshot &&
+    // DEVIATION: Not using Object.is because we know our snapshots will never
+    // be exotic primitive values like NaN, which is !== itself.
+    value !== getSnapshot()
+  ) {
+    didWarnUncachedGetSnapshot = true;
+    // DEVIATION: Using invariant.error instead of console.error directly.
+    invariant.error(
+      'The result of getSnapshot should be cached to avoid an infinite loop',
+    );
+  }
+
+  // Because updates are synchronous, we don't queue them. Instead we force a
+  // re-render whenever the subscribed state changes by updating an some
+  // arbitrary useState hook. Then, during render, we call getSnapshot to read
+  // the current value.
+  //
+  // Because we don't actually use the state returned by the useState hook, we
+  // can save a bit of memory by storing other stuff in that slot.
+  //
+  // To implement the early bailout, we need to track some things on a mutable
+  // object. Usually, we would put that in a useRef hook, but we can stash it in
+  // our useState hook instead.
+  //
+  // To force a re-render, we call forceUpdate({inst}). That works because the
+  // new object always fails an equality check.
+  const [{inst}, forceUpdate] = React.useState({inst: {value, getSnapshot}});
+
+  // Track the latest getSnapshot function with a ref. This needs to be updated
+  // in the layout phase so we can access it during the tearing check that
+  // happens on subscribe.
+  if (canUseLayoutEffect) {
+    // DEVIATION: We avoid calling useLayoutEffect when !canUseLayoutEffect,
+    // which may seem like a conditional hook, but this code ends up behaving
+    // unconditionally (one way or the other) because canUseLayoutEffect is
+    // constant.
+    React.useLayoutEffect(() => {
+      Object.assign(inst, { value, getSnapshot });
+      // Whenever getSnapshot or subscribe changes, we need to check in the
+      // commit phase if there was an interleaved mutation. In concurrent mode
+      // this can happen all the time, but even in synchronous mode, an earlier
+      // effect may have mutated the store.
+      if (checkIfSnapshotChanged(inst)) {
+        // Force a re-render.
+        forceUpdate({inst});
+      }
+    }, [subscribe, value, getSnapshot]);
+  } else {
+    Object.assign(inst, { value, getSnapshot });
+  }
+
+  React.useEffect(() => {
+    // Check for changes right before subscribing. Subsequent changes will be
+    // detected in the subscription handler.
+    if (checkIfSnapshotChanged(inst)) {
+      // Force a re-render.
+      forceUpdate({inst});
+    }
+
+    // Subscribe to the store and return a clean-up function.
+    return subscribe(function handleStoreChange() {
+      // TODO: Because there is no cross-renderer API for batching updates, it's
+      // up to the consumer of this library to wrap their subscription event
+      // with unstable_batchedUpdates. Should we try to detect when this isn't
+      // the case and print a warning in development?
+
+      // The store changed. Check if the snapshot changed since the last time we
+      // read from the store.
+      if (checkIfSnapshotChanged(inst)) {
+        // Force a re-render.
+        forceUpdate({inst});
+      }
+    });
+  }, [subscribe]);
+
+  return value;
+}
+
+function checkIfSnapshotChanged<Snapshot>({
+  value,
+  getSnapshot,
+}: {
+  value: Snapshot;
+  getSnapshot: () => Snapshot;
+}): boolean {
+  try {
+    return value !== getSnapshot();
+  } catch {
+    return true;
+  }
+}

src/utilities/common/__tests__/canUse.ts

@@ -0,0 +1,11 @@
+import { canUseDOM, canUseLayoutEffect } from "../canUse";
+
+describe("canUse* boolean constants", () => {
+  // https://github.com/apollographql/apollo-client/pull/9675
+  it("sets canUseDOM to true when using Jest in Node.js with jsdom", () => {
+    expect(canUseDOM).toBe(true);
+  });
+  it("sets canUseLayoutEffect to false when using Jest in Node.js with jsdom", () => {
+    expect(canUseLayoutEffect).toBe(false);
+  });
+});

src/utilities/common/canUse.ts

@@ -1,10 +1,33 @@
-export const canUseWeakMap = typeof WeakMap === 'function' && !(
-  typeof navigator === 'object' &&
-  navigator.product === 'ReactNative'
-);
+import { maybe } from "../globals";
+
+export const canUseWeakMap =
+  typeof WeakMap === 'function' &&
+  maybe(() => navigator.product) !== 'ReactNative';
 
 export const canUseWeakSet = typeof WeakSet === 'function';
 
 export const canUseSymbol =
   typeof Symbol === 'function' &&
   typeof Symbol.for === 'function';
+
+export const canUseDOM =
+  typeof maybe(() => window.document.createElement) === "function";
+
+const usingJSDOM: boolean =
+  // Following advice found in this comment from @domenic (maintainer of jsdom):
+  // https://github.com/jsdom/jsdom/issues/1537#issuecomment-229405327
+  //
+  // Since we control the version of Jest and jsdom used when running Apollo
+  // Client tests, and that version is recent enought to include " jsdom/x.y.z"
+  // at the end of the user agent string, I believe this case is all we need to
+  // check. Testing for "Node.js" was recommended for backwards compatibility
+  // with older version of jsdom, but we don't have that problem.
+  maybe(() => navigator.userAgent.indexOf("jsdom") >= 0) || false;
+
+// Our tests should all continue to pass if we remove this !usingJSDOM
+// condition, thereby allowing useLayoutEffect when using jsdom. Unfortunately,
+// if we allow useLayoutEffect, then useSyncExternalStore generates many
+// warnings about useLayoutEffect doing nothing on the server. While these
+// warnings are harmless, this !usingJSDOM condition seems to be the best way to
+// prevent them (i.e. skipping useLayoutEffect when using jsdom).
+export const canUseLayoutEffect = canUseDOM && !usingJSDOM;