Skip to content

Commit

Permalink
Make time-slicing opt-in (#21072)
Browse files Browse the repository at this point in the history
* Add enableSyncDefaultUpdates feature flag

* Add enableSyncDefaultUpdates implementation

* Fix tests

* Switch feature flag to true by default

* Finish concurrent render whenever for non-sync lanes

* Also return DefaultLane with eventLane

* Gate interruption test

* Add continuout native event test

* Fix tests from rebasing main

* Hardcode lanes, remove added export

* Sync forks
  • Loading branch information
rickhanlonii authored Apr 9, 2021
1 parent b0407b5 commit 933880b
Show file tree
Hide file tree
Showing 46 changed files with 1,996 additions and 538 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ describe('createSubscription', () => {
expect(Scheduler).toFlushAndYield(['b-1']);
});

// @gate experimental || !enableSyncDefaultUpdates
it('should ignore values emitted by a new subscribable until the commit phase', () => {
const log = [];

Expand Down Expand Up @@ -325,7 +326,13 @@ describe('createSubscription', () => {
expect(log).toEqual(['Parent.componentDidMount']);

// Start React update, but don't finish
ReactNoop.render(<Parent observed={observableB} />);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.unstable_startTransition(() => {
ReactNoop.render(<Parent observed={observableB} />);
});
} else {
ReactNoop.render(<Parent observed={observableB} />);
}
expect(Scheduler).toFlushAndYieldThrough(['Subscriber: b-0']);
expect(log).toEqual(['Parent.componentDidMount']);

Expand Down Expand Up @@ -355,6 +362,7 @@ describe('createSubscription', () => {
]);
});

// @gate experimental || !enableSyncDefaultUpdates
it('should not drop values emitted between updates', () => {
const log = [];

Expand Down Expand Up @@ -412,7 +420,13 @@ describe('createSubscription', () => {
expect(log).toEqual(['Parent.componentDidMount']);

// Start React update, but don't finish
ReactNoop.render(<Parent observed={observableB} />);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.unstable_startTransition(() => {
ReactNoop.render(<Parent observed={observableB} />);
});
} else {
ReactNoop.render(<Parent observed={observableB} />);
}
expect(Scheduler).toFlushAndYieldThrough(['Subscriber: b-0']);
expect(log).toEqual(['Parent.componentDidMount']);

Expand Down
1 change: 1 addition & 0 deletions packages/react-art/src/__tests__/ReactART-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ describe('ReactART', () => {
expect(onClick2).toBeCalled();
});

// @gate !enableSyncDefaultUpdates
it('can concurrently render with a "primary" renderer while sharing context', () => {
const CurrentRendererContext = React.createContext(null);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,46 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
expect(container.textContent).toEqual('hovered');
});

// @gate experimental
it('continuous native events flush as expected', async () => {
const root = ReactDOM.unstable_createRoot(container);

const target = React.createRef(null);
function Foo({hovered}) {
const hoverString = hovered ? 'hovered' : 'not hovered';
Scheduler.unstable_yieldValue(hoverString);
return <div ref={target}>{hoverString}</div>;
}

await act(async () => {
root.render(<Foo hovered={false} />);
});
expect(container.textContent).toEqual('not hovered');

await act(async () => {
// Note: React does not use native mouseenter/mouseleave events
// but we should still correctly determine their priority.
const mouseEnterEvent = document.createEvent('MouseEvents');
mouseEnterEvent.initEvent('mouseover', true, true);
target.current.addEventListener('mouseover', () => {
root.render(<Foo hovered={true} />);
});
dispatchAndSetCurrentEvent(target.current, mouseEnterEvent);

// Since mouse end is not discrete, should not have updated yet
expect(Scheduler).toHaveYielded(['not hovered']);
expect(container.textContent).toEqual('not hovered');

expect(Scheduler).toFlushAndYieldThrough(['hovered']);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(container.textContent).toEqual('hovered');
} else {
expect(container.textContent).toEqual('not hovered');
}
});
expect(container.textContent).toEqual('hovered');
});

// @gate experimental
it('should batch inside native events', async () => {
const root = ReactDOM.unstable_createRoot(container);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1865,11 +1865,21 @@ describe('ReactDOMServerPartialHydration', () => {
suspend = true;

await act(async () => {
root.render(<App />);
expect(Scheduler).toFlushAndYieldThrough(['Before']);
// This took a long time to render.
Scheduler.unstable_advanceTime(1000);
expect(Scheduler).toFlushAndYield(['After']);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.unstable_startTransition(() => {
root.render(<App />);
});

expect(Scheduler).toFlushAndYieldThrough(['Before', 'After']);
} else {
root.render(<App />);

expect(Scheduler).toFlushAndYieldThrough(['Before']);
// This took a long time to render.
Scheduler.unstable_advanceTime(1000);
expect(Scheduler).toFlushAndYield(['After']);
}

// This will cause us to skip the second row completely.
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1934,7 +1934,13 @@ describe('DOMPluginEventSystem', () => {
log.length = 0;

// Increase counter
root.render(<Test counter={1} />);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.unstable_startTransition(() => {
root.render(<Test counter={1} />);
});
} else {
root.render(<Test counter={1} />);
}
// Yield before committing
expect(Scheduler).toFlushAndYieldThrough(['Test']);

Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberLane.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const NoLane: Lane = /* */ 0b0000000000000000000

export const SyncLane: Lane = /* */ 0b0000000000000000000000000000001;

const InputContinuousHydrationLane: Lane = /* */ 0b0000000000000000000000000000010;
export const InputContinuousHydrationLane: Lane = /* */ 0b0000000000000000000000000000010;
export const InputContinuousLane: Lanes = /* */ 0b0000000000000000000000000000100;

export const DefaultHydrationLane: Lane = /* */ 0b0000000000000000000000000001000;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberLane.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const NoLane: Lane = /* */ 0b0000000000000000000

export const SyncLane: Lane = /* */ 0b0000000000000000000000000000001;

const InputContinuousHydrationLane: Lane = /* */ 0b0000000000000000000000000000010;
export const InputContinuousHydrationLane: Lane = /* */ 0b0000000000000000000000000000010;
export const InputContinuousLane: Lanes = /* */ 0b0000000000000000000000000000100;

export const DefaultHydrationLane: Lane = /* */ 0b0000000000000000000000000001000;
Expand Down
36 changes: 34 additions & 2 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
disableSchedulerTimeoutInWorkLoop,
enableStrictEffects,
skipUnmountedBoundaries,
enableSyncDefaultUpdates,
enableUpdaterTracking,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
Expand Down Expand Up @@ -138,6 +139,10 @@ import {
NoLanes,
NoLane,
SyncLane,
DefaultLane,
DefaultHydrationLane,
InputContinuousLane,
InputContinuousHydrationLane,
NoTimestamp,
claimNextTransitionLane,
claimNextRetryLane,
Expand Down Expand Up @@ -433,6 +438,13 @@ 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;
}

Expand All @@ -443,6 +455,13 @@ 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;
}

Expand Down Expand Up @@ -695,7 +714,16 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {

// Schedule a new callback.
let newCallbackNode;
if (newCallbackPriority === SyncLane) {
if (
enableSyncDefaultUpdates &&
(newCallbackPriority === DefaultLane ||
newCallbackPriority === DefaultHydrationLane)
) {
newCallbackNode = scheduleCallback(
ImmediateSchedulerPriority,
performSyncWorkOnRoot.bind(null, root),
);
} else if (newCallbackPriority === SyncLane) {
// Special case: Sync React callbacks are scheduled on a special
// internal queue
scheduleSyncCallback(performSyncWorkOnRoot.bind(null, root));
Expand Down Expand Up @@ -1030,7 +1058,11 @@ function performSyncWorkOnRoot(root) {
const finishedWork: Fiber = (root.current.alternate: any);
root.finishedWork = finishedWork;
root.finishedLanes = lanes;
commitRoot(root);
if (enableSyncDefaultUpdates && !includesSomeLane(lanes, SyncLane)) {
finishConcurrentRender(root, exitStatus, lanes);
} else {
commitRoot(root);
}

// Before exiting, make sure there's a callback scheduled for the next
// pending level.
Expand Down
36 changes: 34 additions & 2 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
disableSchedulerTimeoutInWorkLoop,
enableStrictEffects,
skipUnmountedBoundaries,
enableSyncDefaultUpdates,
enableUpdaterTracking,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
Expand Down Expand Up @@ -138,6 +139,10 @@ import {
NoLanes,
NoLane,
SyncLane,
DefaultLane,
DefaultHydrationLane,
InputContinuousLane,
InputContinuousHydrationLane,
NoTimestamp,
claimNextTransitionLane,
claimNextRetryLane,
Expand Down Expand Up @@ -433,6 +438,13 @@ 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;
}

Expand All @@ -443,6 +455,13 @@ 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;
}

Expand Down Expand Up @@ -695,7 +714,16 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {

// Schedule a new callback.
let newCallbackNode;
if (newCallbackPriority === SyncLane) {
if (
enableSyncDefaultUpdates &&
(newCallbackPriority === DefaultLane ||
newCallbackPriority === DefaultHydrationLane)
) {
newCallbackNode = scheduleCallback(
ImmediateSchedulerPriority,
performSyncWorkOnRoot.bind(null, root),
);
} else if (newCallbackPriority === SyncLane) {
// Special case: Sync React callbacks are scheduled on a special
// internal queue
scheduleSyncCallback(performSyncWorkOnRoot.bind(null, root));
Expand Down Expand Up @@ -1030,7 +1058,11 @@ function performSyncWorkOnRoot(root) {
const finishedWork: Fiber = (root.current.alternate: any);
root.finishedWork = finishedWork;
root.finishedLanes = lanes;
commitRoot(root);
if (enableSyncDefaultUpdates && !includesSomeLane(lanes, SyncLane)) {
finishConcurrentRender(root, exitStatus, lanes);
} else {
commitRoot(root);
}

// Before exiting, make sure there's a callback scheduled for the next
// pending level.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe('ReactSuspenseList', () => {
return Component;
}

// @gate experimental || !enableSyncDefaultUpdates
it('appends rendering tasks to the end of the priority queue', async () => {
const A = createAsyncText('A');
const B = createAsyncText('B');
Expand All @@ -63,7 +64,13 @@ describe('ReactSuspenseList', () => {
root.render(<App show={false} />);
expect(Scheduler).toFlushAndYield([]);

root.render(<App show={true} />);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.unstable_startTransition(() => {
root.render(<App show={true} />);
});
} else {
root.render(<App show={true} />);
}
expect(Scheduler).toFlushAndYield([
'Suspend! [A]',
'Suspend! [B]',
Expand Down
Loading

0 comments on commit 933880b

Please sign in to comment.