[wip] Combine useRef and useState hooks

Since the useState hook is only used to force a render, and we don't
use it to track any state, we can stash our mutable instance object
in there instead of using a separate useRef hook. Doesn't affect any
behavior, just saves a bit of hook memory.

Author: acdlite

packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js

@@ -96,9 +96,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     };
   }
 
-  // 18 has an experimental flag to warn about reading refs. Will circumvent
-  // when built-in API is implemented.
-  // @gate !enableUseRefAccessWarning || !__DEV__
   test('basic usage', () => {
     const store = createExternalStore('Initial');
 
@@ -141,9 +138,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     expect(root).toMatchRenderedOutput('Initial');
   });
 
-  // 18 has an experimental flag to warn about reading refs. Will circumvent
-  // when built-in API is implemented.
-  // @gate !enableUseRefAccessWarning || !__DEV__
   test('switch to a different store', () => {
     const storeA = createExternalStore(0);
     const storeB = createExternalStore(0);
@@ -193,9 +187,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     expect(root).toMatchRenderedOutput('1');
   });
 
-  // 18 has an experimental flag to warn about reading refs. Will circumvent
-  // when built-in API is implemented.
-  // @gate !enableUseRefAccessWarning || !__DEV__
   test('selecting a specific value inside getSnapshot', () => {
     const store = createExternalStore({a: 0, b: 0});
 
@@ -239,9 +230,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     expect(root).toMatchRenderedOutput('A1B1');
   });
 
-  // 18 has an experimental flag to warn about reading refs. Will circumvent
-  // when built-in API is implemented.
-  // @gate !enableUseRefAccessWarning || !__DEV__
   test(
     "compares to current state before bailing out, even when there's a " +
       'mutation in between the sync and passive effects',
@@ -284,9 +272,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     },
   );
 
-  // 18 has an experimental flag to warn about reading refs. Will circumvent
-  // when built-in API is implemented.
-  // @gate !enableUseRefAccessWarning || !__DEV__
   test('mutating the store in between render and commit when getSnapshot has changed', () => {
     const store = createExternalStore({a: 1, b: 1});
 
@@ -345,9 +330,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     expect(root).toMatchRenderedOutput('B2');
   });
 
-  // 18 has an experimental flag to warn about reading refs. Will circumvent
-  // when built-in API is implemented.
-  // @gate !enableUseRefAccessWarning || !__DEV__
   test('mutating the store in between render and commit when getSnapshot has _not_ changed', () => {
     // Same as previous test, but `getSnapshot` does not change
     const store = createExternalStore({a: 1, b: 1});
@@ -405,9 +387,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     expect(root).toMatchRenderedOutput('A1');
   });
 
-  // 18 has an experimental flag to warn about reading refs. Will circumvent
-  // when built-in API is implemented.
-  // @gate !enableUseRefAccessWarning || !__DEV__
   test("does not bail out if the previous update hasn't finished yet", () => {
     const store = createExternalStore(0);
 
@@ -443,9 +422,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     expect(root).toMatchRenderedOutput('00');
   });
 
-  // 18 has an experimental flag to warn about reading refs. Will circumvent
-  // when built-in API is implemented.
-  // @gate !enableUseRefAccessWarning || !__DEV__
   test('uses the latest getSnapshot, even if it changed in the same batch as a store update', () => {
     const store = createExternalStore({a: 0, b: 0});
 
@@ -473,9 +449,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     expect(root).toMatchRenderedOutput('2');
   });
 
-  // 18 has an experimental flag to warn about reading refs. Will circumvent
-  // when built-in API is implemented.
-  // @gate !enableUseRefAccessWarning || !__DEV__
   test('handles errors thrown by getSnapshot or isEqual', () => {
     class ErrorBoundary extends React.Component {
       state = {error: null};
@@ -552,9 +525,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
   });
 
   describe('extra features implemented in user-space', () => {
-    // 18 has an experimental flag to warn about reading refs. Will circumvent
-    // when built-in API is implemented.
-    // @gate !enableUseRefAccessWarning || !__DEV__
     test('memoized selectors are only called once per update', () => {
       const store = createExternalStore({a: 0, b: 0});
 
@@ -593,9 +563,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
       expect(root).toMatchRenderedOutput('A1');
     });
 
-    // 18 has an experimental flag to warn about reading refs. Will circumvent
-    // when built-in API is implemented.
-    // @gate !enableUseRefAccessWarning || !__DEV__
     test('Using isEqual to bailout', () => {
       const store = createExternalStore({a: 0, b: 0});
 

packages/use-sync-external-store/src/useSyncExternalStore.js

@@ -14,7 +14,6 @@ import is from 'shared/objectIs';
 // dispatch for CommonJS interop named imports.
 const {
   useState,
-  useRef,
   useEffect,
   useLayoutEffect,
   useDebugValue,
@@ -59,34 +58,27 @@ function useSyncExternalStore_shim<T>(
     }
   }
 
-  // Because updates are synchronous, we don't queue them. Instead we force a
-  // re-render whenever the subscribed state changes, and read the current state
-  // directly in render.
-  //
-  // We force a re-render by bumping a version number.
-  const [, setVersion] = useState(0);
-
   // 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();
 
+  // 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
-  // ref object. Initialize this object on the first render.
-  const refs = useRef(null);
-  let inst;
-  if (refs.current === null) {
-    inst = {
-      // This represents the currently rendered value and getSnapshot function.
-      // We update them with a ref whenever they change.
-      value,
-      getSnapshot,
-    };
-    refs.current = inst;
-  } else {
-    inst = refs.current;
-  }
+  // object. Usually, we would put that with a useRef hook, but we can stash
+  // that 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] = useState({inst: {value, getSnapshot}});
 
   // Track the latest getSnapshot function with a ref. This needs to be updated
   // in the layout phase so that we can access it during the tearing check that
@@ -102,7 +94,7 @@ function useSyncExternalStore_shim<T>(
     // effect may have mutated the store.
     if (checkIfSnapshotChanged(inst)) {
       // Force a re-render.
-      setVersion(bumpVersion);
+      forceUpdate({inst});
     }
   }, [subscribe, value, getSnapshot]);
 
@@ -111,7 +103,7 @@ function useSyncExternalStore_shim<T>(
     // detected in the subscription handler.
     if (checkIfSnapshotChanged(inst)) {
       // Force a re-render.
-      setVersion(bumpVersion);
+      forceUpdate({inst});
     }
     const handleStoreChange = () => {
       // TODO: Because there is no cross-renderer API for batching updates, it's
@@ -123,7 +115,7 @@ function useSyncExternalStore_shim<T>(
       // read from the store.
       if (checkIfSnapshotChanged(inst)) {
         // Force a re-render.
-        setVersion(bumpVersion);
+        forceUpdate({inst});
       }
     };
     // Subscribe to the store and return a clean-up function.
@@ -144,7 +136,3 @@ function checkIfSnapshotChanged(inst) {
     return true;
   }
 }
-
-function bumpVersion(v) {
-  return v + 1;
-}