Call get snapshot in useSyncExternalStore server shim (facebook#22453)

* Call getSnapshot in shim * just change useSyncExternalStoreServer * remove builtInAPI Check in useSyncExternalStoreClient
zhengjitf · Apr 15, 2022 · 95bf549 · 95bf549
1 parent 32be70d
commit 95bf549
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 30 deletions.
diff --git a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShimServer-test.js b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShimServer-test.js
@@ -92,7 +92,9 @@ describe('useSyncExternalStore (userspace shim, server rendering)', () => {
     }
 
     const html = ReactDOMServer.renderToString(<App />);
-    expect(Scheduler).toHaveYielded(['server']);
-    expect(html).toEqual('server');
+
+    // We don't call getServerSnapshot in the shim
+    expect(Scheduler).toHaveYielded(['client']);
+    expect(html).toEqual('client');
   });
 });
diff --git a/packages/use-sync-external-store/src/useSyncExternalStore.js b/packages/use-sync-external-store/src/useSyncExternalStore.js
@@ -10,5 +10,13 @@
 import {canUseDOM} from 'shared/ExecutionEnvironment';
 import {useSyncExternalStore as client} from './useSyncExternalStoreClient';
 import {useSyncExternalStore as server} from './useSyncExternalStoreServer';
+import * as React from 'react';
 
-export const useSyncExternalStore = canUseDOM ? client : server;
+const {unstable_useSyncExternalStore: builtInAPI} = React;
+
+export const useSyncExternalStore =
+  builtInAPI !== undefined
+    ? ((builtInAPI: any): typeof client)
+    : canUseDOM
+    ? client
+    : server;
diff --git a/packages/use-sync-external-store/src/useSyncExternalStoreClient.js b/packages/use-sync-external-store/src/useSyncExternalStoreClient.js
@@ -12,22 +12,7 @@ import is from 'shared/objectIs';
 
 // Intentionally not using named imports because Rollup uses dynamic
 // dispatch for CommonJS interop named imports.
-const {
-  useState,
-  useEffect,
-  useLayoutEffect,
-  useDebugValue,
-  // The built-in API is still prefixed.
-  unstable_useSyncExternalStore: builtInAPI,
-} = React;
-
-// Prefer the built-in API, if it exists. If it doesn't exist, then we assume
-// we're in version 16 or 17, so rendering is always synchronous. The shim
-// does not support concurrent rendering, only the built-in API.
-export const useSyncExternalStore =
-  builtInAPI !== undefined
-    ? ((builtInAPI: any): typeof useSyncExternalStore_client)
-    : useSyncExternalStore_client;
+const {useState, useEffect, useLayoutEffect, useDebugValue} = React;
 
 let didWarnOld18Alpha = false;
 let didWarnUncachedGetSnapshot = false;
@@ -42,7 +27,7 @@ let didWarnUncachedGetSnapshot = false;
 //
 // Do not assume that the clever hacks used by this hook also work in general.
 // The point of this shim is to replace the need for hacks by other libraries.
-function useSyncExternalStore_client<T>(
+export function useSyncExternalStore<T>(
   subscribe: (() => void) => () => void,
   getSnapshot: () => T,
   // Note: The client shim does not use getServerSnapshot, because pre-18

diff --git a/packages/use-sync-external-store/src/useSyncExternalStoreServer.js b/packages/use-sync-external-store/src/useSyncExternalStoreServer.js
@@ -7,19 +7,10 @@
  * @flow
  */
 
-import invariant from 'shared/invariant';
-
 export function useSyncExternalStore<T>(
   subscribe: (() => void) => () => void,
   getSnapshot: () => T,
   getServerSnapshot?: () => T,
 ): T {
-  if (getServerSnapshot === undefined) {
-    invariant(
-      false,
-      'Missing getServerSnapshot, which is required for server-' +
-        'rendered content.',
-    );
-  }
-  return getServerSnapshot();
+  return getSnapshot();
 }