From 6d60db8b10997ffec3813af9560f37bb75b7549a Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sun, 5 Mar 2023 18:33:31 -0500 Subject: [PATCH] Regression test: Bad `shouldYield` causes `act` to hang Based on a bug report from @bvaughn. `act` should not consult `shouldYield` when it's performing work, because in a unit testing environment, I/O (such as `setTimeout`) is likely mocked. So the result of `shouldYield` can't be trusted. In this regression test, I simulate the bug by mocking `shouldYield` to always return `true`. This causes an infinite loop in `act`, because it will keep trying to render and React will keep yielding. I will fix the bug in the next commit by ignoring `shouldYield` whenever we're inside an `act` scope. --- .../ReactSchedulerIntegration-test.js | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js index 1dffc1be85839..2a89bbfa8f9d6 100644 --- a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js @@ -320,3 +320,70 @@ describe( }); }, ); + +describe('`act` bypasses Scheduler methods completely,', () => { + let infiniteLoopGuard; + + beforeEach(() => { + jest.resetModules(); + + infiniteLoopGuard = 0; + + jest.mock('scheduler', () => { + const actual = jest.requireActual('scheduler/unstable_mock'); + return { + ...actual, + unstable_shouldYield() { + // This simulates a bug report where `shouldYield` returns true in a + // unit testing environment. Because `act` will keep working until + // there's no more work left, it would fall into an infinite loop. + // The fix is that when performing work inside `act`, we should bypass + // `shouldYield` completely, because we can't trust it to be correct. + if (infiniteLoopGuard++ > 100) { + throw new Error('Detected an infinte loop'); + } + return true; + }, + }; + }); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + startTransition = React.startTransition; + }); + + afterEach(() => { + jest.mock('scheduler', () => jest.requireActual('scheduler/unstable_mock')); + }); + + it('inside `act`, does not call `shouldYield`, even during a concurrent render', async () => { + function App() { + return ( + <> +
A
+
B
+
C
+ + ); + } + + const root = ReactNoop.createRoot(); + const publicAct = React.unstable_act; + const prevIsReactActEnvironment = global.IS_REACT_ACT_ENVIRONMENT; + try { + global.IS_REACT_ACT_ENVIRONMENT = true; + await publicAct(async () => { + startTransition(() => root.render()); + }); + } finally { + global.IS_REACT_ACT_ENVIRONMENT = prevIsReactActEnvironment; + } + expect(root).toMatchRenderedOutput( + <> +
A
+
B
+
C
+ , + ); + }); +});