Skip to content

Commit

Permalink
Revert "act: Move didScheduleLegacyUpdate to ensureRootIsScheduled (#…
Browse files Browse the repository at this point in the history
…26552)"

This reverts commit fec97ec.
  • Loading branch information
kassens committed Apr 17, 2023
1 parent 9429ec9 commit 7b6e17c
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 80 deletions.
9 changes: 0 additions & 9 deletions packages/react-reconciler/src/ReactFiberRootScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
79 changes: 11 additions & 68 deletions packages/react-reconciler/src/__tests__/ReactIsomorphicAct-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -34,7 +31,6 @@ describe('isomorphic act()', () => {
startTransition = React.startTransition;

waitForMicrotasks = require('internal-test-utils').waitForMicrotasks;
assertLog = require('internal-test-utils').assertLog;
});

beforeEach(() => {
Expand All @@ -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();
Expand Down Expand Up @@ -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(<Text text="C" />);
});
root.render(<Text text="A" />);
root.render(<Text text="B" />);

// 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(<Text text="D" />);
root.render(<Text text="E" />);
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(<Text text="C" />);
});

// 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(<Text text="A" />);
root.render(<Text text="B" />);
});

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(<Text text="D" />);
root.render(<Text text="E" />);
assertLog(['D', 'E']);
// Subsequent updates in the same scope aren't batched.
root.render('C');
expect(root).toMatchRenderedOutput('C');
});
});

Expand Down
6 changes: 3 additions & 3 deletions scripts/jest/matchers/reactTestMatchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@ 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;
}
}

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);
});
Expand Down

0 comments on commit 7b6e17c

Please sign in to comment.