Skip to content

Commit 91bf47c

Browse files
committed
Call cleanup of insertion effects when hidden
Insertion effects do not unmount when a subtree goes offscreen, this means we still have to traverse the subtree in that case. Likely also fixes #26670
1 parent 206df66 commit 91bf47c

11 files changed

+50
-1
lines changed

packages/react-reconciler/src/ReactFiberCommitWork.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import type {
4040
import {
4141
alwaysThrottleRetries,
4242
enableCreateEventHandleAPI,
43+
enableHiddenSubtreeInsertionEffectCleanup,
4344
enablePersistedModeClonedFlag,
4445
enableProfilerTimer,
4546
enableProfilerCommitHooks,
@@ -1324,7 +1325,10 @@ function commitDeletionEffectsOnFiber(
13241325
case ForwardRef:
13251326
case MemoComponent:
13261327
case SimpleMemoComponent: {
1327-
if (!offscreenSubtreeWasHidden) {
1328+
if (
1329+
enableHiddenSubtreeInsertionEffectCleanup ||
1330+
!offscreenSubtreeWasHidden
1331+
) {
13281332
const updateQueue: FunctionComponentUpdateQueue | null =
13291333
(deletedFiber.updateQueue: any);
13301334
if (updateQueue !== null) {

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2872,6 +2872,38 @@ describe('ReactHooksWithNoopRenderer', () => {
28722872
});
28732873
});
28742874

2875+
// @gate enableActivity
2876+
it('runs insertion effect cleanup when unmounting in Offscreen state', async () => {
2877+
function Logger(props) {
2878+
useInsertionEffect(() => {
2879+
Scheduler.log(`create`);
2880+
return () => {
2881+
Scheduler.log(`destroy`);
2882+
};
2883+
}, []);
2884+
return null;
2885+
}
2886+
2887+
const Activity = React.unstable_Activity;
2888+
await act(async () => {
2889+
ReactNoop.render(
2890+
<Activity mode="hidden">
2891+
<Logger name="hidden" />
2892+
</Activity>,
2893+
);
2894+
await waitForAll(['create']);
2895+
});
2896+
2897+
await act(async () => {
2898+
ReactNoop.render(null);
2899+
await waitForAll(
2900+
gate(flags => flags.enableHiddenSubtreeInsertionEffectCleanup)
2901+
? ['destroy']
2902+
: [],
2903+
);
2904+
});
2905+
});
2906+
28752907
it('assumes insertion effect destroy function is either a function or undefined', async () => {
28762908
function App(props) {
28772909
useInsertionEffect(() => {

packages/shared/ReactFeatureFlags.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,11 @@ export const transitionLaneExpirationMs = 5000;
172172
// Renames the internal symbol for elements since they have changed signature/constructor
173173
export const renameElementSymbol = true;
174174

175+
/**
176+
* Enables a fix to run insertion effect cleanup on hidden subtrees.
177+
*/
178+
export const enableHiddenSubtreeInsertionEffectCleanup = true;
179+
175180
/**
176181
* Removes legacy style context defined using static `contextTypes` and consumed with static `childContextTypes`.
177182
*/

packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
export const alwaysThrottleRetries = __VARIANT__;
2121
export const enableAddPropertiesFastPath = __VARIANT__;
2222
export const enableObjectFiber = __VARIANT__;
23+
export const enableHiddenSubtreeInsertionEffectCleanup = __VARIANT__;
2324
export const enablePersistedModeClonedFlag = __VARIANT__;
2425
export const enableShallowPropDiffing = __VARIANT__;
2526
export const passChildrenWhenCloningPersistedNodes = __VARIANT__;

packages/shared/forks/ReactFeatureFlags.native-fb.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export const {
2222
alwaysThrottleRetries,
2323
enableAddPropertiesFastPath,
2424
enableFabricCompleteRootInCommitPhase,
25+
enableHiddenSubtreeInsertionEffectCleanup,
2526
enableObjectFiber,
2627
enablePersistedModeClonedFlag,
2728
enableShallowPropDiffing,

packages/shared/forks/ReactFeatureFlags.native-oss.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ export const enableFizzExternalRuntime = true;
5050
export const enableFlightReadableStream = true;
5151
export const enableGetInspectorDataForInstanceInProduction = false;
5252
export const enableHalt = false;
53+
export const enableHiddenSubtreeInsertionEffectCleanup = false;
5354
export const enableInfiniteRenderLoopDetection = true;
5455
export const enableLazyContextPropagation = false;
5556
export const enableContextProfiling = false;

packages/shared/forks/ReactFeatureFlags.test-renderer.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export const enableLegacyFBSupport = false;
4646
export const enableFilterEmptyStringAttributesDOM = true;
4747
export const enableGetInspectorDataForInstanceInProduction = false;
4848
export const enableFabricCompleteRootInCommitPhase = false;
49+
export const enableHiddenSubtreeInsertionEffectCleanup = false;
4950

5051
export const enableRetryLaneExpiration = false;
5152
export const retryLaneExpirationMs = 5000;

packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export const enableHalt = false;
4444
export const enableInfiniteRenderLoopDetection = true;
4545
export const enableLazyContextPropagation = false;
4646
export const enableContextProfiling = false;
47+
export const enableHiddenSubtreeInsertionEffectCleanup = true;
4748
export const enableLegacyCache = false;
4849
export const enableLegacyFBSupport = false;
4950
export const enableLegacyHidden = false;

packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export const enableFilterEmptyStringAttributesDOM = true;
4949
export const enableGetInspectorDataForInstanceInProduction = false;
5050
export const enableRenderableContext = false;
5151
export const enableFabricCompleteRootInCommitPhase = false;
52+
export const enableHiddenSubtreeInsertionEffectCleanup = true;
5253

5354
export const enableRetryLaneExpiration = false;
5455
export const retryLaneExpirationMs = 5000;

packages/shared/forks/ReactFeatureFlags.www-dynamic.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
2121
export const enableAddPropertiesFastPath = __VARIANT__;
2222
export const enableDeferRootSchedulingToMicrotask = __VARIANT__;
2323
export const enableDO_NOT_USE_disableStrictPassiveEffect = __VARIANT__;
24+
export const enableHiddenSubtreeInsertionEffectCleanup = __VARIANT__;
2425
export const enableNoCloningMemoCache = __VARIANT__;
2526
export const enableObjectFiber = __VARIANT__;
2627
export const enableRenderableContext = __VARIANT__;

0 commit comments

Comments
 (0)