Avoid acccessing React internals from use-sync-external-store/shim (#29868)

Co-authored-by: eps1lon <[email protected]>

Author: phryneas

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

@@ -20,7 +20,7 @@ let useState;
 let useEffect;
 let useLayoutEffect;
 let assertLog;
-let originalError;
+let assertConsoleErrorDev;
 
 // This tests shared behavior between the built-in and shim implementations of
 // of useSyncExternalStore.
@@ -50,9 +50,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
             : 'react-dom-17/umd/react-dom.production.min.js',
         ),
       );
-      // Because React 17 prints extra logs we need to ignore them.
-      originalError = console.error;
-      console.error = jest.fn();
     }
     React = require('react');
     ReactDOM = require('react-dom');
@@ -63,6 +60,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     useLayoutEffect = React.useLayoutEffect;
     const InternalTestUtils = require('internal-test-utils');
     assertLog = InternalTestUtils.assertLog;
+    assertConsoleErrorDev = InternalTestUtils.assertConsoleErrorDev;
     const internalAct = require('internal-test-utils').act;
 
     // The internal act implementation doesn't batch updates by default, since
@@ -85,11 +83,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     useSyncExternalStoreWithSelector =
       require('use-sync-external-store/shim/with-selector').useSyncExternalStoreWithSelector;
   });
-  afterEach(() => {
-    if (gate(flags => flags.enableUseSyncExternalStoreShim)) {
-      console.error = originalError;
-    }
-  });
   function Text({text}) {
     Scheduler.log(text);
     return text;
@@ -630,36 +623,30 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
     const container = document.createElement('div');
     const root = createRoot(container);
     await expect(async () => {
-      await expect(async () => {
-        await act(() => {
-          ReactDOM.flushSync(async () =>
-            root.render(React.createElement(App, null)),
-          );
-        });
-      }).rejects.toThrow(
-        'Maximum update depth exceeded. This can happen when a component repeatedly ' +
-          'calls setState inside componentWillUpdate or componentDidUpdate. React limits ' +
-          'the number of nested updates to prevent infinite loops.',
-      );
-    }).toErrorDev(
+      await act(() => {
+        ReactDOM.flushSync(async () =>
+          root.render(React.createElement(App, null)),
+        );
+      });
+    }).rejects.toThrow(
+      'Maximum update depth exceeded. This can happen when a component repeatedly ' +
+        'calls setState inside componentWillUpdate or componentDidUpdate. React limits ' +
+        'the number of nested updates to prevent infinite loops.',
+    );
+
+    assertConsoleErrorDev(
       gate(flags => flags.enableUseSyncExternalStoreShim)
         ? [
-            'Maximum update depth exceeded. ',
-            'The result of getSnapshot should be cached to avoid an infinite loop',
-            'The above error occurred in the',
+            [
+              'The result of getSnapshot should be cached to avoid an infinite loop',
+              {withoutStack: true},
+            ],
+            'Error: Maximum update depth exceeded',
+            'The above error occurred i',
           ]
         : [
             'The result of getSnapshot should be cached to avoid an infinite loop',
           ],
-      {
-        withoutStack: gate(flags => {
-          if (flags.enableUseSyncExternalStoreShim) {
-            // Stacks don't work when mixing the source and the npm package.
-            return flags.source ? 1 : 0;
-          }
-          return false;
-        }),
-      },
     );
   });
   it('getSnapshot can return NaN without infinite loop warning', async () => {
@@ -850,10 +837,9 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
         // client. To avoid this server mismatch warning, user must account for
         // this themselves and return the correct value inside `getSnapshot`.
         await act(() => {
-          expect(() =>
-            ReactDOM.hydrate(React.createElement(App, null), container),
-          ).toErrorDev('Text content did not match');
+          ReactDOM.hydrate(React.createElement(App, null), container);
         });
+        assertConsoleErrorDev(['Text content did not match']);
         assertLog(['client', 'Passive effect: client']);
       }
       expect(container.textContent).toEqual('client');

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

@@ -16,7 +16,10 @@ import * as React from 'react';
 export const useSyncExternalStore = React.useSyncExternalStore;
 
 if (__DEV__) {
-  console.error(
+  // Avoid transforming the `console.error` call as it would cause the built artifact
+  // to access React internals, which exist under different paths depending on the
+  // React version.
+  console['error'](
     "The main 'use-sync-external-store' entry point is not supported; all it " +
       "does is re-export useSyncExternalStore from the 'react' package, so " +
       'it only works with React 18+.' +

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

@@ -40,7 +40,10 @@ export function useSyncExternalStore<T>(
     if (!didWarnOld18Alpha) {
       if (React.startTransition !== undefined) {
         didWarnOld18Alpha = true;
-        console.error(
+        // Avoid transforming the `console.error` call as it would cause the built artifact
+        // to access React internals, which exist under different paths depending on the
+        // React version.
+        console['error'](
           'You are using an outdated, pre-release alpha of React 18 that ' +
             'does not support useSyncExternalStore. The ' +
             'use-sync-external-store shim will not work correctly. Upgrade ' +
@@ -59,7 +62,10 @@ export function useSyncExternalStore<T>(
     if (!didWarnUncachedGetSnapshot) {
       const cachedValue = getSnapshot();
       if (!is(value, cachedValue)) {
-        console.error(
+        // Avoid transforming the `console.error` call as it would cause the built artifact
+        // to access React internals, which exist under different paths depending on the
+        // React version.
+        console['error'](
           'The result of getSnapshot should be cached to avoid an infinite loop',
         );
         didWarnUncachedGetSnapshot = true;

scripts/jest/TestFlags.js

@@ -95,13 +95,8 @@ function getTestFlags() {
 
       // This is used by useSyncExternalStoresShared-test.js to decide whether
       // to test the shim or the native implementation of useSES.
-      // TODO: It's disabled when enableRefAsProp is on because the JSX
-      // runtime used by our tests is not compatible with older versions of
-      // React. If we want to keep testing this shim after enableRefIsProp is
-      // on everywhere, we'll need to find some other workaround. Maybe by
-      // only using createElement instead of JSX in that test module.
-      enableUseSyncExternalStoreShim:
-        !__VARIANT__ && !featureFlags.enableRefAsProp,
+
+      enableUseSyncExternalStoreShim: !__VARIANT__,
 
       // If there's a naming conflict between scheduler and React feature flags, the
       // React ones take precedence.