Skip to content

Commit 67e4759

Browse files
authored
[Fiber] Double invoke Effects in Strict Mode during Hydration (#35961)
1 parent 23fcd7c commit 67e4759

File tree

5 files changed

+257
-5
lines changed

5 files changed

+257
-5
lines changed

packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ let Scheduler;
2020
let Suspense;
2121
let SuspenseList;
2222
let useSyncExternalStore;
23+
let use;
2324
let act;
2425
let IdleEventPriority;
2526
let waitForAll;
@@ -116,6 +117,7 @@ describe('ReactDOMServerPartialHydration', () => {
116117
Activity = React.Activity;
117118
Suspense = React.Suspense;
118119
useSyncExternalStore = React.useSyncExternalStore;
120+
use = React.use;
119121
if (gate(flags => flags.enableSuspenseList)) {
120122
SuspenseList = React.unstable_SuspenseList;
121123
}
@@ -256,6 +258,77 @@ describe('ReactDOMServerPartialHydration', () => {
256258
expect(container.textContent).toBe('HelloHello');
257259
});
258260

261+
it('replays effects when a suspended boundary hydrates in StrictMode', async () => {
262+
const log = [];
263+
let suspend = false;
264+
let resolve;
265+
const promise = new Promise(resolvePromise => (resolve = resolvePromise));
266+
267+
function EffectfulChild() {
268+
React.useLayoutEffect(() => {
269+
log.push('layout mount');
270+
return () => log.push('layout unmount');
271+
}, []);
272+
React.useEffect(() => {
273+
log.push('effect mount');
274+
return () => log.push('effect unmount');
275+
}, []);
276+
return 'Hello';
277+
}
278+
279+
function Child() {
280+
if (suspend) {
281+
use(promise);
282+
}
283+
return <EffectfulChild />;
284+
}
285+
286+
function App() {
287+
return (
288+
<Suspense fallback="Loading...">
289+
<Child />
290+
</Suspense>
291+
);
292+
}
293+
294+
const element = (
295+
<React.StrictMode>
296+
<App />
297+
</React.StrictMode>
298+
);
299+
300+
suspend = false;
301+
const finalHTML = ReactDOMServer.renderToString(element);
302+
const container = document.createElement('div');
303+
container.innerHTML = finalHTML;
304+
expect(container.textContent).toBe('Hello');
305+
306+
suspend = true;
307+
ReactDOMClient.hydrateRoot(container, element);
308+
await waitForAll([]);
309+
expect(log).toEqual([]);
310+
expect(container.textContent).toBe('Hello');
311+
312+
suspend = false;
313+
resolve();
314+
await promise;
315+
await waitForAll([]);
316+
317+
expect(container.textContent).toBe('Hello');
318+
if (__DEV__) {
319+
expect(log).toEqual([
320+
'layout mount',
321+
'effect mount',
322+
'layout unmount',
323+
'effect unmount',
324+
'layout mount',
325+
'effect mount',
326+
]);
327+
} else {
328+
expect(log).toEqual(['layout mount', 'effect mount']);
329+
}
330+
});
331+
259332
it('falls back to client rendering boundary on mismatch', async () => {
260333
let client = false;
261334
let suspend = false;

packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,56 @@ describe('ReactDOMServerHydration', () => {
392392
expect(element.textContent).toBe('Hi');
393393
});
394394

395+
it('replays effects when hydrating a StrictMode subtree', async () => {
396+
const log = [];
397+
function Child() {
398+
React.useLayoutEffect(() => {
399+
log.push('layout mount');
400+
return () => log.push('layout unmount');
401+
}, []);
402+
React.useEffect(() => {
403+
log.push('effect mount');
404+
return () => log.push('effect unmount');
405+
}, []);
406+
return <span>Hello</span>;
407+
}
408+
409+
function App() {
410+
return (
411+
<div>
412+
<Child />
413+
</div>
414+
);
415+
}
416+
417+
const markup = (
418+
<React.StrictMode>
419+
<App />
420+
</React.StrictMode>
421+
);
422+
423+
const element = document.createElement('div');
424+
element.innerHTML = ReactDOMServer.renderToString(markup);
425+
expect(element.textContent).toBe('Hello');
426+
427+
await act(() => {
428+
ReactDOMClient.hydrateRoot(element, markup);
429+
});
430+
431+
if (__DEV__) {
432+
expect(log).toEqual([
433+
'layout mount',
434+
'effect mount',
435+
'layout unmount',
436+
'effect unmount',
437+
'layout mount',
438+
'effect mount',
439+
]);
440+
} else {
441+
expect(log).toEqual(['layout mount', 'effect mount']);
442+
}
443+
});
444+
395445
it('should be able to render and hydrate forwardRef components', async () => {
396446
const FunctionComponent = ({label, forwardedRef}) => (
397447
<div ref={forwardedRef}>{label}</div>

packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ import {
8888
NoFlags,
8989
PerformedWork,
9090
Placement,
91+
PlacementDEV,
9192
Hydrating,
9293
Callback,
9394
ContentReset,
@@ -1080,7 +1081,8 @@ function updateDehydratedActivityComponent(
10801081
// Conceptually this is similar to Placement in that a new subtree is
10811082
// inserted into the React tree here. It just happens to not need DOM
10821083
// mutations because it already exists.
1083-
primaryChildFragment.flags |= Hydrating;
1084+
// We should still treat it as a newly inserted Fiber to double invoke Strict Effects.
1085+
primaryChildFragment.flags |= Hydrating | PlacementDEV;
10841086
return primaryChildFragment;
10851087
}
10861088
} else {
@@ -1899,7 +1901,8 @@ function updateHostRoot(
18991901
// Conceptually this is similar to Placement in that a new subtree is
19001902
// inserted into the React tree here. It just happens to not need DOM
19011903
// mutations because it already exists.
1902-
node.flags = (node.flags & ~Placement) | Hydrating;
1904+
// We should still treat it as a newly inserted Fiber to double invoke Strict Effects.
1905+
node.flags = (node.flags & ~Placement) | Hydrating | PlacementDEV;
19031906
node = node.sibling;
19041907
}
19051908
}
@@ -3104,7 +3107,8 @@ function updateDehydratedSuspenseComponent(
31043107
// Conceptually this is similar to Placement in that a new subtree is
31053108
// inserted into the React tree here. It just happens to not need DOM
31063109
// mutations because it already exists.
3107-
primaryChildFragment.flags |= Hydrating;
3110+
// We should still treat it as a newly inserted Fiber to double invoke Strict Effects.
3111+
primaryChildFragment.flags |= Hydrating | PlacementDEV;
31083112
return primaryChildFragment;
31093113
}
31103114
} else {

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5312,9 +5312,11 @@ function doubleInvokeEffectsInDEVIfNecessary(
53125312
if (fiber.memoizedState === null) {
53135313
// Only consider Offscreen that is visible.
53145314
// TODO (Offscreen) Handle manual mode.
5315-
if (isInStrictMode && fiber.flags & Visibility) {
5316-
// Double invoke effects on Offscreen's subtree only
5315+
if (isInStrictMode && fiber.flags & (Visibility | PlacementDEV)) {
5316+
// Double invoke effects on Offscreen's subtree
53175317
// if it is visible and its visibility has changed.
5318+
// However, we also need to consider newly hydrated Offscreen because their
5319+
// visibility flags might not have changed.
53185320
runWithFiberInDEV(fiber, doubleInvokeEffectsOnFiber, root, fiber);
53195321
} else if (fiber.subtreeFlags & PlacementDEV) {
53205322
// Something in the subtree could have been suspended.

packages/react-reconciler/src/__tests__/ActivityStrictMode-test.js

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,4 +230,127 @@ describe('Activity StrictMode', () => {
230230
'Child mount',
231231
]);
232232
});
233+
234+
// @gate __DEV__
235+
it('should double invoke effects on newly inserted children while Activity becomes visible', async () => {
236+
function Parent({children}) {
237+
log.push('Parent rendered');
238+
React.useEffect(() => {
239+
log.push('Parent mount');
240+
return () => {
241+
log.push('Parent unmount');
242+
};
243+
});
244+
245+
return <div>{children}</div>;
246+
}
247+
248+
function Child({name}) {
249+
log.push(`Child ${name} rendered`);
250+
React.useEffect(() => {
251+
log.push(`Child ${name} mount`);
252+
return () => {
253+
log.push(`Child ${name} unmount`);
254+
};
255+
});
256+
257+
return null;
258+
}
259+
260+
await act(() => {
261+
ReactNoop.render(
262+
<React.StrictMode>
263+
<Activity mode="hidden">
264+
<Parent />
265+
</Activity>
266+
</React.StrictMode>,
267+
);
268+
});
269+
270+
expect(log).toEqual(['Parent rendered', 'Parent rendered']);
271+
272+
log.length = 0;
273+
await act(() => {
274+
ReactNoop.render(
275+
<React.StrictMode>
276+
<Activity mode="visible">
277+
<Parent>
278+
<Child name="one" />
279+
</Parent>
280+
</Activity>
281+
</React.StrictMode>,
282+
);
283+
});
284+
285+
expect(log).toEqual([
286+
'Parent rendered',
287+
'Parent rendered',
288+
'Child one rendered',
289+
'Child one rendered',
290+
'Child one mount',
291+
'Parent mount',
292+
// StrictMode double invocation
293+
'Parent unmount',
294+
'Child one unmount',
295+
'Child one mount',
296+
'Parent mount',
297+
]);
298+
299+
log.length = 0;
300+
await act(() => {
301+
ReactNoop.render(
302+
<React.StrictMode>
303+
<Activity mode="visible">
304+
<Parent>
305+
<Child name="one" />
306+
</Parent>
307+
</Activity>
308+
</React.StrictMode>,
309+
);
310+
});
311+
312+
expect(log).toEqual([
313+
'Parent rendered',
314+
'Parent rendered',
315+
'Child one rendered',
316+
'Child one rendered',
317+
// single Effect invocation. No double invocation on update.
318+
'Child one unmount',
319+
'Parent unmount',
320+
'Child one mount',
321+
'Parent mount',
322+
]);
323+
324+
log.length = 0;
325+
await act(() => {
326+
ReactNoop.render(
327+
<React.StrictMode>
328+
<Activity mode="visible">
329+
<Parent>
330+
<Child name="one" />
331+
<Child name="two" />
332+
</Parent>
333+
</Activity>
334+
</React.StrictMode>,
335+
);
336+
});
337+
338+
expect(log).toEqual([
339+
'Parent rendered',
340+
'Parent rendered',
341+
'Child one rendered',
342+
'Child one rendered',
343+
'Child two rendered',
344+
'Child two rendered',
345+
// single Effect invocation for existing Components.
346+
'Child one unmount',
347+
'Parent unmount',
348+
'Child one mount',
349+
'Child two mount',
350+
'Parent mount',
351+
// Double Effect invocation for new Component "two"
352+
'Child two unmount',
353+
'Child two mount',
354+
]);
355+
});
233356
});

0 commit comments

Comments
 (0)