Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Noop unstable_batchedUpdates #28120

Merged
merged 1 commit into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/react-dom/index.classic.fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ export {
findDOMNode,
flushSync,
unmountComponentAtNode,
unstable_batchedUpdates,
unstable_createEventHandle,
unstable_renderSubtreeIntoContainer,
unstable_runWithPriority, // DO NOT USE: Temporarily exposed to migrate off of Scheduler.runWithPriority.
Expand All @@ -38,6 +37,11 @@ export {
version,
} from './src/client/ReactDOM';

export {createRoot, hydrateRoot, render} from './src/client/ReactDOMRootFB';
export {
createRoot,
hydrateRoot,
render,
unstable_batchedUpdates,
} from './src/client/ReactDOMRootFB';

export {Internals as __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED};
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/ReactLegacyMount-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ describe('ReactMount', () => {
expect(calls).toBe(5);
});

// @gate !disableLegacyMode
// @gate !disableLegacyMode && classic
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this no-ops in every entry point except www-classic, these tests need the extra gate.

it('initial mount of legacy root is sync inside batchedUpdates, as if it were wrapped in flushSync', () => {
const container1 = document.createElement('div');
const container2 = document.createElement('div');
Expand Down
18 changes: 9 additions & 9 deletions packages/react-dom/src/__tests__/ReactLegacyUpdates-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('ReactLegacyUpdates', () => {
assertLog = InternalTestUtils.assertLog;
});

// @gate !disableLegacyMode
// @gate !disableLegacyMode && classic
it('should batch state when updating state twice', () => {
let updateCount = 0;

Expand Down Expand Up @@ -63,7 +63,7 @@ describe('ReactLegacyUpdates', () => {
expect(updateCount).toBe(1);
});

// @gate !disableLegacyMode
// @gate !disableLegacyMode && classic
it('should batch state when updating two different state keys', () => {
let updateCount = 0;

Expand Down Expand Up @@ -97,7 +97,7 @@ describe('ReactLegacyUpdates', () => {
expect(updateCount).toBe(1);
});

// @gate !disableLegacyMode
// @gate !disableLegacyMode && classic
it('should batch state and props together', () => {
let updateCount = 0;

Expand Down Expand Up @@ -131,7 +131,7 @@ describe('ReactLegacyUpdates', () => {
expect(updateCount).toBe(1);
});

// @gate !disableLegacyMode
// @gate !disableLegacyMode && classic
it('should batch parent/child state updates together', () => {
let parentUpdateCount = 0;

Expand Down Expand Up @@ -187,7 +187,7 @@ describe('ReactLegacyUpdates', () => {
expect(childUpdateCount).toBe(1);
});

// @gate !disableLegacyMode
// @gate !disableLegacyMode && classic
it('should batch child/parent state updates together', () => {
let parentUpdateCount = 0;

Expand Down Expand Up @@ -245,7 +245,7 @@ describe('ReactLegacyUpdates', () => {
expect(childUpdateCount).toBe(1);
});

// @gate !disableLegacyMode
// @gate !disableLegacyMode && classic
it('should support chained state updates', () => {
let updateCount = 0;

Expand Down Expand Up @@ -286,7 +286,7 @@ describe('ReactLegacyUpdates', () => {
expect(updateCount).toBe(2);
});

// @gate !disableLegacyMode
// @gate !disableLegacyMode && classic
it('should batch forceUpdate together', () => {
let shouldUpdateCount = 0;
let updateCount = 0;
Expand Down Expand Up @@ -548,7 +548,7 @@ describe('ReactLegacyUpdates', () => {
);
});

// @gate !disableLegacyMode
// @gate !disableLegacyMode && classic
it('should queue mount-ready handlers across different roots', () => {
// We'll define two components A and B, then update both of them. When A's
// componentDidUpdate handlers is called, B's DOM should already have been
Expand Down Expand Up @@ -849,7 +849,7 @@ describe('ReactLegacyUpdates', () => {
expect(callbackCount).toBe(1);
});

// @gate !disableLegacyMode
// @gate !disableLegacyMode && classic
it('does not call render after a component as been deleted', () => {
let renderCount = 0;
let componentB = null;
Expand Down
12 changes: 9 additions & 3 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import {
import {createEventHandle} from 'react-dom-bindings/src/client/ReactDOMEventHandle';

import {
batchedUpdates,
flushSync as flushSyncWithoutWarningIfAlreadyRendering,
isAlreadyRendering,
injectIntoDevTools,
Expand Down Expand Up @@ -167,9 +166,16 @@ function flushSync<R>(fn: (() => R) | void): R | void {
// Expose findDOMNode on internals
Internals.findDOMNode = findDOMNode;

function unstable_batchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
// batchedUpdates was a legacy mode feature that is a no-op outside of
// legacy mode. In 19, we made it an actual no-op, but we're keeping it
// for now since there may be libraries that still include it.
return fn(a);
rickhanlonii marked this conversation as resolved.
Show resolved Hide resolved
}

export {
createPortal,
batchedUpdates as unstable_batchedUpdates,
unstable_batchedUpdates,
flushSync,
ReactVersion as version,
// Disabled behind disableLegacyReactDOMAPIs
Expand All @@ -196,7 +202,7 @@ Internals.Events = [
getFiberCurrentPropsFromNode,
enqueueStateRestore,
restoreStateIfNeeded,
batchedUpdates,
unstable_batchedUpdates,
];

const foundDevTools = injectIntoDevTools({
Expand Down
3 changes: 3 additions & 0 deletions packages/react-dom/src/client/ReactDOMRootFB.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
} from 'react-dom-bindings/src/client/HTMLNodeType';

import {
batchedUpdates,
createContainer,
createHydrationContainer,
findHostInstanceWithNoPortals,
Expand Down Expand Up @@ -416,3 +417,5 @@ export function unstable_renderSubtreeIntoContainer(
callback,
);
}

export {batchedUpdates as unstable_batchedUpdates};
35 changes: 21 additions & 14 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
disableLegacyContext,
alwaysThrottleRetries,
enableInfiniteRenderLoopDetection,
disableLegacyMode,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import is from 'shared/objectIs';
Expand Down Expand Up @@ -1455,21 +1456,27 @@ export function deferredUpdates<A>(fn: () => A): A {
}

export function batchedUpdates<A, R>(fn: A => R, a: A): R {
const prevExecutionContext = executionContext;
executionContext |= BatchedContext;
try {
if (disableLegacyMode) {
// batchedUpdates is a no-op now, but there's still some internal react-dom
// code calling it, that we can't remove until we remove legacy mode.
return fn(a);
} finally {
executionContext = prevExecutionContext;
// If there were legacy sync updates, flush them at the end of the outer
// most batchedUpdates-like method.
if (
executionContext === NoContext &&
// Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
!(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
) {
resetRenderTimer();
flushSyncWorkOnLegacyRootsOnly();
} else {
const prevExecutionContext = executionContext;
executionContext |= BatchedContext;
try {
return fn(a);
} finally {
executionContext = prevExecutionContext;
// If there were legacy sync updates, flush them at the end of the outer
// most batchedUpdates-like method.
if (
executionContext === NoContext &&
// Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
!(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
) {
resetRenderTimer();
flushSyncWorkOnLegacyRootsOnly();
}
}
}
}
Expand Down