diff --git a/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js b/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js index 291a9a4e30d39..de547c76e7385 100644 --- a/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js +++ b/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js @@ -1231,7 +1231,7 @@ describe('preprocessData', () => { testMarks.push(...createUserTimingData(clearedMarks)); - const data = await preprocessData(testMarks); + const data = await act(() => preprocessData(testMarks)); expect(data.suspenseEvents).toHaveLength(1); expect(data.suspenseEvents[0].promiseName).toBe('Testing displayName'); } @@ -1682,7 +1682,7 @@ describe('preprocessData', () => { testMarks.push(...createUserTimingData(clearedMarks)); - const data = await preprocessData(testMarks); + const data = await act(() => preprocessData(testMarks)); expect(data.suspenseEvents).toHaveLength(1); expect(data.suspenseEvents[0].warning).toMatchInlineSnapshot( `"A component suspended during an update which caused a fallback to be shown. Consider using the Transition API to avoid hiding components after they've been mounted."`, @@ -1740,7 +1740,7 @@ describe('preprocessData', () => { testMarks.push(...createUserTimingData(clearedMarks)); - const data = await preprocessData(testMarks); + const data = await act(() => preprocessData(testMarks)); expect(data.suspenseEvents).toHaveLength(1); expect(data.suspenseEvents[0].warning).toBe(null); } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 58df2e687db5b..7aaef5a37959f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -2368,6 +2368,8 @@ export function pingSuspendedRoot( const eventTime = requestEventTime(); markRootPinged(root, pingedLanes, eventTime); + warnIfSuspenseResolutionNotWrappedWithActDEV(root); + if ( workInProgressRoot === root && isSubsetOfLanes(workInProgressRootRenderLanes, pingedLanes) @@ -2902,3 +2904,27 @@ function warnIfUpdatesNotWrappedWithActDEV(fiber: Fiber): void { } } } + +function warnIfSuspenseResolutionNotWrappedWithActDEV(root: FiberRoot): void { + if (__DEV__) { + if ( + root.tag !== LegacyRoot && + isConcurrentActEnvironment() && + ReactCurrentActQueue.current === null + ) { + console.error( + 'A suspended resource finished loading inside a test, but the event ' + + 'was not wrapped in act(...).\n\n' + + 'When testing, code that resolves suspended data should be wrapped ' + + 'into act(...):\n\n' + + 'act(() => {\n' + + ' /* finish loading suspended data */\n' + + '});\n' + + '/* assert on the output */\n\n' + + "This ensures that you're testing the behavior the user would see " + + 'in the browser.' + + ' Learn more at https://reactjs.org/link/wrap-tests-with-act', + ); + } + } +} diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index bb7e34bee42a1..07eb097d6ffe4 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -2368,6 +2368,8 @@ export function pingSuspendedRoot( const eventTime = requestEventTime(); markRootPinged(root, pingedLanes, eventTime); + warnIfSuspenseResolutionNotWrappedWithActDEV(root); + if ( workInProgressRoot === root && isSubsetOfLanes(workInProgressRootRenderLanes, pingedLanes) @@ -2902,3 +2904,27 @@ function warnIfUpdatesNotWrappedWithActDEV(fiber: Fiber): void { } } } + +function warnIfSuspenseResolutionNotWrappedWithActDEV(root: FiberRoot): void { + if (__DEV__) { + if ( + root.tag !== LegacyRoot && + isConcurrentActEnvironment() && + ReactCurrentActQueue.current === null + ) { + console.error( + 'A suspended resource finished loading inside a test, but the event ' + + 'was not wrapped in act(...).\n\n' + + 'When testing, code that resolves suspended data should be wrapped ' + + 'into act(...):\n\n' + + 'act(() => {\n' + + ' /* finish loading suspended data */\n' + + '});\n' + + '/* assert on the output */\n\n' + + "This ensures that you're testing the behavior the user would see " + + 'in the browser.' + + ' Learn more at https://reactjs.org/link/wrap-tests-with-act', + ); + } + } +} diff --git a/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js b/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js index 1fc035b853078..8be5b0bb9720e 100644 --- a/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js +++ b/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js @@ -140,9 +140,20 @@ describe('DebugTracing', () => { // @gate experimental && build === 'development' && enableDebugTracing it('should log concurrent render with suspense', async () => { - const fakeSuspensePromise = Promise.resolve(true); + let isResolved = false; + let resolveFakeSuspensePromise; + const fakeSuspensePromise = new Promise(resolve => { + resolveFakeSuspensePromise = () => { + resolve(); + isResolved = true; + }; + }); + function Example() { - throw fakeSuspensePromise; + if (!isResolved) { + throw fakeSuspensePromise; + } + return null; } ReactTestRenderer.act(() => @@ -164,7 +175,7 @@ describe('DebugTracing', () => { logs.splice(0); - await fakeSuspensePromise; + await ReactTestRenderer.act(async () => await resolveFakeSuspensePromise()); expect(logs).toEqual(['log: ⚛️ Example resolved']); }); diff --git a/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js b/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js index 99ec72c9fc8eb..324d153273361 100644 --- a/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js +++ b/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js @@ -12,6 +12,10 @@ let Scheduler; let ReactNoop; let useState; let act; +let Suspense; +let startTransition; +let getCacheForType; +let caches; // These tests are mostly concerned with concurrent roots. The legacy root // behavior is covered by other older test suites and is unchanged from @@ -24,11 +28,110 @@ describe('act warnings', () => { ReactNoop = require('react-noop-renderer'); act = React.unstable_act; useState = React.useState; + Suspense = React.Suspense; + startTransition = React.startTransition; + getCacheForType = React.unstable_getCacheForType; + caches = []; }); - function Text(props) { - Scheduler.unstable_yieldValue(props.text); - return props.text; + function createTextCache() { + const data = new Map(); + const version = caches.length + 1; + const cache = { + version, + data, + resolve(text) { + const record = data.get(text); + if (record === undefined) { + const newRecord = { + status: 'resolved', + value: text, + }; + data.set(text, newRecord); + } else if (record.status === 'pending') { + const thenable = record.value; + record.status = 'resolved'; + record.value = text; + thenable.pings.forEach(t => t()); + } + }, + reject(text, error) { + const record = data.get(text); + if (record === undefined) { + const newRecord = { + status: 'rejected', + value: error, + }; + data.set(text, newRecord); + } else if (record.status === 'pending') { + const thenable = record.value; + record.status = 'rejected'; + record.value = error; + thenable.pings.forEach(t => t()); + } + }, + }; + caches.push(cache); + return cache; + } + + function readText(text) { + const textCache = getCacheForType(createTextCache); + const record = textCache.data.get(text); + if (record !== undefined) { + switch (record.status) { + case 'pending': + Scheduler.unstable_yieldValue(`Suspend! [${text}]`); + throw record.value; + case 'rejected': + Scheduler.unstable_yieldValue(`Error! [${text}]`); + throw record.value; + case 'resolved': + return textCache.version; + } + } else { + Scheduler.unstable_yieldValue(`Suspend! [${text}]`); + + const thenable = { + pings: [], + then(resolve) { + if (newRecord.status === 'pending') { + thenable.pings.push(resolve); + } else { + Promise.resolve().then(() => resolve(newRecord.value)); + } + }, + }; + + const newRecord = { + status: 'pending', + value: thenable, + }; + textCache.data.set(text, newRecord); + + throw thenable; + } + } + + function Text({text}) { + Scheduler.unstable_yieldValue(text); + return text; + } + + function AsyncText({text}) { + readText(text); + Scheduler.unstable_yieldValue(text); + return text; + } + + function resolveText(text) { + if (caches.length === 0) { + throw Error('Cache does not exist.'); + } else { + // Resolve the most recently created cache. An older cache can by + // resolved with `caches[index].resolve(text)`. + caches[caches.length - 1].resolve(text); + } } function withActEnvironment(value, scope) { @@ -187,4 +290,72 @@ describe('act warnings', () => { expect(root).toMatchRenderedOutput('1'); }); }); + + // @gate __DEV__ + // @gate enableCache + test('warns if Suspense retry is not wrapped', () => { + function App() { + return ( + }> + + + ); + } + + withActEnvironment(true, () => { + const root = ReactNoop.createRoot(); + act(() => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['Suspend! [Async]', 'Loading...']); + expect(root).toMatchRenderedOutput('Loading...'); + + // This is a retry, not a ping, because we already showed a fallback. + expect(() => + resolveText('Async'), + ).toErrorDev( + 'A suspended resource finished loading inside a test, but the event ' + + 'was not wrapped in act(...)', + {withoutStack: true}, + ); + }); + }); + + // @gate __DEV__ + // @gate enableCache + test('warns if Suspense ping is not wrapped', () => { + function App({showMore}) { + return ( + }> + {showMore ? : } + + ); + } + + withActEnvironment(true, () => { + const root = ReactNoop.createRoot(); + act(() => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['(empty)']); + expect(root).toMatchRenderedOutput('(empty)'); + + act(() => { + startTransition(() => { + root.render(); + }); + }); + expect(Scheduler).toHaveYielded(['Suspend! [Async]', 'Loading...']); + expect(root).toMatchRenderedOutput('(empty)'); + + // This is a ping, not a retry, because no fallback is showing. + expect(() => + resolveText('Async'), + ).toErrorDev( + 'A suspended resource finished loading inside a test, but the event ' + + 'was not wrapped in act(...)', + {withoutStack: true}, + ); + }); + }); });