Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call cleanup of insertion effects when hidden #30954

Merged
merged 7 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import type {
import {
alwaysThrottleRetries,
enableCreateEventHandleAPI,
enableHiddenSubtreeInsertionEffectCleanup,
enablePersistedModeClonedFlag,
enableProfilerTimer,
enableProfilerCommitHooks,
Expand Down Expand Up @@ -1324,7 +1325,10 @@ function commitDeletionEffectsOnFiber(
case ForwardRef:
case MemoComponent:
case SimpleMemoComponent: {
if (!offscreenSubtreeWasHidden) {
if (
enableHiddenSubtreeInsertionEffectCleanup ||
!offscreenSubtreeWasHidden
) {
const updateQueue: FunctionComponentUpdateQueue | null =
(deletedFiber.updateQueue: any);
if (updateQueue !== null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2872,6 +2872,38 @@ describe('ReactHooksWithNoopRenderer', () => {
});
});

// @gate enableActivity
it('runs insertion effect cleanup when unmounting in Offscreen state', async () => {
function Logger(props) {
useInsertionEffect(() => {
Scheduler.log(`create`);
return () => {
Scheduler.log(`destroy`);
};
}, []);
return null;
}

const Activity = React.unstable_Activity;
await act(async () => {
ReactNoop.render(
<Activity mode="hidden">
<Logger name="hidden" />
</Activity>,
);
await waitForAll(['create']);
});

await act(async () => {
ReactNoop.render(null);
await waitForAll(
gate(flags => flags.enableHiddenSubtreeInsertionEffectCleanup)
? ['destroy']
: [],
);
});
});

it('assumes insertion effect destroy function is either a function or undefined', async () => {
function App(props) {
useInsertionEffect(() => {
Expand Down
5 changes: 5 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@ export const transitionLaneExpirationMs = 5000;
// Renames the internal symbol for elements since they have changed signature/constructor
export const renameElementSymbol = true;

/**
* Enables a fix to run insertion effect cleanup on hidden subtrees.
*/
export const enableHiddenSubtreeInsertionEffectCleanup = true;

/**
* Removes legacy style context defined using static `contextTypes` and consumed with static `childContextTypes`.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
export const alwaysThrottleRetries = __VARIANT__;
export const enableAddPropertiesFastPath = __VARIANT__;
export const enableObjectFiber = __VARIANT__;
export const enableHiddenSubtreeInsertionEffectCleanup = __VARIANT__;
export const enablePersistedModeClonedFlag = __VARIANT__;
export const enableShallowPropDiffing = __VARIANT__;
export const passChildrenWhenCloningPersistedNodes = __VARIANT__;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const {
alwaysThrottleRetries,
enableAddPropertiesFastPath,
enableFabricCompleteRootInCommitPhase,
enableHiddenSubtreeInsertionEffectCleanup,
enableObjectFiber,
enablePersistedModeClonedFlag,
enableShallowPropDiffing,
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const enableFizzExternalRuntime = true;
export const enableFlightReadableStream = true;
export const enableGetInspectorDataForInstanceInProduction = false;
export const enableHalt = false;
export const enableHiddenSubtreeInsertionEffectCleanup = false;
export const enableInfiniteRenderLoopDetection = true;
export const enableLazyContextPropagation = false;
export const enableContextProfiling = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = true;
export const enableGetInspectorDataForInstanceInProduction = false;
export const enableFabricCompleteRootInCommitPhase = false;
export const enableHiddenSubtreeInsertionEffectCleanup = false;

export const enableRetryLaneExpiration = false;
export const retryLaneExpirationMs = 5000;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export const enableHalt = false;
export const enableInfiniteRenderLoopDetection = true;
export const enableLazyContextPropagation = false;
export const enableContextProfiling = false;
export const enableHiddenSubtreeInsertionEffectCleanup = true;
export const enableLegacyCache = false;
export const enableLegacyFBSupport = false;
export const enableLegacyHidden = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const enableFilterEmptyStringAttributesDOM = true;
export const enableGetInspectorDataForInstanceInProduction = false;
export const enableRenderableContext = false;
export const enableFabricCompleteRootInCommitPhase = false;
export const enableHiddenSubtreeInsertionEffectCleanup = true;

export const enableRetryLaneExpiration = false;
export const retryLaneExpirationMs = 5000;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
export const enableAddPropertiesFastPath = __VARIANT__;
export const enableDeferRootSchedulingToMicrotask = __VARIANT__;
export const enableDO_NOT_USE_disableStrictPassiveEffect = __VARIANT__;
export const enableHiddenSubtreeInsertionEffectCleanup = __VARIANT__;
export const enableNoCloningMemoCache = __VARIANT__;
export const enableObjectFiber = __VARIANT__;
export const enableRenderableContext = __VARIANT__;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const {
enableRetryLaneExpiration,
enableTransitionTracing,
enableTrustedTypesIntegration,
enableHiddenSubtreeInsertionEffectCleanup,
favorSafetyOverHydrationPerf,
renameElementSymbol,
retryLaneExpirationMs,
Expand Down
Loading