diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js index 351035d0c341..4570c10a62b4 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -36,7 +36,10 @@ export type Lane = number; export type LaneMap = Array; import invariant from 'shared/invariant'; -import {enableCache} from 'shared/ReactFeatureFlags'; +import { + enableCache, + enableTransitionEntanglement, +} from 'shared/ReactFeatureFlags'; import { ImmediatePriority as ImmediateSchedulerPriority, @@ -306,9 +309,15 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { return NoLanes; } - // If there are higher priority lanes, we'll include them even if they - // are suspended. - nextLanes = pendingLanes & getEqualOrHigherPriorityLanes(nextLanes); + if (enableTransitionEntanglement) { + // We don't need to include higher priority lanes, because in this + // experiment we always unsuspend all transitions whenever we receive + // an update. + } else { + // If there are higher priority lanes, we'll include them even if they + // are suspended. + nextLanes = pendingLanes & getEqualOrHigherPriorityLanes(nextLanes); + } // If we're already in the middle of a render, switching lanes will interrupt // it and we'll lose our progress. We should only do this if the new lanes are @@ -673,8 +682,27 @@ export function markRootUpdated( // Unsuspend any update at equal or lower priority. const higherPriorityLanes = updateLane - 1; // Turns 0b1000 into 0b0111 - root.suspendedLanes &= higherPriorityLanes; - root.pingedLanes &= higherPriorityLanes; + if (enableTransitionEntanglement) { + // If there are any suspended transitions, it's possible this new update + // could unblock them. Clear the suspended lanes so that we can try rendering + // them again. + // + // TODO: We really only need to unsuspend only lanes that are in the + // `subtreeLanes` of the updated fiber, or the update lanes of the return + // path. This would exclude suspended updates in an unrelated sibling tree, + // since there's no way for this update to unblock it. + // + // We don't do this if the incoming update is idle, because we never process + // idle updates until after all the regular updates have finished; there's no + // way it could unblock a transition. + if ((updateLane & IdleLanes) === NoLanes) { + root.suspendedLanes = NoLanes; + root.pingedLanes = NoLanes; + } + } else { + root.suspendedLanes &= higherPriorityLanes; + root.pingedLanes &= higherPriorityLanes; + } const eventTimes = root.eventTimes; const index = laneToIndex(updateLane); diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js index 7a00fe095e6c..7be355d2c245 100644 --- a/packages/react-reconciler/src/ReactFiberLane.old.js +++ b/packages/react-reconciler/src/ReactFiberLane.old.js @@ -36,7 +36,10 @@ export type Lane = number; export type LaneMap = Array; import invariant from 'shared/invariant'; -import {enableCache} from 'shared/ReactFeatureFlags'; +import { + enableCache, + enableTransitionEntanglement, +} from 'shared/ReactFeatureFlags'; import { ImmediatePriority as ImmediateSchedulerPriority, @@ -306,9 +309,15 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { return NoLanes; } - // If there are higher priority lanes, we'll include them even if they - // are suspended. - nextLanes = pendingLanes & getEqualOrHigherPriorityLanes(nextLanes); + if (enableTransitionEntanglement) { + // We don't need to include higher priority lanes, because in this + // experiment we always unsuspend all transitions whenever we receive + // an update. + } else { + // If there are higher priority lanes, we'll include them even if they + // are suspended. + nextLanes = pendingLanes & getEqualOrHigherPriorityLanes(nextLanes); + } // If we're already in the middle of a render, switching lanes will interrupt // it and we'll lose our progress. We should only do this if the new lanes are @@ -673,8 +682,27 @@ export function markRootUpdated( // Unsuspend any update at equal or lower priority. const higherPriorityLanes = updateLane - 1; // Turns 0b1000 into 0b0111 - root.suspendedLanes &= higherPriorityLanes; - root.pingedLanes &= higherPriorityLanes; + if (enableTransitionEntanglement) { + // If there are any suspended transitions, it's possible this new update + // could unblock them. Clear the suspended lanes so that we can try rendering + // them again. + // + // TODO: We really only need to unsuspend only lanes that are in the + // `subtreeLanes` of the updated fiber, or the update lanes of the return + // path. This would exclude suspended updates in an unrelated sibling tree, + // since there's no way for this update to unblock it. + // + // We don't do this if the incoming update is idle, because we never process + // idle updates until after all the regular updates have finished; there's no + // way it could unblock a transition. + if ((updateLane & IdleLanes) === NoLanes) { + root.suspendedLanes = NoLanes; + root.pingedLanes = NoLanes; + } + } else { + root.suspendedLanes &= higherPriorityLanes; + root.pingedLanes &= higherPriorityLanes; + } const eventTimes = root.eventTimes; const index = laneToIndex(updateLane); diff --git a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js index 1b4433670974..fd224b2a1f81 100644 --- a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js @@ -733,13 +733,19 @@ describe('ReactExpiration', () => { // Both normal pri updates should have expired. expect(Scheduler).toFlushExpired([ 'Sibling', - // Note: we also flushed the high pri update here, because in the - // current implementation, once we pick the next lanes to work on, we - // entangle it with all pending at equal or higher priority. We could - // feasibly change this heuristic so that the high pri update doesn't - // render until after the expired updates have finished. But the - // important thing in this test is that the normal updates expired. - 'High pri: 1', + gate(flags => flags.enableTransitionEntanglement) + ? // Notice that the high pri update didn't flush yet. Expiring one lane + // doesn't affect other lanes. (Unless they are intentionally + // entangled, like we do for overlapping transitions that affect the + // same state.) + 'High pri: 0' + : // In the current implementation, once we pick the next lanes to work + // on, we entangle it with all pending at equal or higher priority. + // We could feasibly change this heuristic so that the high pri + // update doesn't render until after the expired updates have + // finished. But the important thing in this test is that the normal + // updates expired. + 'High pri: 1', 'Normal pri: 2', 'Sibling', ]); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js index c45a10c99d6d..de74c18e5ca6 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js @@ -2443,7 +2443,7 @@ describe('ReactSuspenseList', () => { expect(ReactNoop).toMatchRenderedOutput(null); - ReactNoop.act(() => { + await ReactNoop.act(async () => { // Add a few items at the end. updateLowPri(true); @@ -2451,33 +2451,22 @@ describe('ReactSuspenseList', () => { expect(Scheduler).toFlushAndYieldThrough(['B', 'C']); // Schedule another update at higher priority. - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => updateHighPri(true), - ); + ReactNoop.flushSync(() => updateHighPri(true)); // That will intercept the previous render. - }); + expect(Scheduler).toHaveYielded([ + 'Suspend! [A]', + 'Loading A', + // Re-render at forced. + 'Suspend! [A]', + 'Loading A', + ]); + expect(ReactNoop).toMatchRenderedOutput(Loading A); - jest.runAllTimers(); - - expect(Scheduler).toHaveYielded([ - // First attempt at high pri. - 'Suspend! [A]', - 'Loading A', - // Re-render at forced. - 'Suspend! [A]', - 'Loading A', - // We auto-commit this on DEV. // Try again on low-pri. - 'Suspend! [A]', - 'Loading A', - // Re-render at forced. - 'Suspend! [A]', - 'Loading A', - ]); - - expect(ReactNoop).toMatchRenderedOutput(Loading A); + expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading A']); + expect(ReactNoop).toMatchRenderedOutput(Loading A); + }); await AsyncA.resolve(); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 8611a9da06bc..e09482a525ac 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -149,3 +149,6 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; + +// Experiment to simplify/improve how transitions are scheduled +export const enableTransitionEntanglement = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 0fd02c2fd3f1..494cae50c8b4 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableTransitionEntanglement = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index c76048a8b2b4..6c4616798eb3 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableTransitionEntanglement = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 447d77effbc4..3971598060d5 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableTransitionEntanglement = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index 6a6d8f39c59b..0d31d83b657f 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableTransitionEntanglement = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 56e7dde89dfb..f0479741a570 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableTransitionEntanglement = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 537c3fbf1c09..3c2da8f5fc9d 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableTransitionEntanglement = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 24dd55704700..26eda01b2a24 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableTransitionEntanglement = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index cf858b767cc3..e6cbe5c387f5 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -55,3 +55,4 @@ export const enableUseRefAccessWarning = __VARIANT__; export const enableProfilerNestedUpdateScheduledHook = __VARIANT__; export const disableSchedulerTimeoutInWorkLoop = __VARIANT__; +export const enableTransitionEntanglement = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 85ce6eca5728..f39c8bee578e 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -31,6 +31,7 @@ export const { enableUseRefAccessWarning, disableNativeComponentFrames, disableSchedulerTimeoutInWorkLoop, + enableTransitionEntanglement, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build.