Skip to content

Commit 0cac4d5

Browse files
sammy-SCrbalicki2
sammy-SC
and
rbalicki2
authored
Double invoked effects on suspended children (#25307)
Co-authored-by: Robert Balicki <robertbalicki@fb.com>
1 parent b274890 commit 0cac4d5

File tree

3 files changed

+165
-30
lines changed

3 files changed

+165
-30
lines changed

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

+46-15
Original file line numberDiff line numberDiff line change
@@ -3211,35 +3211,66 @@ function recursivelyTraverseAndDoubleInvokeEffectsInDEV(
32113211
}
32123212
let child = parentFiber.child;
32133213
while (child !== null) {
3214-
doubleInvokeEffectsInDEV(root, child, isInStrictMode);
3214+
doubleInvokeEffectsInDEVIfNecessary(root, child, isInStrictMode);
32153215
child = child.sibling;
32163216
}
32173217
}
32183218

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(
32203228
root: FiberRoot,
32213229
fiber: Fiber,
32223230
parentIsInStrictMode: boolean,
32233231
) {
32243232
const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE;
32253233
const isInStrictMode = parentIsInStrictMode || isStrictModeFiber;
32263234

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.
32283259
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+
);
32393272
}
32403273
resetCurrentDebugFiberInDEV();
3241-
} else {
3242-
recursivelyTraverseAndDoubleInvokeEffectsInDEV(root, fiber, isInStrictMode);
32433274
}
32443275
}
32453276

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

+46-15
Original file line numberDiff line numberDiff line change
@@ -3211,35 +3211,66 @@ function recursivelyTraverseAndDoubleInvokeEffectsInDEV(
32113211
}
32123212
let child = parentFiber.child;
32133213
while (child !== null) {
3214-
doubleInvokeEffectsInDEV(root, child, isInStrictMode);
3214+
doubleInvokeEffectsInDEVIfNecessary(root, child, isInStrictMode);
32153215
child = child.sibling;
32163216
}
32173217
}
32183218

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(
32203228
root: FiberRoot,
32213229
fiber: Fiber,
32223230
parentIsInStrictMode: boolean,
32233231
) {
32243232
const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE;
32253233
const isInStrictMode = parentIsInStrictMode || isStrictModeFiber;
32263234

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.
32283259
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+
);
32393272
}
32403273
resetCurrentDebugFiberInDEV();
3241-
} else {
3242-
recursivelyTraverseAndDoubleInvokeEffectsInDEV(root, fiber, isInStrictMode);
32433274
}
32443275
}
32453276

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

+73
Original file line numberDiff line numberDiff line change
@@ -153,4 +153,77 @@ describe('ReactOffscreenStrictMode', () => {
153153
);
154154
});
155155
});
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+
});
156229
});

0 commit comments

Comments
 (0)