Avoid acccessing React internals from `use-sync-external-store/shim` (#29868)
Co-authored-by: eps1lon <sebastian.silbermann@vercel.com>
Lenz Weber-Tronic committed
Jun 12, 2024 at 20:13 UTC
50e89ec9f2d44ab140e771e585226f7771da9652
4 files changed
+36
-46
packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js
+22
-36
@@ -20,7 +20,7 @@ let useState;
20
let useEffect;
21
let useLayoutEffect;
22
let assertLog;
23
-let originalError;
23
+let assertConsoleErrorDev;
24
25
// This tests shared behavior between the built-in and shim implementations of
26
// of useSyncExternalStore.
@@ -50,9 +50,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
50
: 'react-dom-17/umd/react-dom.production.min.js',
51
),
52
);
53
- // Because React 17 prints extra logs we need to ignore them.
54
- originalError = console.error;
55
- console.error = jest.fn();
53
}
54
React = require('react');
55
ReactDOM = require('react-dom');
@@ -63,6 +60,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
60
useLayoutEffect = React.useLayoutEffect;
61
const InternalTestUtils = require('internal-test-utils');
62
assertLog = InternalTestUtils.assertLog;
63
+ assertConsoleErrorDev = InternalTestUtils.assertConsoleErrorDev;
64
const internalAct = require('internal-test-utils').act;
65
66
// The internal act implementation doesn't batch updates by default, since
@@ -85,11 +83,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
83
useSyncExternalStoreWithSelector =
84
require('use-sync-external-store/shim/with-selector').useSyncExternalStoreWithSelector;
85
});
88
- afterEach(() => {
89
- if (gate(flags => flags.enableUseSyncExternalStoreShim)) {
90
- console.error = originalError;
91
- }
92
- });
86
function Text({text}) {
87
Scheduler.log(text);
88
return text;
@@ -630,36 +623,30 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
623
const container = document.createElement('div');
624
const root = createRoot(container);
625
await expect(async () => {
633
- await expect(async () => {
634
- await act(() => {
635
- ReactDOM.flushSync(async () =>
636
- root.render(React.createElement(App, null)),
637
- );
638
- });
639
- }).rejects.toThrow(
640
- 'Maximum update depth exceeded. This can happen when a component repeatedly ' +
641
- 'calls setState inside componentWillUpdate or componentDidUpdate. React limits ' +
642
- 'the number of nested updates to prevent infinite loops.',
643
- );
644
- }).toErrorDev(
626
+ await act(() => {
627
+ ReactDOM.flushSync(async () =>
628
+ root.render(React.createElement(App, null)),
629
+ );
630
+ });
631
+ }).rejects.toThrow(
632
+ 'Maximum update depth exceeded. This can happen when a component repeatedly ' +
633
+ 'calls setState inside componentWillUpdate or componentDidUpdate. React limits ' +
634
+ 'the number of nested updates to prevent infinite loops.',
635
+ );
636
+
637
+ assertConsoleErrorDev(
638
gate(flags => flags.enableUseSyncExternalStoreShim)
639
? [
647
- 'Maximum update depth exceeded. ',
648
- 'The result of getSnapshot should be cached to avoid an infinite loop',
649
- 'The above error occurred in the',
640
+ [
641
+ 'The result of getSnapshot should be cached to avoid an infinite loop',
642
+ {withoutStack: true},
643
+ ],
644
+ 'Error: Maximum update depth exceeded',
645
+ 'The above error occurred i',
646
]
647
: [
648
'The result of getSnapshot should be cached to avoid an infinite loop',
649
],
654
- {
655
- withoutStack: gate(flags => {
656
- if (flags.enableUseSyncExternalStoreShim) {
657
- // Stacks don't work when mixing the source and the npm package.
658
- return flags.source ? 1 : 0;
659
- }
660
- return false;
661
- }),
662
- },
650
);
651
});
652
it('getSnapshot can return NaN without infinite loop warning', async () => {
@@ -850,10 +837,9 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
837
// client. To avoid this server mismatch warning, user must account for
838
// this themselves and return the correct value inside `getSnapshot`.
839
await act(() => {
853
- expect(() =>
854
- ReactDOM.hydrate(React.createElement(App, null), container),
855
- ).toErrorDev('Text content did not match');
840
+ ReactDOM.hydrate(React.createElement(App, null), container);
841
});
842
+ assertConsoleErrorDev(['Text content did not match']);
843
assertLog(['client', 'Passive effect: client']);
844
}
845
expect(container.textContent).toEqual('client');
packages/use-sync-external-store/src/useSyncExternalStore.js
+4
-1
@@ -16,7 +16,10 @@ import * as React from 'react';
16
export const useSyncExternalStore = React.useSyncExternalStore;
17
18
if (__DEV__) {
19
- console.error(
19
+ // Avoid transforming the `console.error` call as it would cause the built artifact
20
+ // to access React internals, which exist under different paths depending on the
21
+ // React version.
22
+ console['error'](
23
"The main 'use-sync-external-store' entry point is not supported; all it " +
24
"does is re-export useSyncExternalStore from the 'react' package, so " +
25
'it only works with React 18+.' +
packages/use-sync-external-store/src/useSyncExternalStoreShimClient.js
+8
-2
@@ -40,7 +40,10 @@ export function useSyncExternalStore<T>(
40
if (!didWarnOld18Alpha) {
41
if (React.startTransition !== undefined) {
42
didWarnOld18Alpha = true;
43
- console.error(
43
+ // Avoid transforming the `console.error` call as it would cause the built artifact
44
+ // to access React internals, which exist under different paths depending on the
45
+ // React version.
46
+ console['error'](
47
'You are using an outdated, pre-release alpha of React 18 that ' +
48
'does not support useSyncExternalStore. The ' +
49
'use-sync-external-store shim will not work correctly. Upgrade ' +
@@ -59,7 +62,10 @@ export function useSyncExternalStore<T>(
62
if (!didWarnUncachedGetSnapshot) {
63
const cachedValue = getSnapshot();
64
if (!is(value, cachedValue)) {
62
- console.error(
65
+ // Avoid transforming the `console.error` call as it would cause the built artifact
66
+ // to access React internals, which exist under different paths depending on the
67
+ // React version.
68
+ console['error'](
69
'The result of getSnapshot should be cached to avoid an infinite loop',
70
);
71
didWarnUncachedGetSnapshot = true;
scripts/jest/TestFlags.js
+2
-7
@@ -95,13 +95,8 @@ function getTestFlags() {
95
96
// This is used by useSyncExternalStoresShared-test.js to decide whether
97
// to test the shim or the native implementation of useSES.
98
- // TODO: It's disabled when enableRefAsProp is on because the JSX
99
- // runtime used by our tests is not compatible with older versions of
100
- // React. If we want to keep testing this shim after enableRefIsProp is
101
- // on everywhere, we'll need to find some other workaround. Maybe by
102
- // only using createElement instead of JSX in that test module.
103
- enableUseSyncExternalStoreShim:
104
- !__VARIANT__ && !featureFlags.enableRefAsProp,
98
+
99
+ enableUseSyncExternalStoreShim: !__VARIANT__,
100
101
// If there's a naming conflict between scheduler and React feature flags, the
102
// React ones take precedence.