Remove warning for dangling passive effects (#22609)
In legacy mode, a test can get into a situation where passive effects are "dangling" — an update finished, and scheduled some passive effects, but the effects don't flush. This is why React warns if you don't wrap updates in act. The act API is responsible for flushing passive effects. But there are some cases where the act API (in legacy roots) intentionally doesn't warn, like updates that originate from roots and classes. It's possible those updates will render children that contain useEffect. Because of this, dangling effects are still possible, and React doesn't warn about it. So we implemented a second act warning for dangling effects. However, in concurrent roots, we now enforce that all APIs that schedule React work must be wrapped in act. There's no scenario where dangling passive effects can happen that doesn't already trigger the warning for updates. So the dangling effects warning is redundant. The warning was never part of a public release. It was only enabled in concurrent roots. So we can delete it.
Andrew Clark committed
Oct 21, 2021 at 18:39 UTC
3c4c1c4703e8f7362370c44a48ac9348229e3791
5 files changed
+2
-113
packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
-31
@@ -98,43 +98,12 @@ describe('ReactTestUtils.act()', () => {
98
}).toErrorDev([]);
99
});
100
101
- it('warns in strict mode', () => {
102
- expect(() => {
103
- ReactDOM.render(
104
- <React.StrictMode>
105
- <App />
106
- </React.StrictMode>,
107
- document.createElement('div'),
108
- );
109
- }).toErrorDev([
110
- 'An update to App ran an effect, but was not wrapped in act(...)',
111
- ]);
112
- });
113
-
101
// @gate __DEV__
102
it('does not warn in concurrent mode', () => {
103
const root = ReactDOM.createRoot(document.createElement('div'));
104
act(() => root.render(<App />));
105
Scheduler.unstable_flushAll();
106
});
120
-
121
- it('warns in concurrent mode if root is strict', () => {
122
- // TODO: We don't need this error anymore in concurrent mode because
123
- // effects can only be scheduled as the result of an update, and we now
124
- // enforce all updates must be wrapped with act, not just hook updates.
125
- expect(() => {
126
- const root = ReactDOM.createRoot(document.createElement('div'), {
127
- unstable_strictMode: true,
128
- });
129
- root.render(<App />);
130
- }).toErrorDev(
131
- 'An update to Root inside a test was not wrapped in act(...)',
132
- {withoutStack: true},
133
- );
134
- expect(() => Scheduler.unstable_flushAll()).toErrorDev(
135
- 'An update to App ran an effect, but was not wrapped in act(...)',
136
- );
137
- });
107
});
108
});
109
packages/react-reconciler/src/ReactFiberHooks.new.js
-7
@@ -82,7 +82,6 @@ import {
82
scheduleUpdateOnFiber,
83
requestUpdateLane,
84
requestEventTime,
85
- warnIfNotCurrentlyActingEffectsInDEV,
85
markSkippedUpdateLanes,
86
isInterleavedUpdate,
87
} from './ReactFiberWorkLoop.new';
@@ -1676,9 +1675,6 @@ function mountEffect(
1675
create: () => (() => void) | void,
1676
deps: Array<mixed> | void | null,
1677
): void {
1679
- if (__DEV__) {
1680
- warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
1681
- }
1678
if (
1679
__DEV__ &&
1680
enableStrictEffects &&
@@ -1704,9 +1700,6 @@ function updateEffect(
1700
create: () => (() => void) | void,
1701
deps: Array<mixed> | void | null,
1702
): void {
1707
- if (__DEV__) {
1708
- warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
1709
- }
1703
return updateEffectImpl(PassiveEffect, HookPassive, create, deps);
1704
}
1705
packages/react-reconciler/src/ReactFiberHooks.old.js
-7
@@ -82,7 +82,6 @@ import {
82
scheduleUpdateOnFiber,
83
requestUpdateLane,
84
requestEventTime,
85
- warnIfNotCurrentlyActingEffectsInDEV,
85
markSkippedUpdateLanes,
86
isInterleavedUpdate,
87
} from './ReactFiberWorkLoop.old';
@@ -1676,9 +1675,6 @@ function mountEffect(
1675
create: () => (() => void) | void,
1676
deps: Array<mixed> | void | null,
1677
): void {
1679
- if (__DEV__) {
1680
- warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
1681
- }
1678
if (
1679
__DEV__ &&
1680
enableStrictEffects &&
@@ -1704,9 +1700,6 @@ function updateEffect(
1700
create: () => (() => void) | void,
1701
deps: Array<mixed> | void | null,
1702
): void {
1707
- if (__DEV__) {
1708
- warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
1709
- }
1703
return updateEffectImpl(PassiveEffect, HookPassive, create, deps);
1704
}
1705
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+1
-34
@@ -97,12 +97,7 @@ import {
97
createWorkInProgress,
98
assignFiberPropertiesInDEV,
99
} from './ReactFiber.new';
100
-import {
101
- NoMode,
102
- StrictLegacyMode,
103
- ProfileMode,
104
- ConcurrentMode,
105
-} from './ReactTypeOfMode';
100
+import {NoMode, ProfileMode, ConcurrentMode} from './ReactTypeOfMode';
101
import {
102
HostRoot,
103
IndeterminateComponent,
@@ -2860,34 +2855,6 @@ function shouldForceFlushFallbacksInDEV() {
2855
return __DEV__ && ReactCurrentActQueue.current !== null;
2856
}
2857
2863
-export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void {
2864
- if (__DEV__) {
2865
- const isActEnvironment =
2866
- fiber.mode & ConcurrentMode
2867
- ? isConcurrentActEnvironment()
2868
- : isLegacyActEnvironment(fiber);
2869
- if (
2870
- isActEnvironment &&
2871
- (fiber.mode & StrictLegacyMode) !== NoMode &&
2872
- ReactCurrentActQueue.current === null
2873
- ) {
2874
- console.error(
2875
- 'An update to %s ran an effect, but was not wrapped in act(...).\n\n' +
2876
- 'When testing, code that causes React state updates should be ' +
2877
- 'wrapped into act(...):\n\n' +
2878
- 'act(() => {\n' +
2879
- ' /* fire events that update state */\n' +
2880
- '});\n' +
2881
- '/* assert on the output */\n\n' +
2882
- "This ensures that you're testing the behavior the user would see " +
2883
- 'in the browser.' +
2884
- ' Learn more at https://reactjs.org/link/wrap-tests-with-act',
2885
- getComponentNameFromFiber(fiber),
2886
- );
2887
- }
2888
- }
2889
-}
2890
-
2858
function warnIfUpdatesNotWrappedWithActDEV(fiber: Fiber): void {
2859
if (__DEV__) {
2860
if (fiber.mode & ConcurrentMode) {
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+1
-34
@@ -97,12 +97,7 @@ import {
97
createWorkInProgress,
98
assignFiberPropertiesInDEV,
99
} from './ReactFiber.old';
100
-import {
101
- NoMode,
102
- StrictLegacyMode,
103
- ProfileMode,
104
- ConcurrentMode,
105
-} from './ReactTypeOfMode';
100
+import {NoMode, ProfileMode, ConcurrentMode} from './ReactTypeOfMode';
101
import {
102
HostRoot,
103
IndeterminateComponent,
@@ -2860,34 +2855,6 @@ function shouldForceFlushFallbacksInDEV() {
2855
return __DEV__ && ReactCurrentActQueue.current !== null;
2856
}
2857
2863
-export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void {
2864
- if (__DEV__) {
2865
- const isActEnvironment =
2866
- fiber.mode & ConcurrentMode
2867
- ? isConcurrentActEnvironment()
2868
- : isLegacyActEnvironment(fiber);
2869
- if (
2870
- isActEnvironment &&
2871
- (fiber.mode & StrictLegacyMode) !== NoMode &&
2872
- ReactCurrentActQueue.current === null
2873
- ) {
2874
- console.error(
2875
- 'An update to %s ran an effect, but was not wrapped in act(...).\n\n' +
2876
- 'When testing, code that causes React state updates should be ' +
2877
- 'wrapped into act(...):\n\n' +
2878
- 'act(() => {\n' +
2879
- ' /* fire events that update state */\n' +
2880
- '});\n' +
2881
- '/* assert on the output */\n\n' +
2882
- "This ensures that you're testing the behavior the user would see " +
2883
- 'in the browser.' +
2884
- ' Learn more at https://reactjs.org/link/wrap-tests-with-act',
2885
- getComponentNameFromFiber(fiber),
2886
- );
2887
- }
2888
- }
2889
-}
2890
-
2858
function warnIfUpdatesNotWrappedWithActDEV(fiber: Fiber): void {
2859
if (__DEV__) {
2860
if (fiber.mode & ConcurrentMode) {