Skip to content

Commit

Permalink
Don't bailout after Suspending in Legacy Mode
Browse files Browse the repository at this point in the history
  • Loading branch information
gaearon committed Jun 30, 2020
1 parent 5408be1 commit 77ec289
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 30 deletions.
24 changes: 19 additions & 5 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
Expand Up @@ -63,6 +63,7 @@ import {
Update,
Ref,
Deletion,
ForceUpdateForLegacySuspense,
} from './ReactSideEffectTags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import {
Expand Down Expand Up @@ -548,6 +549,13 @@ function updateSimpleMemoComponent(
workInProgress,
renderLanes,
);
} else if (
(current.effectTag & ForceUpdateForLegacySuspense) !==
NoEffect
) {
// This is a special case that only exists for legacy mode.
// See https://github.com/facebook/react/pull/19216.
didReceiveUpdate = true;
}
}
}
Expand Down Expand Up @@ -3253,11 +3261,17 @@ function beginWork(
}
return bailoutOnAlreadyFinishedWork(current, workInProgress, renderLanes);
} else {
// An update was scheduled on this fiber, but there are no new props
// nor legacy context. Set this to false. If an update queue or context
// consumer produces a changed value, it will set this to true. Otherwise,
// the component will assume the children have not changed and bail out.
didReceiveUpdate = false;
if ((current.effectTag & ForceUpdateForLegacySuspense) !== NoEffect) {
// This is a special case that only exists for legacy mode.
// See https://github.com/facebook/react/pull/19216.
didReceiveUpdate = true;
} else {
// An update was scheduled on this fiber, but there are no new props
// nor legacy context. Set this to false. If an update queue or context
// consumer produces a changed value, it will set this to true. Otherwise,
// the component will assume the children have not changed and bail out.
didReceiveUpdate = false;
}
}
} else {
didReceiveUpdate = false;
Expand Down
24 changes: 19 additions & 5 deletions packages/react-reconciler/src/ReactFiberBeginWork.old.js
Expand Up @@ -63,6 +63,7 @@ import {
Update,
Ref,
Deletion,
ForceUpdateForLegacySuspense,
} from './ReactSideEffectTags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import {
Expand Down Expand Up @@ -548,6 +549,13 @@ function updateSimpleMemoComponent(
workInProgress,
renderLanes,
);
} else if (
(current.effectTag & ForceUpdateForLegacySuspense) !==
NoEffect
) {
// This is a special case that only exists for legacy mode.
// See https://github.com/facebook/react/pull/19216.
didReceiveUpdate = true;
}
}
}
Expand Down Expand Up @@ -3253,11 +3261,17 @@ function beginWork(
}
return bailoutOnAlreadyFinishedWork(current, workInProgress, renderLanes);
} else {
// An update was scheduled on this fiber, but there are no new props
// nor legacy context. Set this to false. If an update queue or context
// consumer produces a changed value, it will set this to true. Otherwise,
// the component will assume the children have not changed and bail out.
didReceiveUpdate = false;
if ((current.effectTag & ForceUpdateForLegacySuspense) !== NoEffect) {
// This is a special case that only exists for legacy mode.
// See https://github.com/facebook/react/pull/19216.
didReceiveUpdate = true;
} else {
// An update was scheduled on this fiber, but there are no new props
// nor legacy context. Set this to false. If an update queue or context
// consumer produces a changed value, it will set this to true. Otherwise,
// the component will assume the children have not changed and bail out.
didReceiveUpdate = false;
}
}
} else {
didReceiveUpdate = false;
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberThrow.new.js
Expand Up @@ -28,6 +28,7 @@ import {
NoEffect,
ShouldCapture,
LifecycleEffectMask,
ForceUpdateForLegacySuspense,
} from './ReactSideEffectTags';
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.new';
import {NoMode, BlockingMode} from './ReactTypeOfMode';
Expand Down Expand Up @@ -238,6 +239,7 @@ function throwException(
// should *not* suspend the commit.
if ((workInProgress.mode & BlockingMode) === NoMode) {
workInProgress.effectTag |= DidCapture;
sourceFiber.effectTag |= ForceUpdateForLegacySuspense;

// We're going to commit this fiber even though it didn't complete.
// But we shouldn't call any lifecycle methods or callbacks. Remove
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberThrow.old.js
Expand Up @@ -28,6 +28,7 @@ import {
NoEffect,
ShouldCapture,
LifecycleEffectMask,
ForceUpdateForLegacySuspense,
} from './ReactSideEffectTags';
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.old';
import {NoMode, BlockingMode, DebugTracingMode} from './ReactTypeOfMode';
Expand Down Expand Up @@ -249,6 +250,7 @@ function throwException(
// should *not* suspend the commit.
if ((workInProgress.mode & BlockingMode) === NoMode) {
workInProgress.effectTag |= DidCapture;
sourceFiber.effectTag |= ForceUpdateForLegacySuspense;

// We're going to commit this fiber even though it didn't complete.
// But we shouldn't call any lifecycle methods or callbacks. Remove
Expand Down
40 changes: 21 additions & 19 deletions packages/react-reconciler/src/ReactSideEffectTags.js
Expand Up @@ -10,29 +10,31 @@
export type SideEffectTag = number;

// Don't change these two values. They're used by React Dev Tools.
export const NoEffect = /* */ 0b00000000000000;
export const PerformedWork = /* */ 0b00000000000001;
export const NoEffect = /* */ 0b000000000000000;
export const PerformedWork = /* */ 0b000000000000001;

// You can change the rest (and add more).
export const Placement = /* */ 0b00000000000010;
export const Update = /* */ 0b00000000000100;
export const PlacementAndUpdate = /* */ 0b00000000000110;
export const Deletion = /* */ 0b00000000001000;
export const ContentReset = /* */ 0b00000000010000;
export const Callback = /* */ 0b00000000100000;
export const DidCapture = /* */ 0b00000001000000;
export const Ref = /* */ 0b00000010000000;
export const Snapshot = /* */ 0b00000100000000;
export const Passive = /* */ 0b00001000000000;
export const PassiveUnmountPendingDev = /* */ 0b10000000000000;
export const Hydrating = /* */ 0b00010000000000;
export const HydratingAndUpdate = /* */ 0b00010000000100;
export const Placement = /* */ 0b000000000000010;
export const Update = /* */ 0b000000000000100;
export const PlacementAndUpdate = /* */ 0b000000000000110;
export const Deletion = /* */ 0b000000000001000;
export const ContentReset = /* */ 0b000000000010000;
export const Callback = /* */ 0b000000000100000;
export const DidCapture = /* */ 0b000000001000000;
export const Ref = /* */ 0b000000010000000;
export const Snapshot = /* */ 0b000000100000000;
export const Passive = /* */ 0b000001000000000;
export const PassiveUnmountPendingDev = /* */ 0b010000000000000;
export const Hydrating = /* */ 0b000010000000000;
export const HydratingAndUpdate = /* */ 0b000010000000100;

// Passive & Update & Callback & Ref & Snapshot
export const LifecycleEffectMask = /* */ 0b00001110100100;
export const LifecycleEffectMask = /* */ 0b000001110100100;

// Union of all host effects
export const HostEffectMask = /* */ 0b00011111111111;
export const HostEffectMask = /* */ 0b000011111111111;

export const Incomplete = /* */ 0b00100000000000;
export const ShouldCapture = /* */ 0b01000000000000;
// These are not really side effects, but we still reuse this field.
export const Incomplete = /* */ 0b000100000000000;
export const ShouldCapture = /* */ 0b001000000000000;
export const ForceUpdateForLegacySuspense = /* */ 0b100000000000000;
Expand Up @@ -803,7 +803,7 @@ describe('ReactSuspense', () => {

const ValueContext = createContext(null);

let MemoizedChild = forwardRef(() => {
const MemoizedChild = forwardRef(() => {
const text = useContext(ValueContext);
try {
TextResource.read([text, 1000]);
Expand Down

0 comments on commit 77ec289

Please sign in to comment.