diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js index ced32541ee8df..501af9e8b12a7 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -39,6 +39,7 @@ import { enableCache, enableSchedulingProfiler, enableUpdaterTracking, + enableSyncDefaultUpdates, } from 'shared/ReactFeatureFlags'; import {isDevToolsPresent} from './ReactFiberDevToolsHook.new'; @@ -263,9 +264,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { // Default priority updates should not interrupt transition updates. The // only difference between default updates and transition updates is that // default updates do not support refresh transitions. - // TODO: This applies to sync default updates, too. Which is probably what - // we want for default priority events, but not for continuous priority - // events like hover. (nextLane === DefaultLane && (wipLane & TransitionLanes) !== NoLanes) ) { // Keep working on the existing in-progress tree. Do not interrupt. @@ -273,6 +271,18 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { } } + if ( + // TODO: Check for root override, once that lands + enableSyncDefaultUpdates && + (nextLanes & InputContinuousLane) !== NoLanes + ) { + // When updates are sync by default, we entangle continous priority updates + // and default updates, so they render in the same batch. The only reason + // they use separate lanes is because continuous updates should interrupt + // transitions, but default updates should not. + nextLanes |= pendingLanes & DefaultLane; + } + // Check for entangled lanes and add them to the batch. // // A lane is said to be entangled with another when it's not allowed to render @@ -467,6 +477,19 @@ export function includesOnlyTransitions(lanes: Lanes) { return (lanes & TransitionLanes) === lanes; } +export function shouldTimeSlice(root: FiberRoot, lanes: Lanes) { + if (!enableSyncDefaultUpdates) { + return true; + } + const SyncDefaultLanes = + InputContinuousHydrationLane | + InputContinuousLane | + DefaultHydrationLane | + DefaultLane; + // TODO: Check for root override, once that lands + return (lanes & SyncDefaultLanes) === NoLanes; +} + export function isTransitionLane(lane: Lane) { return (lane & TransitionLanes) !== 0; } diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js index 5af142edbbae9..701d4c46cf727 100644 --- a/packages/react-reconciler/src/ReactFiberLane.old.js +++ b/packages/react-reconciler/src/ReactFiberLane.old.js @@ -39,6 +39,7 @@ import { enableCache, enableSchedulingProfiler, enableUpdaterTracking, + enableSyncDefaultUpdates, } from 'shared/ReactFeatureFlags'; import {isDevToolsPresent} from './ReactFiberDevToolsHook.old'; @@ -263,9 +264,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { // Default priority updates should not interrupt transition updates. The // only difference between default updates and transition updates is that // default updates do not support refresh transitions. - // TODO: This applies to sync default updates, too. Which is probably what - // we want for default priority events, but not for continuous priority - // events like hover. (nextLane === DefaultLane && (wipLane & TransitionLanes) !== NoLanes) ) { // Keep working on the existing in-progress tree. Do not interrupt. @@ -273,6 +271,18 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { } } + if ( + // TODO: Check for root override, once that lands + enableSyncDefaultUpdates && + (nextLanes & InputContinuousLane) !== NoLanes + ) { + // When updates are sync by default, we entangle continous priority updates + // and default updates, so they render in the same batch. The only reason + // they use separate lanes is because continuous updates should interrupt + // transitions, but default updates should not. + nextLanes |= pendingLanes & DefaultLane; + } + // Check for entangled lanes and add them to the batch. // // A lane is said to be entangled with another when it's not allowed to render @@ -467,6 +477,19 @@ export function includesOnlyTransitions(lanes: Lanes) { return (lanes & TransitionLanes) === lanes; } +export function shouldTimeSlice(root: FiberRoot, lanes: Lanes) { + if (!enableSyncDefaultUpdates) { + return true; + } + const SyncDefaultLanes = + InputContinuousHydrationLane | + InputContinuousLane | + DefaultHydrationLane | + DefaultLane; + // TODO: Check for root override, once that lands + return (lanes & SyncDefaultLanes) === NoLanes; +} + export function isTransitionLane(lane: Lane) { return (lane & TransitionLanes) !== 0; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 665df80b13347..9ebace0c72010 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -32,7 +32,6 @@ import { disableSchedulerTimeoutInWorkLoop, enableStrictEffects, skipUnmountedBoundaries, - enableSyncDefaultUpdates, enableUpdaterTracking, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -139,10 +138,6 @@ import { NoLanes, NoLane, SyncLane, - DefaultLane, - DefaultHydrationLane, - InputContinuousLane, - InputContinuousHydrationLane, NoTimestamp, claimNextTransitionLane, claimNextRetryLane, @@ -154,6 +149,7 @@ import { includesNonIdleWork, includesOnlyRetries, includesOnlyTransitions, + shouldTimeSlice, getNextLanes, markStarvedLanesAsExpired, getLanesToRetrySynchronouslyOnError, @@ -437,13 +433,6 @@ export function requestUpdateLane(fiber: Fiber): Lane { // TODO: Move this type conversion to the event priority module. const updateLane: Lane = (getCurrentUpdatePriority(): any); if (updateLane !== NoLane) { - if ( - enableSyncDefaultUpdates && - (updateLane === InputContinuousLane || - updateLane === InputContinuousHydrationLane) - ) { - return DefaultLane; - } return updateLane; } @@ -454,13 +443,6 @@ export function requestUpdateLane(fiber: Fiber): Lane { // use that directly. // TODO: Move this type conversion to the event priority module. const eventLane: Lane = (getCurrentEventPriority(): any); - if ( - enableSyncDefaultUpdates && - (eventLane === InputContinuousLane || - eventLane === InputContinuousHydrationLane) - ) { - return DefaultLane; - } return eventLane; } @@ -811,13 +793,10 @@ function performConcurrentWorkOnRoot(root, didTimeout) { return null; } - let exitStatus = - enableSyncDefaultUpdates && - (includesSomeLane(lanes, DefaultLane) || - includesSomeLane(lanes, DefaultHydrationLane)) - ? // Time slicing is disabled for default updates in this root. - renderRootSync(root, lanes) - : renderRootConcurrent(root, lanes); + let exitStatus = shouldTimeSlice(root, lanes) + ? renderRootConcurrent(root, lanes) + : // Time slicing is disabled for default updates in this root. + renderRootSync(root, lanes); if (exitStatus !== RootIncomplete) { if (exitStatus === RootErrored) { executionContext |= RetryAfterError; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index baebee3dc0490..09a116c97bd7d 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -32,7 +32,6 @@ import { disableSchedulerTimeoutInWorkLoop, enableStrictEffects, skipUnmountedBoundaries, - enableSyncDefaultUpdates, enableUpdaterTracking, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -139,10 +138,6 @@ import { NoLanes, NoLane, SyncLane, - DefaultLane, - DefaultHydrationLane, - InputContinuousLane, - InputContinuousHydrationLane, NoTimestamp, claimNextTransitionLane, claimNextRetryLane, @@ -154,6 +149,7 @@ import { includesNonIdleWork, includesOnlyRetries, includesOnlyTransitions, + shouldTimeSlice, getNextLanes, markStarvedLanesAsExpired, getLanesToRetrySynchronouslyOnError, @@ -437,13 +433,6 @@ export function requestUpdateLane(fiber: Fiber): Lane { // TODO: Move this type conversion to the event priority module. const updateLane: Lane = (getCurrentUpdatePriority(): any); if (updateLane !== NoLane) { - if ( - enableSyncDefaultUpdates && - (updateLane === InputContinuousLane || - updateLane === InputContinuousHydrationLane) - ) { - return DefaultLane; - } return updateLane; } @@ -454,13 +443,6 @@ export function requestUpdateLane(fiber: Fiber): Lane { // use that directly. // TODO: Move this type conversion to the event priority module. const eventLane: Lane = (getCurrentEventPriority(): any); - if ( - enableSyncDefaultUpdates && - (eventLane === InputContinuousLane || - eventLane === InputContinuousHydrationLane) - ) { - return DefaultLane; - } return eventLane; } @@ -811,13 +793,10 @@ function performConcurrentWorkOnRoot(root, didTimeout) { return null; } - let exitStatus = - enableSyncDefaultUpdates && - (includesSomeLane(lanes, DefaultLane) || - includesSomeLane(lanes, DefaultHydrationLane)) - ? // Time slicing is disabled for default updates in this root. - renderRootSync(root, lanes) - : renderRootConcurrent(root, lanes); + let exitStatus = shouldTimeSlice(root, lanes) + ? renderRootConcurrent(root, lanes) + : // Time slicing is disabled for default updates in this root. + renderRootSync(root, lanes); if (exitStatus !== RootIncomplete) { if (exitStatus === RootErrored) { executionContext |= RetryAfterError; diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 485d477ed7e8e..07d3f7ff526aa 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -1405,18 +1405,6 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.unstable_runWithPriority(ContinuousEventPriority, () => { setParentState(false); }); - if (gate(flags => flags.enableSyncDefaultUpdates)) { - // TODO: Default updates do not interrupt transition updates, to - // prevent starvation. However, when sync default updates are enabled, - // continuous updates are treated like default updates. In this case, - // we probably don't want this behavior; continuous should be allowed - // to interrupt. - expect(Scheduler).toFlushUntilNextPaint([ - 'Child two render', - 'Child one commit', - 'Child two commit', - ]); - } expect(Scheduler).toFlushUntilNextPaint([ 'Parent false render', 'Parent false commit', diff --git a/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js b/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js index 0aee7df66df4a..116cc4c4d8e00 100644 --- a/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js +++ b/packages/react-reconciler/src/__tests__/ReactUpdatePriority-test.js @@ -1,6 +1,8 @@ let React; let ReactNoop; let Scheduler; +let ContinuousEventPriority; +let startTransition; let useState; let useEffect; @@ -11,6 +13,9 @@ describe('ReactUpdatePriority', () => { React = require('react'); ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); + ContinuousEventPriority = require('react-reconciler/constants') + .ContinuousEventPriority; + startTransition = React.unstable_startTransition; useState = React.useState; useEffect = React.useEffect; }); @@ -78,4 +83,53 @@ describe('ReactUpdatePriority', () => { // Now the idle update has flushed expect(Scheduler).toHaveYielded(['Idle: 2, Default: 2']); }); + + // @gate experimental + test('continuous updates should interrupt transisions', async () => { + const root = ReactNoop.createRoot(); + + let setCounter; + let setIsHidden; + function App() { + const [counter, _setCounter] = useState(1); + const [isHidden, _setIsHidden] = useState(false); + setCounter = _setCounter; + setIsHidden = _setIsHidden; + if (isHidden) { + return ; + } + return ( + <> + + + + + ); + } + + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A1', 'B1', 'C1']); + expect(root).toMatchRenderedOutput('A1B1C1'); + + await ReactNoop.act(async () => { + startTransition(() => { + setCounter(2); + }); + expect(Scheduler).toFlushAndYieldThrough(['A2']); + ReactNoop.unstable_runWithPriority(ContinuousEventPriority, () => { + setIsHidden(true); + }); + }); + expect(Scheduler).toHaveYielded([ + // Because the hide update has continous priority, it should interrupt the + // in-progress transition + '(hidden)', + // When the transition resumes, it's a no-op because the children are + // now hidden. + '(hidden)', + ]); + expect(root).toMatchRenderedOutput('(hidden)'); + }); }); diff --git a/packages/react-reconciler/src/__tests__/SchedulingProfilerLabels-test.internal.js b/packages/react-reconciler/src/__tests__/SchedulingProfilerLabels-test.internal.js index da59c153f43dd..6474be855b38f 100644 --- a/packages/react-reconciler/src/__tests__/SchedulingProfilerLabels-test.internal.js +++ b/packages/react-reconciler/src/__tests__/SchedulingProfilerLabels-test.internal.js @@ -168,18 +168,10 @@ describe('SchedulingProfiler labels', () => { event.initEvent('mouseover', true, true); dispatchAndSetCurrentEvent(targetRef.current, event); }); - if (gate(flags => flags.enableSyncDefaultUpdates)) { - expect(clearedMarks).toContain( - `--schedule-state-update-${formatLanes( - ReactFiberLane.DefaultLane, - )}-App`, - ); - } else { - expect(clearedMarks).toContain( - `--schedule-state-update-${formatLanes( - ReactFiberLane.InputContinuousLane, - )}-App`, - ); - } + expect(clearedMarks).toContain( + `--schedule-state-update-${formatLanes( + ReactFiberLane.InputContinuousLane, + )}-App`, + ); }); });