Skip to content

Commit

Permalink
Batch default, continuous and sync lane
Browse files Browse the repository at this point in the history
  • Loading branch information
tyao1 committed Nov 28, 2022
1 parent edbfc63 commit 711efaf
Show file tree
Hide file tree
Showing 16 changed files with 342 additions and 135 deletions.
27 changes: 21 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,17 +275,32 @@ describe('ReactDOMFiberAsync', () => {
expect(ops).toEqual([]);
});
// Only the active updates have flushed
expect(container.textContent).toEqual('BC');
expect(ops).toEqual(['BC']);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(container.textContent).toEqual('ABC');
expect(ops).toEqual(['ABC']);
} else {
expect(container.textContent).toEqual('BC');
expect(ops).toEqual(['BC']);
}

instance.push('D');
expect(container.textContent).toEqual('BC');
expect(ops).toEqual(['BC']);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
instance.push('D');
expect(container.textContent).toEqual('ABC');
expect(ops).toEqual(['ABC']);
} else {
instance.push('D');
expect(container.textContent).toEqual('BC');
expect(ops).toEqual(['BC']);
}

// Flush the async updates
Scheduler.unstable_flushAll();
expect(container.textContent).toEqual('ABCD');
expect(ops).toEqual(['BC', 'ABCD']);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(ops).toEqual(['ABC', 'ABCD']);
} else {
expect(ops).toEqual(['BC', 'ABCD']);
}
});

// @gate www
Expand Down
26 changes: 25 additions & 1 deletion packages/react-reconciler/src/ReactFiberLane.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
enableUpdaterTracking,
allowConcurrentByDefault,
enableTransitionTracing,
enableSyncDefaultUpdates,
} from 'shared/ReactFeatureFlags';
import {isDevToolsPresent} from './ReactFiberDevToolsHook.new';
import {ConcurrentUpdatesByDefaultMode, NoMode} from './ReactTypeOfMode';
Expand Down Expand Up @@ -135,8 +136,28 @@ let nextRetryLane: Lane = RetryLane1;
function getHighestPriorityLanes(lanes: Lanes | Lane): Lanes {
switch (getHighestPriorityLane(lanes)) {
case SyncHydrationLane:
if (enableSyncDefaultUpdates) {
let ret = SyncHydrationLane;
if (lanes & DefaultHydrationLane) {
ret |= DefaultHydrationLane;
}
if (lanes & InputContinuousHydrationLane) {
ret |= InputContinuousHydrationLane;
}
return ret;
}
return SyncHydrationLane;
case SyncLane:
if (enableSyncDefaultUpdates) {
let ret = SyncLane;
if (lanes & DefaultLane) {
ret |= DefaultLane;
}
if (lanes & InputContinuousLane) {
ret |= InputContinuousLane;
}
return ret;
}
return SyncLane;
case InputContinuousHydrationLane:
return InputContinuousHydrationLane;
Expand Down Expand Up @@ -251,7 +272,10 @@ 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.
(nextLane === DefaultLane && (wipLane & TransitionLanes) !== NoLanes)
// Interrupt transtion if default is batched with sync.
(!enableSyncDefaultUpdates &&
nextLane === DefaultLane &&
(wipLane & TransitionLanes) !== NoLanes)
) {
// Keep working on the existing in-progress tree. Do not interrupt.
return wipLanes;
Expand Down
26 changes: 25 additions & 1 deletion packages/react-reconciler/src/ReactFiberLane.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
enableUpdaterTracking,
allowConcurrentByDefault,
enableTransitionTracing,
enableSyncDefaultUpdates,
} from 'shared/ReactFeatureFlags';
import {isDevToolsPresent} from './ReactFiberDevToolsHook.old';
import {ConcurrentUpdatesByDefaultMode, NoMode} from './ReactTypeOfMode';
Expand Down Expand Up @@ -135,8 +136,28 @@ let nextRetryLane: Lane = RetryLane1;
function getHighestPriorityLanes(lanes: Lanes | Lane): Lanes {
switch (getHighestPriorityLane(lanes)) {
case SyncHydrationLane:
if (enableSyncDefaultUpdates) {
let ret = SyncHydrationLane;
if (lanes & DefaultHydrationLane) {
ret |= DefaultHydrationLane;
}
if (lanes & InputContinuousHydrationLane) {
ret |= InputContinuousHydrationLane;
}
return ret;
}
return SyncHydrationLane;
case SyncLane:
if (enableSyncDefaultUpdates) {
let ret = SyncLane;
if (lanes & DefaultLane) {
ret |= DefaultLane;
}
if (lanes & InputContinuousLane) {
ret |= InputContinuousLane;
}
return ret;
}
return SyncLane;
case InputContinuousHydrationLane:
return InputContinuousHydrationLane;
Expand Down Expand Up @@ -251,7 +272,10 @@ 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.
(nextLane === DefaultLane && (wipLane & TransitionLanes) !== NoLanes)
// Interrupt transtion if default is batched with sync.
(!enableSyncDefaultUpdates &&
nextLane === DefaultLane &&
(wipLane & TransitionLanes) !== NoLanes)
) {
// Keep working on the existing in-progress tree. Do not interrupt.
return wipLanes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,18 @@ describe('ReactBlockingMode', () => {
}),
);

// Only the second update should have flushed synchronously
expect(Scheduler).toHaveYielded(['B1']);
expect(root).toMatchRenderedOutput('A0B1');

// Now flush the first update
expect(Scheduler).toFlushAndYield(['A1']);
expect(root).toMatchRenderedOutput('A1B1');
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(Scheduler).toHaveYielded(['A1', 'B1']);
expect(root).toMatchRenderedOutput('A1B1');
} else {
// Only the second update should have flushed synchronously
expect(Scheduler).toHaveYielded(['B1']);
expect(root).toMatchRenderedOutput('A0B1');

// Now flush the first update
expect(Scheduler).toFlushAndYield(['A1']);
expect(root).toMatchRenderedOutput('A1B1');
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,17 @@ describe('ReactClassSetStateCallback', () => {
expect(Scheduler).toHaveYielded([0]);

await act(async () => {
app.setState({step: 1}, () =>
Scheduler.unstable_yieldValue('Callback 1'),
);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.startTransition(() => {
app.setState({step: 1}, () =>
Scheduler.unstable_yieldValue('Callback 1'),
);
});
} else {
app.setState({step: 1}, () =>
Scheduler.unstable_yieldValue('Callback 1'),
);
}
ReactNoop.flushSync(() => {
app.setState({step: 2}, () =>
Scheduler.unstable_yieldValue('Callback 2'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,9 @@ describe('ReactExpiration', () => {
// Before the update can finish, update again. Even though no time has
// advanced, this update should be given a different expiration time than
// the currently rendering one. So, C and D should render with 1, not 2.
subscribers.forEach(s => s.setState({text: '2'}));
React.startTransition(() => {
subscribers.forEach(s => s.setState({text: '2'}));
});
expect(Scheduler).toFlushAndYieldThrough([
'1 [C] [render]',
'1 [D] [render]',
Expand Down
12 changes: 9 additions & 3 deletions packages/react-reconciler/src/__tests__/ReactFlushSync-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,21 @@ describe('ReactFlushSync', () => {
// The passive effect will schedule a sync update and a normal update.
// They should commit in two separate batches. First the sync one.
expect(() => {
expect(Scheduler).toFlushUntilNextPaint(['1, 0']);
expect(Scheduler).toFlushUntilNextPaint(
gate(flags => flags.enableSyncDefaultUpdates) ? ['1, 1'] : ['1', '0'],
);
}).toErrorDev('flushSync was called from inside a lifecycle method');

// The remaining update is not sync
ReactNoop.flushSync();
expect(Scheduler).toHaveYielded([]);

// Now flush it.
expect(Scheduler).toFlushUntilNextPaint(['1, 1']);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(Scheduler).toFlushUntilNextPaint([]);
} else {
// Now flush it.
expect(Scheduler).toFlushUntilNextPaint(['1 / 1']);
}
});
expect(root).toMatchRenderedOutput('1, 1');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -568,9 +568,13 @@ describe('ReactHooks', () => {
});
};

// Update at normal priority
ReactTestRenderer.unstable_batchedUpdates(() => update(n => n * 100));

if (gate(flags => flags.enableSyncDefaultUpdates)) {
// Update at transition priority
React.startTransition(() => update(n => n * 100));
} else {
// Update at normal priority
ReactTestRenderer.unstable_batchedUpdates(() => update(n => n * 100));
}
// The new state is eagerly computed.
expect(Scheduler).toHaveYielded(['Compute state (1 -> 100)']);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,13 @@ describe('ReactHooksWithNoopRenderer', () => {
ReactNoop.discreteUpdates(() => {
setRow(5);
});
setRow(20);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.startTransition(() => {
setRow(20);
});
} else {
setRow(20);
}
});
expect(Scheduler).toHaveYielded(['Up', 'Down']);
expect(root).toMatchRenderedOutput(<span prop="Down" />);
Expand Down Expand Up @@ -955,11 +961,15 @@ describe('ReactHooksWithNoopRenderer', () => {
ReactNoop.flushSync(() => {
counter.current.dispatch(INCREMENT);
});
expect(Scheduler).toHaveYielded(['Count: 1']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);

expect(Scheduler).toFlushAndYield(['Count: 4']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 4')]);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(Scheduler).toHaveYielded(['Count: 4']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 4')]);
} else {
expect(Scheduler).toHaveYielded(['Count: 1']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
expect(Scheduler).toFlushAndYield(['Count: 4']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 4')]);
}
});
});

Expand Down Expand Up @@ -1717,11 +1727,15 @@ describe('ReactHooksWithNoopRenderer', () => {
// As a result we, somewhat surprisingly, commit them in the opposite order.
// This should be fine because any non-discrete set of work doesn't guarantee order
// and easily could've happened slightly later too.
expect(Scheduler).toHaveYielded([
'Will set count to 1',
'Count: 2',
'Count: 1',
]);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(Scheduler).toHaveYielded(['Will set count to 1', 'Count: 1']);
} else {
expect(Scheduler).toHaveYielded([
'Will set count to 1',
'Count: 2',
'Count: 1',
]);
}

expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
});
Expand Down
58 changes: 41 additions & 17 deletions packages/react-reconciler/src/__tests__/ReactIncremental-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1910,21 +1910,37 @@ describe('ReactIncremental', () => {
<ShowBoth />
</Intl>,
);
expect(Scheduler).toFlushAndYield([
'ShowLocale {"locale":"sv"}',
'ShowBoth {"locale":"sv"}',
'Intl {}',
'ShowLocale {"locale":"en"}',
'Router {}',
'Indirection {}',
'ShowLocale {"locale":"en"}',
'ShowRoute {"route":"/about"}',
'ShowNeither {}',
'Intl {}',
'ShowBoth {"locale":"ru","route":"/about"}',
'ShowBoth {"locale":"en","route":"/about"}',
'ShowBoth {"locale":"en"}',
]);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(Scheduler).toFlushAndYield([
'Intl {}',
'ShowLocale {"locale":"en"}',
'Router {}',
'Indirection {}',
'ShowLocale {"locale":"en"}',
'ShowRoute {"route":"/about"}',
'ShowNeither {}',
'Intl {}',
'ShowBoth {"locale":"ru","route":"/about"}',
'ShowBoth {"locale":"en","route":"/about"}',
'ShowBoth {"locale":"en"}',
]);
} else {
expect(Scheduler).toFlushAndYield([
'ShowLocale {"locale":"sv"}',
'ShowBoth {"locale":"sv"}',
'Intl {}',
'ShowLocale {"locale":"en"}',
'Router {}',
'Indirection {}',
'ShowLocale {"locale":"en"}',
'ShowRoute {"route":"/about"}',
'ShowNeither {}',
'Intl {}',
'ShowBoth {"locale":"ru","route":"/about"}',
'ShowBoth {"locale":"en","route":"/about"}',
'ShowBoth {"locale":"en"}',
]);
}
});

it('does not leak own context into context provider', () => {
Expand Down Expand Up @@ -2758,7 +2774,11 @@ describe('ReactIncremental', () => {
// Interrupt at same priority
ReactNoop.render(<Parent step={2} />);

expect(Scheduler).toFlushAndYield(['Child: 1', 'Parent: 2', 'Child: 2']);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(Scheduler).toFlushAndYield(['Parent: 2', 'Child: 2']);
} else {
expect(Scheduler).toFlushAndYield(['Child: 1', 'Parent: 2', 'Child: 2']);
}
});

it('does not interrupt for update at lower priority', () => {
Expand All @@ -2785,7 +2805,11 @@ describe('ReactIncremental', () => {
ReactNoop.expire(2000);
ReactNoop.render(<Parent step={2} />);

expect(Scheduler).toFlushAndYield(['Child: 1', 'Parent: 2', 'Child: 2']);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(Scheduler).toFlushAndYield(['Parent: 2', 'Child: 2']);
} else {
expect(Scheduler).toFlushAndYield(['Child: 1', 'Parent: 2', 'Child: 2']);
}
});

it('does interrupt for update at higher priority', () => {
Expand Down
Loading

0 comments on commit 711efaf

Please sign in to comment.