[wip] Update current value with a ref

Originally we updated it in the subscription handler so that we could
be sure it always represented the latest snapshot. However, we run into
problems because the `getSnapshot` could change the next time in the
same batch as the re-render, in which case the value is no longer
correct. So this moves it back to the commit phase.

We avoid the stale `getSnapshot` problem with a tearing check in the
commit phase whenever `getSnapshot` changes. Since `getSnapshot` is
designed to be fast, I combined both into a single `useLayoutEffect`,
along with the one we do when `subscribe` changes, to save some memory.

Author: acdlite

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

@@ -78,48 +78,33 @@ function useSyncExternalStore_shim<T>(
   let inst;
   if (refs.current === null) {
     inst = {
-      // This represents the last value that we read from the external store,
-      // not the last value that we rendered. We can use it to eagerly bail out,
-      // regardless of the current rendered state.
+      // This represents the currently rendered value and getSnapshot function.
+      // We update them with a ref whenever they change.
       value,
-
-      // This is the currently rendered getSnapshot functions. We track them
-      // with a commit effect. However, it's possible we might receive a store
-      // update in between render and when the commit effect runs. Refer to the
-      // next branch to see how we handle that.
       getSnapshot,
     };
     refs.current = inst;
   } else {
     inst = refs.current;
-
-    // Every time we read from the store, we must update the latest value.
-    inst.value = value;
   }
 
   // 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
   // happens on subscribe.
   // TODO: Circumvent SSR warning
   useLayoutEffect(() => {
+    inst.value = value;
     inst.getSnapshot = getSnapshot;
-    if (checkIfSnapshotChanged(inst)) {
-      // Force a re-render.
-      setVersion(bumpVersion);
-    }
-  }, [getSnapshot]);
 
-  // We re-subscribe in a passive effect whenever a new subscribe function is
-  // passed. Before we allow the browser to paint, though, we should check to
-  // confirm that the value we read during render is consistent, in case there
-  // were additional mutations since then.
-  // TODO: Circumvent SSR warning
-  useLayoutEffect(() => {
+    // 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.
       setVersion(bumpVersion);
     }
-  }, [subscribe]);
+  }, [subscribe, value, getSnapshot]);
 
   useEffect(() => {
     // Check for changes right before subscribing. Subsequent changes will be
@@ -154,13 +139,7 @@ function checkIfSnapshotChanged(inst) {
   const prevValue = inst.value;
   try {
     const nextValue = latestGetSnapshot();
-    if (is(prevValue, nextValue)) {
-      return false;
-    } else {
-      // Force a re-render.
-      inst.value = nextValue;
-      return true;
-    }
+    return !is(prevValue, nextValue);
   } catch (error) {
     return true;
   }