Skip to content

Commit 8bff898

Browse files
gaearonTsdevendra1
gaearon
and
Tsdevendra1
authored
Don't bailout after Suspending in Legacy Mode (facebook#19216)
* Add a failing test for legacy Suspense blocking context updates in memo * Add more test case coverage for variations of facebook#17356 * Don't bailout after Suspending in Legacy Mode Co-authored-by: Tharuka Devendra <tsdevendra1@gmail.com>
1 parent f4097c1 commit 8bff898

File tree

6 files changed

+507
-29
lines changed

6 files changed

+507
-29
lines changed

‎packages/react-reconciler/src/ReactFiberBeginWork.new.js

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ import {
6363
Update,
6464
Ref,
6565
Deletion,
66+
ForceUpdateForLegacySuspense,
6667
} from './ReactSideEffectTags';
6768
import ReactSharedInternals from 'shared/ReactSharedInternals';
6869
import {
@@ -548,6 +549,13 @@ function updateSimpleMemoComponent(
548549
workInProgress,
549550
renderLanes,
550551
);
552+
} else if (
553+
(current.effectTag & ForceUpdateForLegacySuspense) !==
554+
NoEffect
555+
) {
556+
// This is a special case that only exists for legacy mode.
557+
// See https://github.com/facebook/react/pull/19216.
558+
didReceiveUpdate = true;
551559
}
552560
}
553561
}
@@ -3263,11 +3271,17 @@ function beginWork(
32633271
}
32643272
return bailoutOnAlreadyFinishedWork(current, workInProgress, renderLanes);
32653273
} else {
3266-
// An update was scheduled on this fiber, but there are no new props
3267-
// nor legacy context. Set this to false. If an update queue or context
3268-
// consumer produces a changed value, it will set this to true. Otherwise,
3269-
// the component will assume the children have not changed and bail out.
3270-
didReceiveUpdate = false;
3274+
if ((current.effectTag & ForceUpdateForLegacySuspense) !== NoEffect) {
3275+
// This is a special case that only exists for legacy mode.
3276+
// See https://github.com/facebook/react/pull/19216.
3277+
didReceiveUpdate = true;
3278+
} else {
3279+
// An update was scheduled on this fiber, but there are no new props
3280+
// nor legacy context. Set this to false. If an update queue or context
3281+
// consumer produces a changed value, it will set this to true. Otherwise,
3282+
// the component will assume the children have not changed and bail out.
3283+
didReceiveUpdate = false;
3284+
}
32713285
}
32723286
} else {
32733287
didReceiveUpdate = false;

‎packages/react-reconciler/src/ReactFiberBeginWork.old.js

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ import {
6363
Update,
6464
Ref,
6565
Deletion,
66+
ForceUpdateForLegacySuspense,
6667
} from './ReactSideEffectTags';
6768
import ReactSharedInternals from 'shared/ReactSharedInternals';
6869
import {
@@ -548,6 +549,13 @@ function updateSimpleMemoComponent(
548549
workInProgress,
549550
renderLanes,
550551
);
552+
} else if (
553+
(current.effectTag & ForceUpdateForLegacySuspense) !==
554+
NoEffect
555+
) {
556+
// This is a special case that only exists for legacy mode.
557+
// See https://github.com/facebook/react/pull/19216.
558+
didReceiveUpdate = true;
551559
}
552560
}
553561
}
@@ -3263,11 +3271,17 @@ function beginWork(
32633271
}
32643272
return bailoutOnAlreadyFinishedWork(current, workInProgress, renderLanes);
32653273
} else {
3266-
// An update was scheduled on this fiber, but there are no new props
3267-
// nor legacy context. Set this to false. If an update queue or context
3268-
// consumer produces a changed value, it will set this to true. Otherwise,
3269-
// the component will assume the children have not changed and bail out.
3270-
didReceiveUpdate = false;
3274+
if ((current.effectTag & ForceUpdateForLegacySuspense) !== NoEffect) {
3275+
// This is a special case that only exists for legacy mode.
3276+
// See https://github.com/facebook/react/pull/19216.
3277+
didReceiveUpdate = true;
3278+
} else {
3279+
// An update was scheduled on this fiber, but there are no new props
3280+
// nor legacy context. Set this to false. If an update queue or context
3281+
// consumer produces a changed value, it will set this to true. Otherwise,
3282+
// the component will assume the children have not changed and bail out.
3283+
didReceiveUpdate = false;
3284+
}
32713285
}
32723286
} else {
32733287
didReceiveUpdate = false;

‎packages/react-reconciler/src/ReactFiberThrow.new.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
NoEffect,
2929
ShouldCapture,
3030
LifecycleEffectMask,
31+
ForceUpdateForLegacySuspense,
3132
} from './ReactSideEffectTags';
3233
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.new';
3334
import {NoMode, BlockingMode} from './ReactTypeOfMode';
@@ -238,6 +239,7 @@ function throwException(
238239
// should *not* suspend the commit.
239240
if ((workInProgress.mode & BlockingMode) === NoMode) {
240241
workInProgress.effectTag |= DidCapture;
242+
sourceFiber.effectTag |= ForceUpdateForLegacySuspense;
241243

242244
// We're going to commit this fiber even though it didn't complete.
243245
// But we shouldn't call any lifecycle methods or callbacks. Remove

‎packages/react-reconciler/src/ReactFiberThrow.old.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
NoEffect,
2929
ShouldCapture,
3030
LifecycleEffectMask,
31+
ForceUpdateForLegacySuspense,
3132
} from './ReactSideEffectTags';
3233
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.old';
3334
import {NoMode, BlockingMode, DebugTracingMode} from './ReactTypeOfMode';
@@ -249,6 +250,7 @@ function throwException(
249250
// should *not* suspend the commit.
250251
if ((workInProgress.mode & BlockingMode) === NoMode) {
251252
workInProgress.effectTag |= DidCapture;
253+
sourceFiber.effectTag |= ForceUpdateForLegacySuspense;
252254

253255
// We're going to commit this fiber even though it didn't complete.
254256
// But we shouldn't call any lifecycle methods or callbacks. Remove

‎packages/react-reconciler/src/ReactSideEffectTags.js

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,29 +10,31 @@
1010
export type SideEffectTag = number;
1111

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

1616
// You can change the rest (and add more).
17-
export const Placement = /* */ 0b00000000000010;
18-
export const Update = /* */ 0b00000000000100;
19-
export const PlacementAndUpdate = /* */ 0b00000000000110;
20-
export const Deletion = /* */ 0b00000000001000;
21-
export const ContentReset = /* */ 0b00000000010000;
22-
export const Callback = /* */ 0b00000000100000;
23-
export const DidCapture = /* */ 0b00000001000000;
24-
export const Ref = /* */ 0b00000010000000;
25-
export const Snapshot = /* */ 0b00000100000000;
26-
export const Passive = /* */ 0b00001000000000;
27-
export const PassiveUnmountPendingDev = /* */ 0b10000000000000;
28-
export const Hydrating = /* */ 0b00010000000000;
29-
export const HydratingAndUpdate = /* */ 0b00010000000100;
17+
export const Placement = /* */ 0b000000000000010;
18+
export const Update = /* */ 0b000000000000100;
19+
export const PlacementAndUpdate = /* */ 0b000000000000110;
20+
export const Deletion = /* */ 0b000000000001000;
21+
export const ContentReset = /* */ 0b000000000010000;
22+
export const Callback = /* */ 0b000000000100000;
23+
export const DidCapture = /* */ 0b000000001000000;
24+
export const Ref = /* */ 0b000000010000000;
25+
export const Snapshot = /* */ 0b000000100000000;
26+
export const Passive = /* */ 0b000001000000000;
27+
export const PassiveUnmountPendingDev = /* */ 0b010000000000000;
28+
export const Hydrating = /* */ 0b000010000000000;
29+
export const HydratingAndUpdate = /* */ 0b000010000000100;
3030

3131
// Passive & Update & Callback & Ref & Snapshot
32-
export const LifecycleEffectMask = /* */ 0b00001110100100;
32+
export const LifecycleEffectMask = /* */ 0b000001110100100;
3333

3434
// Union of all host effects
35-
export const HostEffectMask = /* */ 0b00011111111111;
35+
export const HostEffectMask = /* */ 0b000011111111111;
3636

37-
export const Incomplete = /* */ 0b00100000000000;
38-
export const ShouldCapture = /* */ 0b01000000000000;
37+
// These are not really side effects, but we still reuse this field.
38+
export const Incomplete = /* */ 0b000100000000000;
39+
export const ShouldCapture = /* */ 0b001000000000000;
40+
export const ForceUpdateForLegacySuspense = /* */ 0b100000000000000;

0 commit comments

Comments
 (0)