diff --git a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js
index bc85d18b108ce..4f24770546176 100644
--- a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js
@@ -72,42 +72,6 @@ describe('ReactDOMHooks', () => {
expect(container3.textContent).toBe('6');
});
- it('can batch synchronous work inside effects with other work', () => {
- let otherContainer = document.createElement('div');
-
- let calledA = false;
- function A() {
- calledA = true;
- return 'A';
- }
-
- let calledB = false;
- function B() {
- calledB = true;
- return 'B';
- }
-
- let _set;
- function Foo() {
- _set = React.useState(0)[1];
- React.useEffect(() => {
- ReactDOM.render(<A />, otherContainer);
- });
- return null;
- }
-
- ReactDOM.render(<Foo />, container);
- ReactDOM.unstable_batchedUpdates(() => {
- _set(0); // Forces the effect to be flushed
- expect(otherContainer.textContent).toBe('A');
- ReactDOM.render(<B />, otherContainer);
- expect(otherContainer.textContent).toBe('A');
- });
- expect(otherContainer.textContent).toBe('B');
- expect(calledA).toBe(true);
- expect(calledB).toBe(true);
- });
-
it('should not bail out when an update is scheduled from within an event handler', () => {
const {createRef, useCallback, useState} = React;
diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js
index 17fd298ef7fce..9c65a665c8eeb 100644
--- a/packages/react-reconciler/src/ReactFiberClassComponent.js
+++ b/packages/react-reconciler/src/ReactFiberClassComponent.js
@@ -52,7 +52,6 @@ import {
requestCurrentTime,
computeExpirationForFiber,
scheduleWork,
- flushPassiveEffects,
} from './ReactFiberScheduler';
const fakeInternalInstance = {};
@@ -194,7 +193,6 @@ const classComponentUpdater = {
update.callback = callback;
}
- flushPassiveEffects();
enqueueUpdate(fiber, update);
scheduleWork(fiber, expirationTime);
},
@@ -214,7 +212,6 @@ const classComponentUpdater = {
update.callback = callback;
}
- flushPassiveEffects();
enqueueUpdate(fiber, update);
scheduleWork(fiber, expirationTime);
},
@@ -233,7 +230,6 @@ const classComponentUpdater = {
update.callback = callback;
}
- flushPassiveEffects();
enqueueUpdate(fiber, update);
scheduleWork(fiber, expirationTime);
},
diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
index b5b7f9dd79f7a..ffa4779eae712 100644
--- a/packages/react-reconciler/src/ReactFiberHooks.js
+++ b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -31,7 +31,6 @@ import {
import {
scheduleWork,
computeExpirationForFiber,
- flushPassiveEffects,
requestCurrentTime,
warnIfNotCurrentlyActingUpdatesInDev,
markRenderEventTime,
@@ -1108,8 +1107,6 @@ function dispatchAction<S, A>(
lastRenderPhaseUpdate.next = update;
}
} else {
- flushPassiveEffects();
-
const currentTime = requestCurrentTime();
const expirationTime = computeExpirationForFiber(currentTime, fiber);
diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js
index 3e1be7241e9d4..cae00c7b8964c 100644
--- a/packages/react-reconciler/src/ReactFiberReconciler.js
+++ b/packages/react-reconciler/src/ReactFiberReconciler.js
@@ -152,7 +152,6 @@ function scheduleRootUpdate(
update.callback = callback;
}
- flushPassiveEffects();
enqueueUpdate(current, update);
scheduleWork(current, expirationTime);
@@ -392,8 +391,6 @@ if (__DEV__) {
id--;
}
if (currentHook !== null) {
- flushPassiveEffects();
-
const newState = copyWithSet(currentHook.memoizedState, path, value);
currentHook.memoizedState = newState;
currentHook.baseState = newState;
@@ -411,7 +408,6 @@ if (__DEV__) {
// Support DevTools props for function components, forwardRef, memo, host components, etc.
overrideProps = (fiber: Fiber, path: Array<string | number>, value: any) => {
- flushPassiveEffects();
fiber.pendingProps = copyWithSet(fiber.memoizedProps, path, value);
if (fiber.alternate) {
fiber.alternate.pendingProps = fiber.pendingProps;
@@ -420,7 +416,6 @@ if (__DEV__) {
};
scheduleUpdate = (fiber: Fiber) => {
- flushPassiveEffects();
scheduleWork(fiber, Sync);
};
diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js
index 3de722bf11d21..09b5c2b4b0c71 100644
--- a/packages/react-reconciler/src/ReactFiberScheduler.js
+++ b/packages/react-reconciler/src/ReactFiberScheduler.js
@@ -560,6 +560,9 @@ export function flushInteractiveUpdates() {
return;
}
flushPendingDiscreteUpdates();
+ // If the discrete updates scheduled passive effects, flush them now so that
+ // they fire before the next serial event.
+ flushPassiveEffects();
}
function resolveLocksOnRoot(root: FiberRoot, expirationTime: ExpirationTime) {
@@ -595,6 +598,8 @@ export function interactiveUpdates<A, B, C, R>(
// should explicitly call flushInteractiveUpdates.
flushPendingDiscreteUpdates();
}
+ // TODO: Remove this call for the same reason as above.
+ flushPassiveEffects();
return runWithPriority(UserBlockingPriority, fn.bind(null, a, b, c));
}
diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
index e2e5178ba37f5..50276bd8b7e06 100644
--- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
@@ -1719,6 +1719,58 @@ describe('ReactHooks', () => {
).toThrow('Hello');
});
+ // Regression test for https://github.com/facebook/react/issues/15057
+ it('does not fire a false positive warning when previous effect unmounts the component', () => {
+ let {useState, useEffect} = React;
+ let globalListener;
+
+ function A() {
+ const [show, setShow] = useState(true);
+ function hideMe() {
+ setShow(false);
+ }
+ return show ? <B hideMe={hideMe} /> : null;
+ }
+
+ function B(props) {
+ return <C {...props} />;
+ }
+
+ function C({hideMe}) {
+ const [, setState] = useState();
+
+ useEffect(() => {
+ let isStale = false;
+
+ globalListener = () => {
+ if (!isStale) {
+ setState('hello');
+ }
+ };
+
+ return () => {
+ isStale = true;
+ hideMe();
+ };
+ });
+ return null;
+ }
+
+ ReactTestRenderer.act(() => {
+ ReactTestRenderer.create(<A />);
+ });
+
+ expect(() => {
+ globalListener();
+ globalListener();
+ }).toWarnDev([
+ 'An update to C inside a test was not wrapped in act',
+ 'An update to C inside a test was not wrapped in act',
+ // Note: should *not* warn about updates on unmounted component.
+ // Because there's no way for component to know it got unmounted.
+ ]);
+ });
+
// Regression test for https://github.com/facebook/react/issues/14790
it('does not fire a false positive warning when suspending memo', async () => {
const {Suspense, useState} = React;
diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
index 83cfd07dc91a1..b900086a49e74 100644
--- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
@@ -670,8 +670,7 @@ describe('ReactHooksWithNoopRenderer', () => {
// Destroying the first child shouldn't prevent the passive effect from
// being executed
ReactNoop.render([passive]);
- expect(Scheduler).toHaveYielded(['Passive effect']);
- expect(Scheduler).toFlushAndYield([]);
+ expect(Scheduler).toFlushAndYield(['Passive effect']);
expect(ReactNoop.getChildren()).toEqual([span('Passive')]);
// (No effects are left to flush.)
@@ -776,11 +775,12 @@ describe('ReactHooksWithNoopRenderer', () => {
ReactNoop.render(<Counter count={1} />, () =>
Scheduler.yieldValue('Sync effect'),
);
- expect(Scheduler).toHaveYielded([
+ expect(Scheduler).toFlushAndYieldThrough([
// The previous effect flushes before the reconciliation
'Committed state when effect was fired: 0',
+ 1,
+ 'Sync effect',
]);
- expect(Scheduler).toFlushAndYieldThrough([1, 'Sync effect']);
expect(ReactNoop.getChildren()).toEqual([span(1)]);
ReactNoop.flushPassiveEffects();
@@ -849,8 +849,10 @@ describe('ReactHooksWithNoopRenderer', () => {
ReactNoop.render(<Counter count={1} />, () =>
Scheduler.yieldValue('Sync effect'),
);
- expect(Scheduler).toHaveYielded(['Schedule update [0]']);
- expect(Scheduler).toFlushAndYieldThrough(['Count: 0']);
+ expect(Scheduler).toFlushAndYieldThrough([
+ 'Schedule update [0]',
+ 'Count: 0',
+ ]);
expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
expect(Scheduler).toFlushAndYieldThrough(['Sync effect']);
@@ -862,7 +864,7 @@ describe('ReactHooksWithNoopRenderer', () => {
expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
});
- it('flushes serial effects before enqueueing work', () => {
+ it('flushes passive effects when flushing discrete updates', () => {
let _updateCount;
function Counter(props) {
const [count, updateCount] = useState(0);
@@ -880,15 +882,17 @@ describe('ReactHooksWithNoopRenderer', () => {
expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
- // Enqueuing this update forces the passive effect to be flushed --
+ // A discrete event forces the passive effect to be flushed --
// updateCount(1) happens first, so 2 wins.
- act(() => _updateCount(2));
+ ReactNoop.interactiveUpdates(() => {
+ act(() => _updateCount(2));
+ });
expect(Scheduler).toHaveYielded(['Will set count to 1']);
expect(Scheduler).toFlushAndYield(['Count: 2']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]);
});
- it('flushes serial effects before enqueueing work (with tracing)', () => {
+ it('flushes passive effects when flushing discrete updates (with tracing)', () => {
const onInteractionScheduledWorkCompleted = jest.fn();
const onWorkCanceled = jest.fn();
SchedulerTracing.unstable_subscribe({
@@ -929,9 +933,11 @@ describe('ReactHooksWithNoopRenderer', () => {
expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(0);
- // Enqueuing this update forces the passive effect to be flushed --
+ // A discrete event forces the passive effect to be flushed --
// updateCount(1) happens first, so 2 wins.
- act(() => _updateCount(2));
+ ReactNoop.interactiveUpdates(() => {
+ act(() => _updateCount(2));
+ });
expect(Scheduler).toHaveYielded(['Will set count to 1']);
expect(Scheduler).toFlushAndYield(['Count: 2']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]);
@@ -1472,8 +1478,8 @@ describe('ReactHooksWithNoopRenderer', () => {
ReactNoop.render(<Counter count={1} />, () =>
Scheduler.yieldValue('Sync effect'),
);
- expect(Scheduler).toHaveYielded(['Mount normal [current: 0]']);
expect(Scheduler).toFlushAndYieldThrough([
+ 'Mount normal [current: 0]',
'Unmount layout [current: 0]',
'Mount layout [current: 1]',
'Sync effect',