Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clean up enableUnifiedSyncLane flag #30062

Merged
merged 3 commits into from
Jun 24, 2024
Merged

Clean up enableUnifiedSyncLane flag #30062

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9564fcd
Clean up enableUnifiedSyncLane
Jun 24, 2024
7e7fdd0
Fix ReactIncrementalUpdates-test
Jun 24, 2024
ce39de9
Fix ReactSuspenseWithNoopRenderer-test
Jun 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 3 additions & 12 deletions packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,21 +313,12 @@ describe('ReactDOMFiberAsync', () => {
assertLog([]);
});
// Only the active updates have flushed
if (gate(flags => flags.enableUnifiedSyncLane)) {
expect(container.textContent).toEqual('ABC');
assertLog(['ABC']);
} else {
expect(container.textContent).toEqual('BC');
assertLog(['BC']);
}
expect(container.textContent).toEqual('ABC');
assertLog(['ABC']);

await act(() => {
instance.push('D');
if (gate(flags => flags.enableUnifiedSyncLane)) {
expect(container.textContent).toEqual('ABC');
} else {
expect(container.textContent).toEqual('BC');
}
expect(container.textContent).toEqual('ABC');
assertLog([]);
});
assertLog(['ABCD']);
Expand Down
16 changes: 6 additions & 10 deletions packages/react-reconciler/src/ReactFiberLane.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
enableRetryLaneExpiration,
enableSchedulingProfiler,
enableTransitionTracing,
enableUnifiedSyncLane,
enableUpdaterTracking,
syncLaneExpirationMs,
transitionLaneExpirationMs,
Expand Down Expand Up @@ -51,9 +50,8 @@ export const InputContinuousLane: Lane = /* */ 0b0000000000000000000
export const DefaultHydrationLane: Lane = /* */ 0b0000000000000000000000000010000;
export const DefaultLane: Lane = /* */ 0b0000000000000000000000000100000;

export const SyncUpdateLanes: Lane = enableUnifiedSyncLane
? SyncLane | InputContinuousLane | DefaultLane
: SyncLane;
export const SyncUpdateLanes: Lane =
SyncLane | InputContinuousLane | DefaultLane;

const TransitionHydrationLane: Lane = /* */ 0b0000000000000000000000001000000;
const TransitionLanes: Lanes = /* */ 0b0000000001111111111111110000000;
Expand Down Expand Up @@ -151,11 +149,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 !== 0) {
return pendingSyncLanes;
}
switch (getHighestPriorityLane(lanes)) {
case SyncHydrationLane:
Expand Down Expand Up @@ -826,7 +822,7 @@ export function getBumpedLaneForHydration(
const renderLane = getHighestPriorityLane(renderLanes);

let lane;
if (enableUnifiedSyncLane && (renderLane & SyncUpdateLanes) !== NoLane) {
if ((renderLane & SyncUpdateLanes) !== NoLane) {
lane = SyncHydrationLane;
} else {
switch (renderLane) {
Expand Down
9 changes: 2 additions & 7 deletions packages/react-reconciler/src/__tests__/Activity-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,15 +698,10 @@ describe('Activity', () => {
);

// 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.
Expand Down
14 changes: 2 additions & 12 deletions packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,7 @@ describe('ReactBlockingMode', () => {
);

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

// Now flush the first update
await waitForAll(['A1']);
expect(root).toMatchRenderedOutput('A1B1');
}
assertLog(['A1', 'B1']);
expect(root).toMatchRenderedOutput('A1B1');
});
});
8 changes: 2 additions & 6 deletions packages/react-reconciler/src/__tests__/ReactClassSetStateCallback-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,9 @@ describe('ReactClassSetStateCallback', () => {
assertLog([0]);

await act(() => {
if (gate(flags => flags.enableUnifiedSyncLane)) {
React.startTransition(() => {
app.setState({step: 1}, () => Scheduler.log('Callback 1'));
});
} else {
React.startTransition(() => {
app.setState({step: 1}, () => Scheduler.log('Callback 1'));
}
});
ReactNoop.flushSync(() => {
app.setState({step: 2}, () => Scheduler.log('Callback 2'));
});
Expand Down
11 changes: 2 additions & 9 deletions packages/react-reconciler/src/__tests__/ReactFlushSync-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,20 +102,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.
await waitForPaint(
gate(flags => flags.enableUnifiedSyncLane) ? ['1, 1'] : ['1, 0'],
);
await waitForPaint(['1, 1']);

// The remaining update is not sync
ReactDOM.flushSync();
assertLog([]);

if (gate(flags => flags.enableUnifiedSyncLane)) {
await waitForPaint([]);
} else {
// Now flush it.
await waitForPaint(['1, 1']);
}
await waitForPaint([]);
});
expect(getVisibleChildren(container)).toEqual('1, 1');

Expand Down
9 changes: 2 additions & 7 deletions packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,13 +541,8 @@ 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.
assertLog(['Compute state (1 -> 100)']);

Expand Down
17 changes: 3 additions & 14 deletions packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -899,15 +899,8 @@ describe('ReactHooksWithNoopRenderer', () => {
ReactNoop.flushSync(() => {
counter.current.dispatch(INCREMENT);
});
if (gate(flags => flags.enableUnifiedSyncLane)) {
assertLog(['Count: 4']);
expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 4" />);
} else {
assertLog(['Count: 1']);
expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 1" />);
await waitForAll(['Count: 4']);
expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 4" />);
}
assertLog(['Count: 4']);
expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 4" />);
});
});

Expand Down Expand Up @@ -1613,11 +1606,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)) {
assertLog(['Will set count to 1', 'Count: 1']);
} else {
assertLog(['Will set count to 1', 'Count: 2', 'Count: 1']);
}
assertLog(['Will set count to 1', 'Count: 1']);

expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 1" />);
});
Expand Down
124 changes: 17 additions & 107 deletions packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,7 @@ describe('ReactIncrementalUpdates', () => {
}

// Schedule some async updates
if (
gate(
flags =>
!flags.forceConcurrentByDefaultForTesting ||
flags.enableUnifiedSyncLane,
)
) {
if (gate(flags => !flags.forceConcurrentByDefaultForTesting)) {
React.startTransition(() => {
instance.setState(createUpdate('a'));
instance.setState(createUpdate('b'));
Expand All @@ -189,13 +183,7 @@ describe('ReactIncrementalUpdates', () => {
});

// The sync updates should have flushed, but not the async ones.
if (
gate(
flags =>
!flags.forceConcurrentByDefaultForTesting &&
flags.enableUnifiedSyncLane,
)
) {
if (gate(flags => !flags.forceConcurrentByDefaultForTesting)) {
assertLog(['d', 'e', 'f']);
expect(ReactNoop).toMatchRenderedOutput(<span prop="def" />);
} else {
Expand All @@ -207,40 +195,16 @@ 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.forceConcurrentByDefaultForTesting &&
!flags.enableUnifiedSyncLane,
)
) {
await waitForAll([
// 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 {
await waitForAll([
// Then we'll re-process everything for 'g'.
'a',
'b',
'c',
'd',
'e',
'f',
'g',
]);
}
await waitForAll([
// Then we'll re-process everything for 'g'.
'a',
'b',
'c',
'd',
'e',
'f',
'g',
]);
expect(ReactNoop).toMatchRenderedOutput(<span prop="abcdefg" />);
});

Expand All @@ -267,13 +231,7 @@ describe('ReactIncrementalUpdates', () => {
}

// Schedule some async updates
if (
gate(
flags =>
!flags.forceConcurrentByDefaultForTesting ||
flags.enableUnifiedSyncLane,
)
) {
if (gate(flags => !flags.forceConcurrentByDefaultForTesting)) {
React.startTransition(() => {
instance.setState(createUpdate('a'));
instance.setState(createUpdate('b'));
Expand Down Expand Up @@ -303,13 +261,7 @@ describe('ReactIncrementalUpdates', () => {
});

// The sync updates should have flushed, but not the async ones.
if (
gate(
flags =>
!flags.forceConcurrentByDefaultForTesting &&
flags.enableUnifiedSyncLane,
)
) {
if (gate(flags => !flags.forceConcurrentByDefaultForTesting)) {
assertLog(['d', 'e', 'f']);
} else {
// Update d was dropped and replaced by e.
Expand All @@ -320,13 +272,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.forceConcurrentByDefaultForTesting &&
!flags.enableUnifiedSyncLane,
)
) {
if (gate(flags => !flags.forceConcurrentByDefaultForTesting)) {
await waitForAll([
// Since 'g' is in a transition, we'll process 'd' separately first.
// That causes us to process 'd' with 'e' and 'f' rebased.
Expand Down Expand Up @@ -684,25 +630,7 @@ describe('ReactIncrementalUpdates', () => {
pushToLog('B'),
);
});
if (gate(flags => flags.enableUnifiedSyncLane)) {
assertLog(['Committed: B', 'Committed: BCD', 'Committed: ABCD']);
} else {
assertLog([
// 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',
]);
}
assertLog(['Committed: B', 'Committed: BCD', 'Committed: ABCD']);
expect(root).toMatchRenderedOutput('ABCD');
});

Expand Down Expand Up @@ -744,25 +672,7 @@ describe('ReactIncrementalUpdates', () => {
pushToLog('B'),
);
});
if (gate(flags => flags.enableUnifiedSyncLane)) {
assertLog(['Committed: B', 'Committed: BCD', 'Committed: ABCD']);
} else {
assertLog([
// 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',
]);
}
assertLog(['Committed: B', 'Committed: BCD', 'Committed: ABCD']);
expect(root).toMatchRenderedOutput('ABCD');
});

Expand Down
Loading
Loading