Skip to content

Commit

Permalink
Proposed new Suspense layout effect semantics (#21079)
Browse files Browse the repository at this point in the history 
This commit contains a proposed change to layout effect semantics within Suspense subtrees: If a component mounts within a Suspense boundary and is later hidden (because of something else suspending) React will cleanup that component’s layout effects (including React-managed refs).

This change will hopefully fix existing bugs that occur because of things like reading layout in a hidden tree and will also enable a point at which to e.g. pause videos and hide user-managed portals. After the suspended boundary resolves, React will setup the component’s layout effects again (including React-managed refs).

The scenario described above is not common. The useTransition API should ensure that Suspense does not revert to its fallback state after being mounted.

Note that these changes are primarily written in terms of the (as of yet internal) Offscreen API as we intend to provide similar effects semantics within recently shown/hidden Offscreen trees in the future. (More to follow.)

(Note that all changes in this PR are behind a new feature flag, enableSuspenseLayoutEffectSemantics, which is disabled for now.)
  • Loading branch information
Brian Vaughn authored Apr 6, 2021
1 parent 316aa36 commit 7c1ba2b
Show file tree
Hide file tree
Showing 25 changed files with 3,878 additions and 140 deletions.
5 changes: 5 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import {
DidCapture,
Update,
Ref,
RefStatic,
ChildDeletion,
ForceUpdateForLegacySuspense,
StaticMask,
Expand All @@ -83,6 +84,7 @@ import {
enableScopeAPI,
enableCache,
enableLazyContextPropagation,
enableSuspenseLayoutEffectSemantics,
} from 'shared/ReactFeatureFlags';
import invariant from 'shared/invariant';
import shallowEqual from 'shared/shallowEqual';
Expand Down Expand Up @@ -854,6 +856,9 @@ function markRef(current: Fiber | null, workInProgress: Fiber) {
) {
// Schedule a Ref effect
workInProgress.flags |= Ref;
if (enableSuspenseLayoutEffectSemantics) {
workInProgress.flags |= RefStatic;
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import {
DidCapture,
Update,
Ref,
RefStatic,
ChildDeletion,
ForceUpdateForLegacySuspense,
StaticMask,
Expand All @@ -83,6 +84,7 @@ import {
enableScopeAPI,
enableCache,
enableLazyContextPropagation,
enableSuspenseLayoutEffectSemantics,
} from 'shared/ReactFeatureFlags';
import invariant from 'shared/invariant';
import shallowEqual from 'shared/shallowEqual';
Expand Down Expand Up @@ -854,6 +856,9 @@ function markRef(current: Fiber | null, workInProgress: Fiber) {
) {
// Schedule a Ref effect
workInProgress.flags |= Ref;
if (enableSuspenseLayoutEffectSemantics) {
workInProgress.flags |= RefStatic;
}
}
}

Expand Down
45 changes: 32 additions & 13 deletions packages/react-reconciler/src/ReactFiberClassComponent.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,15 @@
import type {Fiber} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane.new';
import type {UpdateQueue} from './ReactUpdateQueue.new';
import type {Flags} from './ReactFiberFlags';

import * as React from 'react';
import {MountLayoutDev, Update, Snapshot} from './ReactFiberFlags';
import {
LayoutStatic,
MountLayoutDev,
Update,
Snapshot,
} from './ReactFiberFlags';
import {
debugRenderPhaseSideEffectsForStrictMode,
disableLegacyContext,
Expand All @@ -21,6 +27,7 @@ import {
warnAboutDeprecatedLifecycles,
enableStrictEffects,
enableLazyContextPropagation,
enableSuspenseLayoutEffectSemantics,
} from 'shared/ReactFeatureFlags';
import ReactStrictModeWarnings from './ReactStrictModeWarnings.new';
import {isMounted} from './ReactFiberTreeReflection';
Expand Down Expand Up @@ -908,16 +915,19 @@ function mountClassInstance(
}

if (typeof instance.componentDidMount === 'function') {
let fiberFlags: Flags = Update;
if (enableSuspenseLayoutEffectSemantics) {
fiberFlags |= LayoutStatic;
}
if (
__DEV__ &&
enableStrictEffects &&
(workInProgress.mode & StrictEffectsMode) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
fiberFlags |= MountLayoutDev;
}
workInProgress.flags |= fiberFlags;
}
}

Expand Down Expand Up @@ -987,16 +997,19 @@ function resumeMountClassInstance(
// If an update was already in progress, we should schedule an Update
// effect even though we're bailing out, so that cWU/cDU are called.
if (typeof instance.componentDidMount === 'function') {
let fiberFlags: Flags = Update;
if (enableSuspenseLayoutEffectSemantics) {
fiberFlags |= LayoutStatic;
}
if (
__DEV__ &&
enableStrictEffects &&
(workInProgress.mode & StrictEffectsMode) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
fiberFlags |= MountLayoutDev;
}
workInProgress.flags |= fiberFlags;
}
return false;
}
Expand Down Expand Up @@ -1039,31 +1052,37 @@ function resumeMountClassInstance(
}
}
if (typeof instance.componentDidMount === 'function') {
let fiberFlags: Flags = Update;
if (enableSuspenseLayoutEffectSemantics) {
fiberFlags |= LayoutStatic;
}
if (
__DEV__ &&
enableStrictEffects &&
(workInProgress.mode & StrictEffectsMode) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
fiberFlags |= MountLayoutDev;
}
workInProgress.flags |= fiberFlags;
}
} else {
// If an update was already in progress, we should schedule an Update
// effect even though we're bailing out, so that cWU/cDU are called.
if (typeof instance.componentDidMount === 'function') {
let fiberFlags: Flags = Update;
if (enableSuspenseLayoutEffectSemantics) {
fiberFlags |= LayoutStatic;
}
if (
__DEV__ &&
enableStrictEffects &&
(workInProgress.mode & StrictEffectsMode) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
fiberFlags |= MountLayoutDev;
}
workInProgress.flags |= fiberFlags;
}

// If shouldComponentUpdate returned false, we should still update the
Expand Down
45 changes: 32 additions & 13 deletions packages/react-reconciler/src/ReactFiberClassComponent.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,15 @@
import type {Fiber} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane.old';
import type {UpdateQueue} from './ReactUpdateQueue.old';
import type {Flags} from './ReactFiberFlags';

import * as React from 'react';
import {MountLayoutDev, Update, Snapshot} from './ReactFiberFlags';
import {
LayoutStatic,
MountLayoutDev,
Update,
Snapshot,
} from './ReactFiberFlags';
import {
debugRenderPhaseSideEffectsForStrictMode,
disableLegacyContext,
Expand All @@ -21,6 +27,7 @@ import {
warnAboutDeprecatedLifecycles,
enableStrictEffects,
enableLazyContextPropagation,
enableSuspenseLayoutEffectSemantics,
} from 'shared/ReactFeatureFlags';
import ReactStrictModeWarnings from './ReactStrictModeWarnings.old';
import {isMounted} from './ReactFiberTreeReflection';
Expand Down Expand Up @@ -908,16 +915,19 @@ function mountClassInstance(
}

if (typeof instance.componentDidMount === 'function') {
let fiberFlags: Flags = Update;
if (enableSuspenseLayoutEffectSemantics) {
fiberFlags |= LayoutStatic;
}
if (
__DEV__ &&
enableStrictEffects &&
(workInProgress.mode & StrictEffectsMode) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
fiberFlags |= MountLayoutDev;
}
workInProgress.flags |= fiberFlags;
}
}

Expand Down Expand Up @@ -987,16 +997,19 @@ function resumeMountClassInstance(
// If an update was already in progress, we should schedule an Update
// effect even though we're bailing out, so that cWU/cDU are called.
if (typeof instance.componentDidMount === 'function') {
let fiberFlags: Flags = Update;
if (enableSuspenseLayoutEffectSemantics) {
fiberFlags |= LayoutStatic;
}
if (
__DEV__ &&
enableStrictEffects &&
(workInProgress.mode & StrictEffectsMode) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
fiberFlags |= MountLayoutDev;
}
workInProgress.flags |= fiberFlags;
}
return false;
}
Expand Down Expand Up @@ -1039,31 +1052,37 @@ function resumeMountClassInstance(
}
}
if (typeof instance.componentDidMount === 'function') {
let fiberFlags: Flags = Update;
if (enableSuspenseLayoutEffectSemantics) {
fiberFlags |= LayoutStatic;
}
if (
__DEV__ &&
enableStrictEffects &&
(workInProgress.mode & StrictEffectsMode) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
fiberFlags |= MountLayoutDev;
}
workInProgress.flags |= fiberFlags;
}
} else {
// If an update was already in progress, we should schedule an Update
// effect even though we're bailing out, so that cWU/cDU are called.
if (typeof instance.componentDidMount === 'function') {
let fiberFlags: Flags = Update;
if (enableSuspenseLayoutEffectSemantics) {
fiberFlags |= LayoutStatic;
}
if (
__DEV__ &&
enableStrictEffects &&
(workInProgress.mode & StrictEffectsMode) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
fiberFlags |= MountLayoutDev;
}
workInProgress.flags |= fiberFlags;
}

// If shouldComponentUpdate returned false, we should still update the
Expand Down
Loading

0 comments on commit 7c1ba2b

Please sign in to comment.