Double invoked effects on suspended children (#25307)
Co-authored-by: Robert Balicki <robertbalicki@fb.com>
Samuel Susla committed
Sep 21, 2022 at 16:58 UTC
0cac4d54c3eb144b72b7bf0ccf05b3740c6b747f
3 files changed
+165
-30
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+46
-15
@@ -3211,12 +3211,20 @@ function recursivelyTraverseAndDoubleInvokeEffectsInDEV(
3211
}
3212
let child = parentFiber.child;
3213
while (child !== null) {
3214
- doubleInvokeEffectsInDEV(root, child, isInStrictMode);
3214
+ doubleInvokeEffectsInDEVIfNecessary(root, child, isInStrictMode);
3215
child = child.sibling;
3216
}
3217
}
3218
3219
-function doubleInvokeEffectsInDEV(
3219
+// Unconditionally disconnects and connects passive and layout effects.
3220
+function doubleInvokeEffectsOnFiber(root: FiberRoot, fiber: Fiber) {
3221
+ disappearLayoutEffects(fiber);
3222
+ disconnectPassiveEffect(fiber);
3223
+ reappearLayoutEffects(root, fiber.alternate, fiber, false);
3224
+ reconnectPassiveEffects(root, fiber, NoLanes, null, false);
3225
+}
3226
+
3227
+function doubleInvokeEffectsInDEVIfNecessary(
3228
root: FiberRoot,
3229
fiber: Fiber,
3230
parentIsInStrictMode: boolean,
@@ -3224,22 +3232,45 @@ function doubleInvokeEffectsInDEV(
3232
const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE;
3233
const isInStrictMode = parentIsInStrictMode || isStrictModeFiber;
3234
3227
- if (fiber.flags & PlacementDEV || fiber.tag === OffscreenComponent) {
3235
+ // First case: the fiber **is not** of type OffscreenComponent. No
3236
+ // special rules apply to double invoking effects.
3237
+ if (fiber.tag !== OffscreenComponent) {
3238
+ if (fiber.flags & PlacementDEV) {
3239
+ setCurrentDebugFiberInDEV(fiber);
3240
+ if (isInStrictMode) {
3241
+ doubleInvokeEffectsOnFiber(root, fiber);
3242
+ }
3243
+ resetCurrentDebugFiberInDEV();
3244
+ } else {
3245
+ recursivelyTraverseAndDoubleInvokeEffectsInDEV(
3246
+ root,
3247
+ fiber,
3248
+ isInStrictMode,
3249
+ );
3250
+ }
3251
+ return;
3252
+ }
3253
+
3254
+ // Second case: the fiber **is** of type OffscreenComponent.
3255
+ // This branch contains cases specific to Offscreen.
3256
+ if (fiber.memoizedState === null) {
3257
+ // Only consider Offscreen that is visible.
3258
+ // TODO (Offscreen) Handle manual mode.
3259
setCurrentDebugFiberInDEV(fiber);
3229
- const isNotOffscreen = fiber.tag !== OffscreenComponent;
3230
- // Checks if Offscreen is being revealed. For all other components, evaluates to true.
3231
- const hasOffscreenBecomeVisible =
3232
- isNotOffscreen ||
3233
- (fiber.flags & Visibility && fiber.memoizedState === null);
3234
- if (isInStrictMode && hasOffscreenBecomeVisible) {
3235
- disappearLayoutEffects(fiber);
3236
- disconnectPassiveEffect(fiber);
3237
- reappearLayoutEffects(root, fiber.alternate, fiber, false);
3238
- reconnectPassiveEffects(root, fiber, NoLanes, null, false);
3260
+ if (isInStrictMode && fiber.flags & Visibility) {
3261
+ // Double invoke effects on Offscreen's subtree only
3262
+ // if it is visible and its visibility has changed.
3263
+ doubleInvokeEffectsOnFiber(root, fiber);
3264
+ } else if (fiber.subtreeFlags & PlacementDEV) {
3265
+ // Something in the subtree could have been suspended.
3266
+ // We need to continue traversal and find newly inserted fibers.
3267
+ recursivelyTraverseAndDoubleInvokeEffectsInDEV(
3268
+ root,
3269
+ fiber,
3270
+ isInStrictMode,
3271
+ );
3272
}
3273
resetCurrentDebugFiberInDEV();
3241
- } else {
3242
- recursivelyTraverseAndDoubleInvokeEffectsInDEV(root, fiber, isInStrictMode);
3274
}
3275
}
3276
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+46
-15
@@ -3211,12 +3211,20 @@ function recursivelyTraverseAndDoubleInvokeEffectsInDEV(
3211
}
3212
let child = parentFiber.child;
3213
while (child !== null) {
3214
- doubleInvokeEffectsInDEV(root, child, isInStrictMode);
3214
+ doubleInvokeEffectsInDEVIfNecessary(root, child, isInStrictMode);
3215
child = child.sibling;
3216
}
3217
}
3218
3219
-function doubleInvokeEffectsInDEV(
3219
+// Unconditionally disconnects and connects passive and layout effects.
3220
+function doubleInvokeEffectsOnFiber(root: FiberRoot, fiber: Fiber) {
3221
+ disappearLayoutEffects(fiber);
3222
+ disconnectPassiveEffect(fiber);
3223
+ reappearLayoutEffects(root, fiber.alternate, fiber, false);
3224
+ reconnectPassiveEffects(root, fiber, NoLanes, null, false);
3225
+}
3226
+
3227
+function doubleInvokeEffectsInDEVIfNecessary(
3228
root: FiberRoot,
3229
fiber: Fiber,
3230
parentIsInStrictMode: boolean,
@@ -3224,22 +3232,45 @@ function doubleInvokeEffectsInDEV(
3232
const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE;
3233
const isInStrictMode = parentIsInStrictMode || isStrictModeFiber;
3234
3227
- if (fiber.flags & PlacementDEV || fiber.tag === OffscreenComponent) {
3235
+ // First case: the fiber **is not** of type OffscreenComponent. No
3236
+ // special rules apply to double invoking effects.
3237
+ if (fiber.tag !== OffscreenComponent) {
3238
+ if (fiber.flags & PlacementDEV) {
3239
+ setCurrentDebugFiberInDEV(fiber);
3240
+ if (isInStrictMode) {
3241
+ doubleInvokeEffectsOnFiber(root, fiber);
3242
+ }
3243
+ resetCurrentDebugFiberInDEV();
3244
+ } else {
3245
+ recursivelyTraverseAndDoubleInvokeEffectsInDEV(
3246
+ root,
3247
+ fiber,
3248
+ isInStrictMode,
3249
+ );
3250
+ }
3251
+ return;
3252
+ }
3253
+
3254
+ // Second case: the fiber **is** of type OffscreenComponent.
3255
+ // This branch contains cases specific to Offscreen.
3256
+ if (fiber.memoizedState === null) {
3257
+ // Only consider Offscreen that is visible.
3258
+ // TODO (Offscreen) Handle manual mode.
3259
setCurrentDebugFiberInDEV(fiber);
3229
- const isNotOffscreen = fiber.tag !== OffscreenComponent;
3230
- // Checks if Offscreen is being revealed. For all other components, evaluates to true.
3231
- const hasOffscreenBecomeVisible =
3232
- isNotOffscreen ||
3233
- (fiber.flags & Visibility && fiber.memoizedState === null);
3234
- if (isInStrictMode && hasOffscreenBecomeVisible) {
3235
- disappearLayoutEffects(fiber);
3236
- disconnectPassiveEffect(fiber);
3237
- reappearLayoutEffects(root, fiber.alternate, fiber, false);
3238
- reconnectPassiveEffects(root, fiber, NoLanes, null, false);
3260
+ if (isInStrictMode && fiber.flags & Visibility) {
3261
+ // Double invoke effects on Offscreen's subtree only
3262
+ // if it is visible and its visibility has changed.
3263
+ doubleInvokeEffectsOnFiber(root, fiber);
3264
+ } else if (fiber.subtreeFlags & PlacementDEV) {
3265
+ // Something in the subtree could have been suspended.
3266
+ // We need to continue traversal and find newly inserted fibers.
3267
+ recursivelyTraverseAndDoubleInvokeEffectsInDEV(
3268
+ root,
3269
+ fiber,
3270
+ isInStrictMode,
3271
+ );
3272
}
3273
resetCurrentDebugFiberInDEV();
3241
- } else {
3242
- recursivelyTraverseAndDoubleInvokeEffectsInDEV(root, fiber, isInStrictMode);
3274
}
3275
}
3276
packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js
+73
@@ -153,4 +153,77 @@ describe('ReactOffscreenStrictMode', () => {
153
);
154
});
155
});
156
+
157
+ // @gate __DEV__ && enableStrictEffects && enableOffscreen
158
+ it('should double invoke effects on unsuspended child', async () => {
159
+ let shouldSuspend = true;
160
+ let resolve;
161
+ const suspensePromise = new Promise(_resolve => {
162
+ resolve = _resolve;
163
+ });
164
+
165
+ function Parent() {
166
+ log.push('Parent rendered');
167
+ React.useEffect(() => {
168
+ log.push('Parent mount');
169
+ return () => {
170
+ log.push('Parent unmount');
171
+ };
172
+ });
173
+
174
+ return (
175
+ <React.Suspense fallback="fallback">
176
+ <Child />
177
+ </React.Suspense>
178
+ );
179
+ }
180
+
181
+ function Child() {
182
+ log.push('Child rendered');
183
+ React.useEffect(() => {
184
+ log.push('Child mount');
185
+ return () => {
186
+ log.push('Child unmount');
187
+ };
188
+ });
189
+ if (shouldSuspend) {
190
+ log.push('Child suspended');
191
+ throw suspensePromise;
192
+ }
193
+ return null;
194
+ }
195
+
196
+ act(() => {
197
+ ReactNoop.render(
198
+ <React.StrictMode>
199
+ <Offscreen mode="visible">
200
+ <Parent />
201
+ </Offscreen>
202
+ </React.StrictMode>,
203
+ );
204
+ });
205
+
206
+ log.push('------------------------------');
207
+
208
+ await act(async () => {
209
+ resolve();
210
+ shouldSuspend = false;
211
+ });
212
+
213
+ expect(log).toEqual([
214
+ 'Parent rendered',
215
+ 'Parent rendered',
216
+ 'Child rendered',
217
+ 'Child suspended',
218
+ 'Parent mount',
219
+ 'Parent unmount',
220
+ 'Parent mount',
221
+ '------------------------------',
222
+ 'Child rendered',
223
+ 'Child rendered',
224
+ 'Child mount',
225
+ 'Child unmount',
226
+ 'Child mount',
227
+ ]);
228
+ });
229
});