Skip to content

Commit 25eb72c

Browse files
committed
Land enableTransitionEntanglement changes
Leaving the flag though because I plan to reuse it for additional, similar changes.
1 parent b5bac18 commit 25eb72c

File tree

3 files changed

+36
-105
lines changed

3 files changed

+36
-105
lines changed

packages/react-reconciler/src/ReactFiberLane.new.js

Lines changed: 16 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,7 @@ export type Lane = number;
3636
export type LaneMap<T> = Array<T>;
3737

3838
import invariant from 'shared/invariant';
39-
import {
40-
enableCache,
41-
enableTransitionEntanglement,
42-
} from 'shared/ReactFeatureFlags';
39+
import {enableCache} from 'shared/ReactFeatureFlags';
4340

4441
import {
4542
ImmediatePriority as ImmediateSchedulerPriority,
@@ -309,15 +306,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
309306
return NoLanes;
310307
}
311308

312-
if (enableTransitionEntanglement) {
313-
// We don't need to do anything extra here, because we apply per-lane
314-
// transition entanglement in the entanglement loop below.
315-
} else {
316-
// If there are higher priority lanes, we'll include them even if they
317-
// are suspended.
318-
nextLanes = pendingLanes & getEqualOrHigherPriorityLanes(nextLanes);
319-
}
320-
321309
// If we're already in the middle of a render, switching lanes will interrupt
322310
// it and we'll lose our progress. We should only do this if the new lanes are
323311
// higher priority.
@@ -676,39 +664,21 @@ export function markRootUpdated(
676664
) {
677665
root.pendingLanes |= updateLane;
678666

679-
// TODO: Theoretically, any update to any lane can unblock any other lane. But
680-
// it's not practical to try every single possible combination. We need a
681-
// heuristic to decide which lanes to attempt to render, and in which batches.
682-
// For now, we use the same heuristic as in the old ExpirationTimes model:
683-
// retry any lane at equal or lower priority, but don't try updates at higher
684-
// priority without also including the lower priority updates. This works well
685-
// when considering updates across different priority levels, but isn't
686-
// sufficient for updates within the same priority, since we want to treat
687-
// those updates as parallel.
688-
689-
// Unsuspend any update at equal or lower priority.
690-
const higherPriorityLanes = updateLane - 1; // Turns 0b1000 into 0b0111
691-
692-
if (enableTransitionEntanglement) {
693-
// If there are any suspended transitions, it's possible this new update
694-
// could unblock them. Clear the suspended lanes so that we can try rendering
695-
// them again.
696-
//
697-
// TODO: We really only need to unsuspend only lanes that are in the
698-
// `subtreeLanes` of the updated fiber, or the update lanes of the return
699-
// path. This would exclude suspended updates in an unrelated sibling tree,
700-
// since there's no way for this update to unblock it.
701-
//
702-
// We don't do this if the incoming update is idle, because we never process
703-
// idle updates until after all the regular updates have finished; there's no
704-
// way it could unblock a transition.
705-
if ((updateLane & IdleLanes) === NoLanes) {
706-
root.suspendedLanes = NoLanes;
707-
root.pingedLanes = NoLanes;
708-
}
709-
} else {
710-
root.suspendedLanes &= higherPriorityLanes;
711-
root.pingedLanes &= higherPriorityLanes;
667+
// If there are any suspended transitions, it's possible this new update
668+
// could unblock them. Clear the suspended lanes so that we can try rendering
669+
// them again.
670+
//
671+
// TODO: We really only need to unsuspend only lanes that are in the
672+
// `subtreeLanes` of the updated fiber, or the update lanes of the return
673+
// path. This would exclude suspended updates in an unrelated sibling tree,
674+
// since there's no way for this update to unblock it.
675+
//
676+
// We don't do this if the incoming update is idle, because we never process
677+
// idle updates until after all the regular updates have finished; there's no
678+
// way it could unblock a transition.
679+
if ((updateLane & IdleLanes) === NoLanes) {
680+
root.suspendedLanes = NoLanes;
681+
root.pingedLanes = NoLanes;
712682
}
713683

714684
const eventTimes = root.eventTimes;

packages/react-reconciler/src/ReactFiberLane.old.js

Lines changed: 16 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,7 @@ export type Lane = number;
3636
export type LaneMap<T> = Array<T>;
3737

3838
import invariant from 'shared/invariant';
39-
import {
40-
enableCache,
41-
enableTransitionEntanglement,
42-
} from 'shared/ReactFeatureFlags';
39+
import {enableCache} from 'shared/ReactFeatureFlags';
4340

4441
import {
4542
ImmediatePriority as ImmediateSchedulerPriority,
@@ -309,15 +306,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
309306
return NoLanes;
310307
}
311308

312-
if (enableTransitionEntanglement) {
313-
// We don't need to do anything extra here, because we apply per-lane
314-
// transition entanglement in the entanglement loop below.
315-
} else {
316-
// If there are higher priority lanes, we'll include them even if they
317-
// are suspended.
318-
nextLanes = pendingLanes & getEqualOrHigherPriorityLanes(nextLanes);
319-
}
320-
321309
// If we're already in the middle of a render, switching lanes will interrupt
322310
// it and we'll lose our progress. We should only do this if the new lanes are
323311
// higher priority.
@@ -676,39 +664,21 @@ export function markRootUpdated(
676664
) {
677665
root.pendingLanes |= updateLane;
678666

679-
// TODO: Theoretically, any update to any lane can unblock any other lane. But
680-
// it's not practical to try every single possible combination. We need a
681-
// heuristic to decide which lanes to attempt to render, and in which batches.
682-
// For now, we use the same heuristic as in the old ExpirationTimes model:
683-
// retry any lane at equal or lower priority, but don't try updates at higher
684-
// priority without also including the lower priority updates. This works well
685-
// when considering updates across different priority levels, but isn't
686-
// sufficient for updates within the same priority, since we want to treat
687-
// those updates as parallel.
688-
689-
// Unsuspend any update at equal or lower priority.
690-
const higherPriorityLanes = updateLane - 1; // Turns 0b1000 into 0b0111
691-
692-
if (enableTransitionEntanglement) {
693-
// If there are any suspended transitions, it's possible this new update
694-
// could unblock them. Clear the suspended lanes so that we can try rendering
695-
// them again.
696-
//
697-
// TODO: We really only need to unsuspend only lanes that are in the
698-
// `subtreeLanes` of the updated fiber, or the update lanes of the return
699-
// path. This would exclude suspended updates in an unrelated sibling tree,
700-
// since there's no way for this update to unblock it.
701-
//
702-
// We don't do this if the incoming update is idle, because we never process
703-
// idle updates until after all the regular updates have finished; there's no
704-
// way it could unblock a transition.
705-
if ((updateLane & IdleLanes) === NoLanes) {
706-
root.suspendedLanes = NoLanes;
707-
root.pingedLanes = NoLanes;
708-
}
709-
} else {
710-
root.suspendedLanes &= higherPriorityLanes;
711-
root.pingedLanes &= higherPriorityLanes;
667+
// If there are any suspended transitions, it's possible this new update
668+
// could unblock them. Clear the suspended lanes so that we can try rendering
669+
// them again.
670+
//
671+
// TODO: We really only need to unsuspend only lanes that are in the
672+
// `subtreeLanes` of the updated fiber, or the update lanes of the return
673+
// path. This would exclude suspended updates in an unrelated sibling tree,
674+
// since there's no way for this update to unblock it.
675+
//
676+
// We don't do this if the incoming update is idle, because we never process
677+
// idle updates until after all the regular updates have finished; there's no
678+
// way it could unblock a transition.
679+
if ((updateLane & IdleLanes) === NoLanes) {
680+
root.suspendedLanes = NoLanes;
681+
root.pingedLanes = NoLanes;
712682
}
713683

714684
const eventTimes = root.eventTimes;

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -733,19 +733,10 @@ describe('ReactExpiration', () => {
733733
// Both normal pri updates should have expired.
734734
expect(Scheduler).toFlushExpired([
735735
'Sibling',
736-
gate(flags => flags.enableTransitionEntanglement)
737-
? // Notice that the high pri update didn't flush yet. Expiring one lane
738-
// doesn't affect other lanes. (Unless they are intentionally
739-
// entangled, like we do for overlapping transitions that affect the
740-
// same state.)
741-
'High pri: 0'
742-
: // In the current implementation, once we pick the next lanes to work
743-
// on, we entangle it with all pending at equal or higher priority.
744-
// We could feasibly change this heuristic so that the high pri
745-
// update doesn't render until after the expired updates have
746-
// finished. But the important thing in this test is that the normal
747-
// updates expired.
748-
'High pri: 1',
736+
// Notice that the high pri update didn't flush yet. Expiring one lane
737+
// doesn't affect other lanes. (Unless they are intentionally entangled,
738+
// like we do for overlapping transitions that affect the same state.)
739+
'High pri: 0',
749740
'Normal pri: 2',
750741
'Sibling',
751742
]);

0 commit comments

Comments
 (0)