From 7b6e17c8bba0a10f97f443bb36073595bd9d1240 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Mon, 17 Apr 2023 16:24:30 -0400 Subject: [PATCH] Revert "act: Move didScheduleLegacyUpdate to ensureRootIsScheduled (#26552)" This reverts commit fec97ecbc4bc2e0e1407160289a8f5fac5241cbc. --- .../src/ReactFiberRootScheduler.js | 9 --- .../src/ReactFiberWorkLoop.js | 1 + .../src/__tests__/ReactIsomorphicAct-test.js | 79 +++---------------- scripts/jest/matchers/reactTestMatchers.js | 6 +- 4 files changed, 15 insertions(+), 80 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index db60ab8be5360..be14c4695089c 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -119,15 +119,6 @@ export function ensureRootIsScheduled(root: FiberRoot): void { // unblock additional features we have planned. scheduleTaskForRootDuringMicrotask(root, now()); } - - if ( - __DEV__ && - ReactCurrentActQueue.isBatchingLegacy && - root.tag === LegacyRoot - ) { - // Special `act` case: Record whenever a legacy update is scheduled. - ReactCurrentActQueue.didScheduleLegacyUpdate = true; - } } export function flushSyncWorkOnAllRoots() { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 401defb280fe4..1715eab466c9d 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -828,6 +828,7 @@ export function scheduleUpdateOnFiber( ) { if (__DEV__ && ReactCurrentActQueue.isBatchingLegacy) { // Treat `act` as if it's inside `batchedUpdates`, even in legacy mode. + ReactCurrentActQueue.didScheduleLegacyUpdate = true; } else { // Flush the synchronous work now, unless we're already working or inside // a batch. This is intentionally inside scheduleUpdateOnFiber instead of diff --git a/packages/react-reconciler/src/__tests__/ReactIsomorphicAct-test.js b/packages/react-reconciler/src/__tests__/ReactIsomorphicAct-test.js index 8615d4ab2667e..e64a208c54dc1 100644 --- a/packages/react-reconciler/src/__tests__/ReactIsomorphicAct-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIsomorphicAct-test.js @@ -17,13 +17,10 @@ let Suspense; let DiscreteEventPriority; let startTransition; let waitForMicrotasks; -let Scheduler; -let assertLog; describe('isomorphic act()', () => { beforeEach(() => { React = require('react'); - Scheduler = require('scheduler'); ReactNoop = require('react-noop-renderer'); DiscreteEventPriority = @@ -34,7 +31,6 @@ describe('isomorphic act()', () => { startTransition = React.startTransition; waitForMicrotasks = require('internal-test-utils').waitForMicrotasks; - assertLog = require('internal-test-utils').assertLog; }); beforeEach(() => { @@ -45,11 +41,6 @@ describe('isomorphic act()', () => { jest.restoreAllMocks(); }); - function Text({text}) { - Scheduler.log(text); - return text; - } - // @gate __DEV__ test('bypasses queueMicrotask', async () => { const root = ReactNoop.createRoot(); @@ -141,67 +132,19 @@ describe('isomorphic act()', () => { const root = ReactNoop.createLegacyRoot(); await act(async () => { - queueMicrotask(() => { - Scheduler.log('Current tree in microtask: ' + root.getChildrenAsJSX()); - root.render(); - }); - root.render(); - root.render(); - + // These updates are batched. This replicates the behavior of the original + // `act` implementation, for compatibility. + root.render('A'); + root.render('B'); + // Nothing has rendered yet. + expect(root).toMatchRenderedOutput(null); await null; - assertLog([ - // A and B should render in a single batch _before_ the microtask queue - // has run. This replicates the behavior of the original `act` - // implementation, for compatibility. - 'B', - 'Current tree in microtask: B', - - // C isn't scheduled until a microtask, so it's rendered separately. - 'C', - ]); - - // Subsequent updates should also render in separate batches. - root.render(); - root.render(); - assertLog(['D', 'E']); - }); - }); + // Updates are flushed after the first await. + expect(root).toMatchRenderedOutput('B'); - // @gate __DEV__ - test('in legacy mode, in an async scope, updates are batched until the first `await` (regression test: batchedUpdates)', async () => { - const root = ReactNoop.createLegacyRoot(); - - await act(async () => { - queueMicrotask(() => { - Scheduler.log('Current tree in microtask: ' + root.getChildrenAsJSX()); - root.render(); - }); - - // This is a regression test. The presence of `batchedUpdates` would cause - // these updates to not flush until a microtask. The correct behavior is - // that they flush before the microtask queue, regardless of whether - // they are wrapped with `batchedUpdates`. - ReactNoop.batchedUpdates(() => { - root.render(); - root.render(); - }); - - await null; - assertLog([ - // A and B should render in a single batch _before_ the microtask queue - // has run. This replicates the behavior of the original `act` - // implementation, for compatibility. - 'B', - 'Current tree in microtask: B', - - // C isn't scheduled until a microtask, so it's rendered separately. - 'C', - ]); - - // Subsequent updates should also render in separate batches. - root.render(); - root.render(); - assertLog(['D', 'E']); + // Subsequent updates in the same scope aren't batched. + root.render('C'); + expect(root).toMatchRenderedOutput('C'); }); }); diff --git a/scripts/jest/matchers/reactTestMatchers.js b/scripts/jest/matchers/reactTestMatchers.js index 63d03c5d70936..ecf0329a0407d 100644 --- a/scripts/jest/matchers/reactTestMatchers.js +++ b/scripts/jest/matchers/reactTestMatchers.js @@ -20,13 +20,13 @@ function captureAssertion(fn) { return {pass: true}; } -function assertYieldsWereCleared(Scheduler, caller) { +function assertYieldsWereCleared(Scheduler) { const actualYields = Scheduler.unstable_clearLog(); if (actualYields.length !== 0) { const error = Error( 'The event log is not empty. Call assertLog(...) first.' ); - Error.captureStackTrace(error, caller); + Error.captureStackTrace(error, assertYieldsWereCleared); throw error; } } @@ -34,7 +34,7 @@ function assertYieldsWereCleared(Scheduler, caller) { function toMatchRenderedOutput(ReactNoop, expectedJSX) { if (typeof ReactNoop.getChildrenAsJSX === 'function') { const Scheduler = ReactNoop._Scheduler; - assertYieldsWereCleared(Scheduler, toMatchRenderedOutput); + assertYieldsWereCleared(Scheduler); return captureAssertion(() => { expect(ReactNoop.getChildrenAsJSX()).toEqual(expectedJSX); });