Point useSubscription to useSyncExternalStore shim (#24289)

* Point useSubscription to useSyncExternalStore shim * Update tests * Update README * Ad hoc case
facebook · Apr 11, 2022 · 4997515 · 4997515
1 parent df5d32f
commit 4997515
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 139 deletions.
diff --git a/packages/use-subscription/README.md b/packages/use-subscription/README.md
@@ -1,32 +1,8 @@
 # use-subscription
 
-React hook that safely manages subscriptions in concurrent mode.
+React Hook for subscribing to external data sources.
 
-This utility can be used for subscriptions to a single value that are typically only read in one place and may update frequently (e.g. a component that subscribes to a geolocation API to show a dot on a map).
-
-## When should you NOT use this?
-
-Most other cases have **better long-term solutions**:
-* Redux/Flux stores should use the [context API](https://reactjs.org/docs/context.html) instead.
-* I/O subscriptions (e.g. notifications) that update infrequently should use a mechanism like [`react-cache`](https://github.com/facebook/react/blob/main/packages/react-cache/README.md) instead.
-* Complex libraries like Relay/Apollo should manage subscriptions manually with the same techniques which this library uses under the hood (as referenced [here](https://gist.github.com/bvaughn/d569177d70b50b58bff69c3c4a5353f3)) in a way that is most optimized for their library usage.
-
-## Limitations in concurrent mode
-
-`use-subscription` is safe to use in concurrent mode. However, [it achieves correctness by sometimes de-opting to synchronous mode](https://github.com/facebook/react/issues/13186#issuecomment-403959161), obviating the benefits of concurrent rendering. This is an inherent limitation of storing state outside of React's managed state queue and rendering in response to a change event.
-
-The effect of de-opting to sync mode is that the main thread may periodically be blocked (in the case of CPU-bound work), and placeholders may appear earlier than desired (in the case of IO-bound work).
-
-For **full compatibility** with concurrent rendering, including both **time-slicing** and **React Suspense**, the suggested longer-term solution is to move to one of the patterns described in the previous section.
-
-## What types of subscriptions can this support?
-
-This abstraction can handle a variety of subscription types, including:
-* Event dispatchers like `HTMLInputElement`.
-* Custom pub/sub components like Relay's `FragmentSpecResolver`.
-* Observable types like RxJS `BehaviorSubject` and `ReplaySubject`. (Types like RxJS `Subject` or `Observable` are not supported, because they provide no way to read the "current" value after it has been emitted.)
-
-Note that JavaScript promises are also **not supported** because they provide no way to synchronously read the "current" value.
+**You may now migrate to [`use-sync-external-store`](https://www.npmjs.com/package/use-sync-external-store) directly instead, which has the same API as [`React.useSyncExternalStore`](https://reactjs.org/docs/hooks-reference.html#usesyncexternalstore). The `use-subscription` package is now a thin wrapper over `use-sync-external-store` and will not be updated further.**
 
 # Installation
 

diff --git a/packages/use-subscription/package.json b/packages/use-subscription/package.json
@@ -15,9 +15,12 @@
   ],
   "license": "MIT",
   "peerDependencies": {
-    "react": "^18.0.0"
+    "react": "^16.8.0 || ^17.0.0 || ^18.0.0"
   },
   "devDependencies": {
     "rxjs": "^5.5.6"
+  },
+  "dependencies": {
+    "use-sync-external-store": "^1.0.0"
   }
 }
diff --git a/packages/use-subscription/src/__tests__/useSubscription-test.js b/packages/use-subscription/src/__tests__/useSubscription-test.js
@@ -457,17 +457,13 @@ describe('useSubscription', () => {
       renderer.update(<Parent observed={observableA} />);
 
       // Flush everything and ensure that the correct subscribable is used
-      // We expect the new subscribable to finish rendering,
-      // But then the updated values from the old subscribable should be used.
       expect(Scheduler).toFlushAndYield([
-        'Grandchild: b-0',
+        'Child: a-2',
+        'Grandchild: a-2',
         'Child: a-2',
         'Grandchild: a-2',
       ]);
-      expect(log).toEqual([
-        'Parent.componentDidUpdate:b-0',
-        'Parent.componentDidUpdate:a-2',
-      ]);
+      expect(log).toEqual(['Parent.componentDidUpdate:a-2']);
     });
 
     // Updates from the new subscribable should be ignored.
@@ -628,19 +624,18 @@ describe('useSubscription', () => {
       } else {
         mutate('C');
       }
-      expect(Scheduler).toFlushAndYieldThrough(['render:first:C']);
+      expect(Scheduler).toFlushAndYieldThrough([
+        'render:first:C',
+        'render:second:C',
+      ]);
       if (gate(flags => flags.enableSyncDefaultUpdates)) {
         React.startTransition(() => {
           mutate('D');
         });
       } else {
         mutate('D');
       }
-      expect(Scheduler).toFlushAndYield([
-        'render:second:C',
-        'render:first:D',
-        'render:second:D',
-      ]);
+      expect(Scheduler).toFlushAndYield(['render:first:D', 'render:second:D']);
 
       // No more pending updates
       jest.runAllTimers();

diff --git a/packages/use-subscription/src/useSubscription.js b/packages/use-subscription/src/useSubscription.js
@@ -7,7 +7,7 @@
  * @flow
  */
 
-import {useDebugValue, useEffect, useState} from 'react';
+import {useSyncExternalStore} from 'use-sync-external-store/shim';
 
 // Hook used for safely managing subscriptions in concurrent mode.
 //
@@ -26,100 +26,5 @@ export function useSubscription<Value>({
   getCurrentValue: () => Value,
   subscribe: (callback: Function) => () => void,
 |}): Value {
-  // Read the current value from our subscription.
-  // When this value changes, we'll schedule an update with React.
-  // It's important to also store the hook params so that we can check for staleness.
-  // (See the comment in checkForUpdates() below for more info.)
-  const [state, setState] = useState(() => ({
-    getCurrentValue,
-    subscribe,
-    value: getCurrentValue(),
-  }));
-
-  let valueToReturn = state.value;
-
-  // If parameters have changed since our last render, schedule an update with its current value.
-  if (
-    state.getCurrentValue !== getCurrentValue ||
-    state.subscribe !== subscribe
-  ) {
-    // If the subscription has been updated, we'll schedule another update with React.
-    // React will process this update immediately, so the old subscription value won't be committed.
-    // It is still nice to avoid returning a mismatched value though, so let's override the return value.
-    valueToReturn = getCurrentValue();
-
-    setState({
-      getCurrentValue,
-      subscribe,
-      value: valueToReturn,
-    });
-  }
-
-  // Display the current value for this hook in React DevTools.
-  useDebugValue(valueToReturn);
-
-  // It is important not to subscribe while rendering because this can lead to memory leaks.
-  // (Learn more at reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects)
-  // Instead, we wait until the commit phase to attach our handler.
-  //
-  // We intentionally use a passive effect (useEffect) rather than a synchronous one (useLayoutEffect)
-  // so that we don't stretch the commit phase.
-  // This also has an added benefit when multiple components are subscribed to the same source:
-  // It allows each of the event handlers to safely schedule work without potentially removing an another handler.
-  // (Learn more at https://codesandbox.io/s/k0yvr5970o)
-  useEffect(() => {
-    let didUnsubscribe = false;
-
-    const checkForUpdates = () => {
-      // It's possible that this callback will be invoked even after being unsubscribed,
-      // if it's removed as a result of a subscription event/update.
-      // In this case, React will log a DEV warning about an update from an unmounted component.
-      // We can avoid triggering that warning with this check.
-      if (didUnsubscribe) {
-        return;
-      }
-
-      // We use a state updater function to avoid scheduling work for a stale source.
-      // However it's important to eagerly read the currently value,
-      // so that all scheduled work shares the same value (in the event of multiple subscriptions).
-      // This avoids visual "tearing" when a mutation happens during a (concurrent) render.
-      const value = getCurrentValue();
-
-      setState(prevState => {
-        // Ignore values from stale sources!
-        // Since we subscribe an unsubscribe in a passive effect,
-        // it's possible that this callback will be invoked for a stale (previous) subscription.
-        // This check avoids scheduling an update for that stale subscription.
-        if (
-          prevState.getCurrentValue !== getCurrentValue ||
-          prevState.subscribe !== subscribe
-        ) {
-          return prevState;
-        }
-
-        // Some subscriptions will auto-invoke the handler, even if the value hasn't changed.
-        // If the value hasn't changed, no update is needed.
-        // Return state as-is so React can bail out and avoid an unnecessary render.
-        if (prevState.value === value) {
-          return prevState;
-        }
-
-        return {...prevState, value};
-      });
-    };
-    const unsubscribe = subscribe(checkForUpdates);
-
-    // Because we're subscribing in a passive effect,
-    // it's possible that an update has occurred between render and our effect handler.
-    // Check for this and schedule an update if work has occurred.
-    checkForUpdates();
-
-    return () => {
-      didUnsubscribe = true;
-      unsubscribe();
-    };
-  }, [getCurrentValue, subscribe]);
-
-  // Return the current value for our caller to use while rendering.
-  return valueToReturn;
+  return useSyncExternalStore(subscribe, getCurrentValue);
 }
diff --git a/packages/use-sync-external-store/package.json b/packages/use-sync-external-store/package.json
@@ -19,6 +19,6 @@
   ],
   "license": "MIT",
   "peerDependencies": {
-    "react": "^16.8.0 || ^17.0.0 || ^18.0.0-rc"
+    "react": "^16.8.0 || ^17.0.0 || ^18.0.0"
   }
 }
diff --git a/scripts/rollup/build-all-release-channels.js b/scripts/rollup/build-all-release-channels.js
@@ -251,7 +251,11 @@ function updatePackageVersions(
         }
       }
       if (packageInfo.peerDependencies) {
-        if (!pinToExactVersion && moduleName === 'use-sync-external-store') {
+        if (
+          !pinToExactVersion &&
+          (moduleName === 'use-sync-external-store' ||
+            moduleName === 'use-subscription')
+        ) {
           // use-sync-external-store supports older versions of React, too, so
           // we don't override to the latest version. We should figure out some
           // better way to handle this.