Skip to content

Commit aa17cc2

Browse files
acdliteacdlite
committed
Move flushSync warning to React DOM
When you call in `flushSync` from an effect, React fires a warning. I've moved the implementation of this warning out of the reconciler and into React DOM. `flushSync` is a renderer API, not an isomorphic API, because it has behavior that was designed specifically for the constraints of React DOM. The equivalent API in a different renderer may not be the same. For example, React Native has a different threading model than the browser, so it might not make sense to expose a `flushSync` API to the JavaScript thread.
1 parent 2e41568 commit aa17cc2

9 files changed

+69
-55
lines changed

‎packages/react-dom/src/client/ReactDOM.js

+21-2
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import {createEventHandle} from './ReactDOMEventHandle';
2323
import {
2424
batchedUpdates,
2525
discreteUpdates,
26-
flushSync,
27-
flushSyncWithoutWarningIfAlreadyRendering,
26+
flushSync as flushSyncWithoutWarningIfAlreadyRendering,
27+
isAlreadyRendering,
2828
flushControlled,
2929
injectIntoDevTools,
3030
attemptSynchronousHydration,
@@ -163,6 +163,25 @@ const Internals = {
163163
],
164164
};
165165

166+
// Overload the definition to the two valid signatures.
167+
// Warning, this opts-out of checking the function body.
168+
declare function flushSync<R>(fn: () => R): R;
169+
// eslint-disable-next-line no-redeclare
170+
declare function flushSync(): void;
171+
// eslint-disable-next-line no-redeclare
172+
function flushSync(fn) {
173+
if (__DEV__) {
174+
if (isAlreadyRendering()) {
175+
console.error(
176+
'flushSync was called from inside a lifecycle method. React cannot ' +
177+
'flush when React is already rendering. Consider moving this call to ' +
178+
'a scheduler task or micro task.',
179+
);
180+
}
181+
}
182+
return flushSyncWithoutWarningIfAlreadyRendering(fn);
183+
}
184+
166185
export {
167186
createPortal,
168187
batchedUpdates as unstable_batchedUpdates,

‎packages/react-dom/src/client/ReactDOMLegacy.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
createContainer,
3030
findHostInstanceWithNoPortals,
3131
updateContainer,
32-
flushSyncWithoutWarningIfAlreadyRendering,
32+
flushSync,
3333
getPublicRootInstance,
3434
findHostInstance,
3535
findHostInstanceWithWarning,
@@ -174,7 +174,7 @@ function legacyRenderSubtreeIntoContainer(
174174
};
175175
}
176176
// Initial mount should not be batched.
177-
flushSyncWithoutWarningIfAlreadyRendering(() => {
177+
flushSync(() => {
178178
updateContainer(children, fiberRoot, parentComponent, callback);
179179
});
180180
} else {
@@ -357,7 +357,7 @@ export function unmountComponentAtNode(container: Container) {
357357
}
358358

359359
// Unmount should not be batched.
360-
flushSyncWithoutWarningIfAlreadyRendering(() => {
360+
flushSync(() => {
361361
legacyRenderSubtreeIntoContainer(null, null, container, false, () => {
362362
// $FlowFixMe This should probably use `delete container._reactRootContainer`
363363
container._reactRootContainer = null;

‎packages/react-noop-renderer/src/createReactNoop.js

+14-1
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,19 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
921921
return children;
922922
}
923923

924+
function flushSync<R>(fn: () => R): R {
925+
if (__DEV__) {
926+
if (NoopRenderer.isAlreadyRendering()) {
927+
console.error(
928+
'flushSync was called from inside a lifecycle method. React cannot ' +
929+
'flush when React is already rendering. Consider moving this call to ' +
930+
'a scheduler task or micro task.',
931+
);
932+
}
933+
}
934+
return NoopRenderer.flushSync(fn);
935+
}
936+
924937
let idCounter = 0;
925938

926939
const ReactNoop = {
@@ -1136,7 +1149,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
11361149
}
11371150
},
11381151

1139-
flushSync: NoopRenderer.flushSync,
1152+
flushSync,
11401153
flushPassiveEffects: NoopRenderer.flushPassiveEffects,
11411154

11421155
// Logs the current state of the tree.

‎packages/react-reconciler/src/ReactFiberReconciler.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
discreteUpdates as discreteUpdates_old,
2323
flushControlled as flushControlled_old,
2424
flushSync as flushSync_old,
25-
flushSyncWithoutWarningIfAlreadyRendering as flushSyncWithoutWarningIfAlreadyRendering_old,
25+
isAlreadyRendering as isAlreadyRendering_old,
2626
flushPassiveEffects as flushPassiveEffects_old,
2727
getPublicRootInstance as getPublicRootInstance_old,
2828
attemptSynchronousHydration as attemptSynchronousHydration_old,
@@ -59,7 +59,7 @@ import {
5959
discreteUpdates as discreteUpdates_new,
6060
flushControlled as flushControlled_new,
6161
flushSync as flushSync_new,
62-
flushSyncWithoutWarningIfAlreadyRendering as flushSyncWithoutWarningIfAlreadyRendering_new,
62+
isAlreadyRendering as isAlreadyRendering_new,
6363
flushPassiveEffects as flushPassiveEffects_new,
6464
getPublicRootInstance as getPublicRootInstance_new,
6565
attemptSynchronousHydration as attemptSynchronousHydration_new,
@@ -107,9 +107,9 @@ export const flushControlled = enableNewReconciler
107107
? flushControlled_new
108108
: flushControlled_old;
109109
export const flushSync = enableNewReconciler ? flushSync_new : flushSync_old;
110-
export const flushSyncWithoutWarningIfAlreadyRendering = enableNewReconciler
111-
? flushSyncWithoutWarningIfAlreadyRendering_new
112-
: flushSyncWithoutWarningIfAlreadyRendering_old;
110+
export const isAlreadyRendering = enableNewReconciler
111+
? isAlreadyRendering_new
112+
: isAlreadyRendering_old;
113113
export const flushPassiveEffects = enableNewReconciler
114114
? flushPassiveEffects_new
115115
: flushPassiveEffects_old;

‎packages/react-reconciler/src/ReactFiberReconciler.new.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ import {
5353
flushRoot,
5454
batchedUpdates,
5555
flushSync,
56+
isAlreadyRendering,
5657
flushControlled,
5758
deferredUpdates,
5859
discreteUpdates,
59-
flushSyncWithoutWarningIfAlreadyRendering,
6060
flushPassiveEffects,
6161
} from './ReactFiberWorkLoop.new';
6262
import {
@@ -330,7 +330,7 @@ export {
330330
discreteUpdates,
331331
flushControlled,
332332
flushSync,
333-
flushSyncWithoutWarningIfAlreadyRendering,
333+
isAlreadyRendering,
334334
flushPassiveEffects,
335335
};
336336

‎packages/react-reconciler/src/ReactFiberReconciler.old.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ import {
5353
flushRoot,
5454
batchedUpdates,
5555
flushSync,
56+
isAlreadyRendering,
5657
flushControlled,
5758
deferredUpdates,
5859
discreteUpdates,
59-
flushSyncWithoutWarningIfAlreadyRendering,
6060
flushPassiveEffects,
6161
} from './ReactFiberWorkLoop.old';
6262
import {
@@ -330,7 +330,7 @@ export {
330330
discreteUpdates,
331331
flushControlled,
332332
flushSync,
333-
flushSyncWithoutWarningIfAlreadyRendering,
333+
isAlreadyRendering,
334334
flushPassiveEffects,
335335
};
336336

‎packages/react-reconciler/src/ReactFiberWorkLoop.new.js

+10-20
Original file line numberDiff line numberDiff line change
@@ -1191,11 +1191,11 @@ export function discreteUpdates<A, B, C, D, R>(
11911191

11921192
// Overload the definition to the two valid signatures.
11931193
// Warning, this opts-out of checking the function body.
1194-
declare function flushSyncWithoutWarningIfAlreadyRendering<R>(fn: () => R): R;
1194+
declare function flushSync<R>(fn: () => R): R;
11951195
// eslint-disable-next-line no-redeclare
1196-
declare function flushSyncWithoutWarningIfAlreadyRendering(): void;
1196+
declare function flushSync(): void;
11971197
// eslint-disable-next-line no-redeclare
1198-
export function flushSyncWithoutWarningIfAlreadyRendering(fn) {
1198+
export function flushSync(fn) {
11991199
// In legacy mode, we flush pending passive effects at the beginning of the
12001200
// next event, not at the end of the previous one.
12011201
if (
@@ -1232,23 +1232,13 @@ export function flushSyncWithoutWarningIfAlreadyRendering(fn) {
12321232
}
12331233
}
12341234

1235-
// Overload the definition to the two valid signatures.
1236-
// Warning, this opts-out of checking the function body.
1237-
declare function flushSync<R>(fn: () => R): R;
1238-
// eslint-disable-next-line no-redeclare
1239-
declare function flushSync(): void;
1240-
// eslint-disable-next-line no-redeclare
1241-
export function flushSync(fn) {
1242-
if (__DEV__) {
1243-
if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
1244-
console.error(
1245-
'flushSync was called from inside a lifecycle method. React cannot ' +
1246-
'flush when React is already rendering. Consider moving this call to ' +
1247-
'a scheduler task or micro task.',
1248-
);
1249-
}
1250-
}
1251-
return flushSyncWithoutWarningIfAlreadyRendering(fn);
1235+
export function isAlreadyRendering() {
1236+
// Used by the renderer to print a warning if certain APIs are called from
1237+
// the wrong context.
1238+
return (
1239+
__DEV__ &&
1240+
(executionContext & (RenderContext | CommitContext)) !== NoContext
1241+
);
12521242
}
12531243

12541244
export function flushControlled(fn: () => mixed): void {

‎packages/react-reconciler/src/ReactFiberWorkLoop.old.js

+10-20
Original file line numberDiff line numberDiff line change
@@ -1191,11 +1191,11 @@ export function discreteUpdates<A, B, C, D, R>(
11911191

11921192
// Overload the definition to the two valid signatures.
11931193
// Warning, this opts-out of checking the function body.
1194-
declare function flushSyncWithoutWarningIfAlreadyRendering<R>(fn: () => R): R;
1194+
declare function flushSync<R>(fn: () => R): R;
11951195
// eslint-disable-next-line no-redeclare
1196-
declare function flushSyncWithoutWarningIfAlreadyRendering(): void;
1196+
declare function flushSync(): void;
11971197
// eslint-disable-next-line no-redeclare
1198-
export function flushSyncWithoutWarningIfAlreadyRendering(fn) {
1198+
export function flushSync(fn) {
11991199
// In legacy mode, we flush pending passive effects at the beginning of the
12001200
// next event, not at the end of the previous one.
12011201
if (
@@ -1232,23 +1232,13 @@ export function flushSyncWithoutWarningIfAlreadyRendering(fn) {
12321232
}
12331233
}
12341234

1235-
// Overload the definition to the two valid signatures.
1236-
// Warning, this opts-out of checking the function body.
1237-
declare function flushSync<R>(fn: () => R): R;
1238-
// eslint-disable-next-line no-redeclare
1239-
declare function flushSync(): void;
1240-
// eslint-disable-next-line no-redeclare
1241-
export function flushSync(fn) {
1242-
if (__DEV__) {
1243-
if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
1244-
console.error(
1245-
'flushSync was called from inside a lifecycle method. React cannot ' +
1246-
'flush when React is already rendering. Consider moving this call to ' +
1247-
'a scheduler task or micro task.',
1248-
);
1249-
}
1250-
}
1251-
return flushSyncWithoutWarningIfAlreadyRendering(fn);
1235+
export function isAlreadyRendering() {
1236+
// Used by the renderer to print a warning if certain APIs are called from
1237+
// the wrong context.
1238+
return (
1239+
__DEV__ &&
1240+
(executionContext & (RenderContext | CommitContext)) !== NoContext
1241+
);
12521242
}
12531243

12541244
export function flushControlled(fn: () => mixed): void {

‎packages/react-reconciler/src/__tests__/ReactFlushSync-test.js

+2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ let useState;
66
let useEffect;
77
let startTransition;
88

9+
// TODO: Migrate tests to React DOM instead of React Noop
10+
911
describe('ReactFlushSync', () => {
1012
beforeEach(() => {
1113
jest.resetModules();

0 commit comments

Comments
 (0)