Cleanup enableUnifiedSyncLane flag

This commit is contained in:
Tianyu Yao
2023-01-20 17:58:04 -08:00
parent ee85098019
commit 2cc839ea5b
22 changed files with 81 additions and 266 deletions
+6 -21
View File
@@ -275,32 +275,17 @@ describe('ReactDOMFiberAsync', () => {
expect(ops).toEqual([]);
});
// Only the active updates have flushed
if (gate(flags => flags.enableUnifiedSyncLane)) {
expect(container.textContent).toEqual('ABC');
expect(ops).toEqual(['ABC']);
} else {
expect(container.textContent).toEqual('BC');
expect(ops).toEqual(['BC']);
}
expect(container.textContent).toEqual('ABC');
expect(ops).toEqual(['ABC']);
if (gate(flags => flags.enableUnifiedSyncLane)) {
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']);
}
instance.push('D');
expect(container.textContent).toEqual('ABC');
expect(ops).toEqual(['ABC']);
// Flush the async updates
Scheduler.unstable_flushAll();
expect(container.textContent).toEqual('ABCD');
if (gate(flags => flags.enableUnifiedSyncLane)) {
expect(ops).toEqual(['ABC', 'ABCD']);
} else {
expect(ops).toEqual(['BC', 'ABCD']);
}
expect(ops).toEqual(['ABC', 'ABCD']);
});
// @gate www
+4 -7
View File
@@ -23,7 +23,6 @@ import {
enableUpdaterTracking,
allowConcurrentByDefault,
enableTransitionTracing,
enableUnifiedSyncLane,
} from 'shared/ReactFeatureFlags';
import {isDevToolsPresent} from './ReactFiberDevToolsHook';
import {ConcurrentUpdatesByDefaultMode, NoMode} from './ReactTypeOfMode';
@@ -136,11 +135,9 @@ let nextTransitionLane: Lane = TransitionLane1;
let nextRetryLane: Lane = RetryLane1;
function getHighestPriorityLanes(lanes: Lanes | Lane): Lanes {
if (enableUnifiedSyncLane) {
const pendingSyncLanes = lanes & SyncUpdateLanes;
if (pendingSyncLanes !== 0) {
return pendingSyncLanes;
}
const pendingSyncLanes = lanes & SyncUpdateLanes;
if (pendingSyncLanes !== NoLanes) {
return pendingSyncLanes;
}
switch (getHighestPriorityLane(lanes)) {
case SyncHydrationLane:
@@ -759,7 +756,7 @@ export function getBumpedLaneForHydration(
const renderLane = getHighestPriorityLane(renderLanes);
let lane;
if (enableUnifiedSyncLane && (renderLane & SyncUpdateLanes) !== NoLane) {
if ((renderLane & SyncUpdateLanes) !== NoLane) {
lane = SyncHydrationLane;
} else {
switch (renderLane) {
@@ -158,17 +158,7 @@ describe('ReactBlockingMode', () => {
);
// Now flush the first update
if (gate(flags => flags.enableUnifiedSyncLane)) {
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');
}
expect(Scheduler).toHaveYielded(['A1', 'B1']);
expect(root).toMatchRenderedOutput('A1B1');
});
});
@@ -35,17 +35,11 @@ describe('ReactClassSetStateCallback', () => {
expect(Scheduler).toHaveYielded([0]);
await act(async () => {
if (gate(flags => flags.enableUnifiedSyncLane)) {
React.startTransition(() => {
app.setState({step: 1}, () =>
Scheduler.unstable_yieldValue('Callback 1'),
);
});
} else {
React.startTransition(() => {
app.setState({step: 1}, () =>
Scheduler.unstable_yieldValue('Callback 1'),
);
}
});
ReactNoop.flushSync(() => {
app.setState({step: 2}, () =>
Scheduler.unstable_yieldValue('Callback 2'),
@@ -54,21 +54,13 @@ 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(
gate(flags => flags.enableUnifiedSyncLane) ? ['1, 1'] : ['1, 0'],
);
expect(Scheduler).toFlushUntilNextPaint(['1, 1']);
}).toErrorDev('flushSync was called from inside a lifecycle method');
// The remaining update is not sync
// No remaining update
ReactNoop.flushSync();
expect(Scheduler).toHaveYielded([]);
if (gate(flags => flags.enableUnifiedSyncLane)) {
expect(Scheduler).toFlushUntilNextPaint([]);
} else {
// Now flush it.
expect(Scheduler).toFlushUntilNextPaint(['1, 1']);
}
expect(Scheduler).toFlushUntilNextPaint([]);
});
expect(root).toMatchRenderedOutput('1, 1');
});
@@ -568,13 +568,9 @@ describe('ReactHooks', () => {
});
};
if (gate(flags => flags.enableUnifiedSyncLane)) {
// Update at transition priority
React.startTransition(() => update(n => n * 100));
} else {
// Update at normal priority
ReactTestRenderer.unstable_batchedUpdates(() => update(n => n * 100));
}
// Update at transition priority
React.startTransition(() => update(n => n * 100));
// The new state is eagerly computed.
expect(Scheduler).toHaveYielded(['Compute state (1 -> 100)']);
@@ -961,15 +961,8 @@ describe('ReactHooksWithNoopRenderer', () => {
ReactNoop.flushSync(() => {
counter.current.dispatch(INCREMENT);
});
if (gate(flags => flags.enableUnifiedSyncLane)) {
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')]);
}
expect(Scheduler).toHaveYielded(['Count: 4']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 4')]);
});
});
@@ -1727,15 +1720,7 @@ 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.
if (gate(flags => flags.enableUnifiedSyncLane)) {
expect(Scheduler).toHaveYielded(['Will set count to 1', 'Count: 1']);
} else {
expect(Scheduler).toHaveYielded([
'Will set count to 1',
'Count: 2',
'Count: 1',
]);
}
expect(Scheduler).toHaveYielded(['Will set count to 1', 'Count: 1']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
});
@@ -162,11 +162,7 @@ describe('ReactIncrementalUpdates', () => {
}
// Schedule some async updates
if (
gate(
flags => flags.enableSyncDefaultUpdates || flags.enableUnifiedSyncLane,
)
) {
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.startTransition(() => {
instance.setState(createUpdate('a'));
instance.setState(createUpdate('b'));
@@ -193,11 +189,7 @@ describe('ReactIncrementalUpdates', () => {
});
// The sync updates should have flushed, but not the async ones.
if (
gate(
flags => flags.enableSyncDefaultUpdates && flags.enableUnifiedSyncLane,
)
) {
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(Scheduler).toHaveYielded(['d', 'e', 'f']);
expect(ReactNoop.getChildren()).toEqual([span('def')]);
} else {
@@ -209,38 +201,18 @@ describe('ReactIncrementalUpdates', () => {
// Now flush the remaining work. Even though e and f were already processed,
// they should be processed again, to ensure that the terminal state
// is deterministic.
if (
gate(
flags => flags.enableSyncDefaultUpdates && !flags.enableUnifiedSyncLane,
)
) {
expect(Scheduler).toFlushAndYield([
// Since 'g' is in a transition, we'll process 'd' separately first.
// That causes us to process 'd' with 'e' and 'f' rebased.
'd',
'e',
'f',
// Then we'll re-process everything for 'g'.
'a',
'b',
'c',
'd',
'e',
'f',
'g',
]);
} else {
expect(Scheduler).toFlushAndYield([
// Then we'll re-process everything for 'g'.
'a',
'b',
'c',
'd',
'e',
'f',
'g',
]);
}
expect(Scheduler).toFlushAndYield([
// Although 'g' is in a transition, it is intuerrupted and
// batched with 'd'
// Twe'll re-process everything for 'g'.
'a',
'b',
'c',
'd',
'e',
'f',
'g',
]);
expect(ReactNoop.getChildren()).toEqual([span('abcdefg')]);
});
@@ -273,11 +245,7 @@ describe('ReactIncrementalUpdates', () => {
}
// Schedule some async updates
if (
gate(
flags => flags.enableSyncDefaultUpdates || flags.enableUnifiedSyncLane,
)
) {
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.startTransition(() => {
instance.setState(createUpdate('a'));
instance.setState(createUpdate('b'));
@@ -307,11 +275,7 @@ describe('ReactIncrementalUpdates', () => {
});
// The sync updates should have flushed, but not the async ones.
if (
gate(
flags => flags.enableSyncDefaultUpdates && flags.enableUnifiedSyncLane,
)
) {
if (gate(flags => flags.enableSyncDefaultUpdates)) {
expect(Scheduler).toHaveYielded(['d', 'e', 'f']);
} else {
// Update d was dropped and replaced by e.
@@ -322,38 +286,7 @@ describe('ReactIncrementalUpdates', () => {
// Now flush the remaining work. Even though e and f were already processed,
// they should be processed again, to ensure that the terminal state
// is deterministic.
if (
gate(
flags => flags.enableSyncDefaultUpdates && !flags.enableUnifiedSyncLane,
)
) {
expect(Scheduler).toFlushAndYield([
// Since 'g' is in a transition, we'll process 'd' separately first.
// That causes us to process 'd' with 'e' and 'f' rebased.
'd',
'e',
'f',
// Then we'll re-process everything for 'g'.
'a',
'b',
'c',
'd',
'e',
'f',
'g',
]);
} else {
expect(Scheduler).toFlushAndYield([
// Then we'll re-process everything for 'g'.
'a',
'b',
'c',
'd',
'e',
'f',
'g',
]);
}
expect(Scheduler).toFlushAndYield(['a', 'b', 'c', 'd', 'e', 'f', 'g']);
expect(ReactNoop.getChildren()).toEqual([span('fg')]);
});
@@ -708,29 +641,19 @@ describe('ReactIncrementalUpdates', () => {
pushToLog('B'),
);
});
if (gate(flags => flags.enableUnifiedSyncLane)) {
expect(Scheduler).toHaveYielded([
'Committed: B',
'Committed: BCD',
'Committed: ABCD',
]);
} else {
expect(Scheduler).toHaveYielded([
// A and B are pending. B is higher priority, so we'll render that first.
'Committed: B',
// Because A comes first in the queue, we're now in rebase mode. B must
// be rebased on top of A. Also, in a layout effect, we received two new
// updates: C and D. C is user-blocking and D is synchronous.
//
// First render the synchronous update. What we're testing here is that
// B *is not dropped* even though it has lower than sync priority. That's
// because we already committed it. However, this render should not
// include C, because that update wasn't already committed.
'Committed: BD',
'Committed: BCD',
'Committed: ABCD',
]);
}
expect(Scheduler).toHaveYielded([
// A and B are pending. B is higher priority, so we'll render that first.
'Committed: B',
// Because A comes first in the queue, we're now in rebase mode. B must
// be rebased on top of A. Also, in a layout effect, we received two new
// updates: C and D. C is user-blocking and D is synchronous.
//
// First render the synchronous update. What we're testing here is that
// B *is not dropped* even though it has lower than sync priority. That's
// because we already committed it. C is batched with BD.
'Committed: BCD',
'Committed: ABCD',
]);
expect(root).toMatchRenderedOutput('ABCD');
});
@@ -776,29 +699,19 @@ describe('ReactIncrementalUpdates', () => {
pushToLog('B'),
);
});
if (gate(flags => flags.enableUnifiedSyncLane)) {
expect(Scheduler).toHaveYielded([
'Committed: B',
'Committed: BCD',
'Committed: ABCD',
]);
} else {
expect(Scheduler).toHaveYielded([
// A and B are pending. B is higher priority, so we'll render that first.
'Committed: B',
// Because A comes first in the queue, we're now in rebase mode. B must
// be rebased on top of A. Also, in a layout effect, we received two new
// updates: C and D. C is user-blocking and D is synchronous.
//
// First render the synchronous update. What we're testing here is that
// B *is not dropped* even though it has lower than sync priority. That's
// because we already committed it. However, this render should not
// include C, because that update wasn't already committed.
'Committed: BD',
'Committed: BCD',
'Committed: ABCD',
]);
}
expect(Scheduler).toHaveYielded([
// A and B are pending. B is higher priority, so we'll render that first.
'Committed: B',
// Because A comes first in the queue, we're now in rebase mode. B must
// be rebased on top of A. Also, in a layout effect, we received two new
// updates: C and D. C is user-blocking and D is synchronous.
//
// First render the synchronous update. What we're testing here is that
// B *is not dropped* even though it has lower than sync priority. That's
// because we already committed it. C is batched with BD.
'Committed: BCD',
'Committed: ABCD',
]);
expect(root).toMatchRenderedOutput('ABCD');
});
@@ -690,15 +690,10 @@ describe('ReactOffscreen', () => {
);
// Before the inner update can finish, we receive another pair of updates.
if (gate(flags => flags.enableUnifiedSyncLane)) {
React.startTransition(() => {
setOuter(2);
setInner(2);
});
} else {
React.startTransition(() => {
setOuter(2);
setInner(2);
}
});
// Also, before either of these new updates are processed, the hidden
// tree is revealed at high priority.
@@ -934,28 +934,16 @@ describe('ReactTransition', () => {
updateNormalPri();
});
if (gate(flags => flags.enableUnifiedSyncLane)) {
expect(Scheduler).toHaveYielded([
'Normal pri: 0',
'Commit',
expect(Scheduler).toHaveYielded([
// Finish transition update.
'Normal pri: 0',
'Commit',
// Normal pri update.
'Transition pri: 1',
'Normal pri: 1',
'Commit',
]);
} else {
expect(Scheduler).toHaveYielded([
// Finish transition update.
'Normal pri: 0',
'Commit',
// Normal pri update.
'Transition pri: 1',
'Normal pri: 1',
'Commit',
]);
}
// Normal pri update.
'Transition pri: 1',
'Normal pri: 1',
'Commit',
]);
expect(root).toMatchRenderedOutput('Transition pri: 1, Normal pri: 1');
});
@@ -1558,15 +1558,10 @@ describe('useMutableSource', () => {
expect(Scheduler).toFlushAndYieldThrough(['a0', 'b0']);
// Mutate in an event. This schedules a subscription update on a, which
// already mounted, but not b, which hasn't subscribed yet.
if (gate(flags => flags.enableUnifiedSyncLane)) {
React.startTransition(() => {
mutateA('a1');
mutateB('b1');
});
} else {
React.startTransition(() => {
mutateA('a1');
mutateB('b1');
}
});
// Mutate again at lower priority. This will schedule another subscription
// update on a, but not b. When b mounts and subscriptions, the value it
-2
View File
@@ -137,8 +137,6 @@ export const enableUseRefAccessWarning = false;
// Enables time slicing for updates that aren't wrapped in startTransition.
export const enableSyncDefaultUpdates = true;
export const enableUnifiedSyncLane = __EXPERIMENTAL__;
// Adds an opt-in to time slicing for updates that aren't wrapped in
// startTransition. Only relevant when enableSyncDefaultUpdates is disabled.
export const allowConcurrentByDefault = false;
@@ -68,7 +68,6 @@ export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = true;
export const enableSyncDefaultUpdates = true;
export const enableUnifiedSyncLane = false;
export const allowConcurrentByDefault = true;
export const enableCustomElementPropertySupport = false;
@@ -59,7 +59,6 @@ export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
export const enableSyncDefaultUpdates = true;
export const enableUnifiedSyncLane = false;
export const allowConcurrentByDefault = false;
export const enableCustomElementPropertySupport = false;
@@ -59,7 +59,6 @@ export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
export const enableSyncDefaultUpdates = true;
export const enableUnifiedSyncLane = __EXPERIMENTAL__;
export const allowConcurrentByDefault = false;
export const enableCustomElementPropertySupport = false;
@@ -58,7 +58,6 @@ export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
export const enableSyncDefaultUpdates = true;
export const enableUnifiedSyncLane = false;
export const allowConcurrentByDefault = true;
export const consoleManagedByDevToolsDuringStrictMode = false;
@@ -59,7 +59,6 @@ export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
export const enableSyncDefaultUpdates = true;
export const enableUnifiedSyncLane = false;
export const allowConcurrentByDefault = true;
export const enableCustomElementPropertySupport = false;
@@ -59,7 +59,6 @@ export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
export const enableSyncDefaultUpdates = true;
export const enableUnifiedSyncLane = __EXPERIMENTAL__;
export const allowConcurrentByDefault = false;
export const enableCustomElementPropertySupport = false;
@@ -59,7 +59,6 @@ export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
export const enableSyncDefaultUpdates = true;
export const enableUnifiedSyncLane = __EXPERIMENTAL__;
export const allowConcurrentByDefault = true;
export const enableCustomElementPropertySupport = false;
@@ -23,7 +23,6 @@ export const enableProfilerNestedUpdateScheduledHook = __VARIANT__;
export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
export const enableLazyContextPropagation = __VARIANT__;
export const enableSyncDefaultUpdates = __VARIANT__;
export const enableUnifiedSyncLane = __VARIANT__;
export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = __VARIANT__;
export const enableTransitionTracing = __VARIANT__;
// Enable this flag to help with concurrent mode debugging.
@@ -29,7 +29,6 @@ export const {
disableSchedulerTimeoutInWorkLoop,
enableLazyContextPropagation,
enableSyncDefaultUpdates,
enableUnifiedSyncLane,
enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay,
enableTransitionTracing,
} = dynamicFeatureFlags;
@@ -454,13 +454,9 @@ describe('useSubscription', () => {
observableA.next('a-2');
// Update again
if (gate(flags => flags.enableUnifiedSyncLane)) {
React.startTransition(() => {
renderer.update(<Parent observed={observableA} />);
});
} else {
React.startTransition(() => {
renderer.update(<Parent observed={observableA} />);
}
});
// Flush everything and ensure that the correct subscribable is used
expect(Scheduler).toFlushAndYield([