From 5379b6123f171bb48cc8a9c435c11ccb9f8ff0e7 Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Thu, 5 Jan 2023 15:21:35 -0800 Subject: [PATCH] Batch sync, default and continuous lanes (#25700) ## Summary This is the other approach for unifying default and sync lane https://github.com/facebook/react/pull/25524. The approach in that PR is to merge default and continuous lane into the sync lane, and use a new field to track the priority. But there are a couple places that field will be needed, and it is difficult to correctly reset the field when there is no sync lane. In this PR we take the other approach that doesn't remove any lane, but batch them to get the behavior we want. ## How did you test this change? yarn test Co-authored-by: Andrew Clark --- .../src/__tests__/ReactDOMFiberAsync-test.js | 27 +- ...DOMServerPartialHydration-test.internal.js | 120 ++++----- ...MServerSelectiveHydration-test.internal.js | 26 +- .../react-reconciler/src/ReactFiberLane.js | 93 ++++--- .../__tests__/ReactBatching-test.internal.js | 18 +- .../ReactClassSetStateCallback-test.js | 14 +- .../src/__tests__/ReactExpiration-test.js | 4 +- .../src/__tests__/ReactFlushSync-test.js | 12 +- .../src/__tests__/ReactHooks-test.internal.js | 10 +- .../ReactHooksWithNoopRenderer-test.js | 36 ++- .../__tests__/ReactIncrementalUpdates-test.js | 240 ++++++++++-------- .../src/__tests__/ReactOffscreen-test.js | 11 +- .../__tests__/ReactOffscreenSuspense-test.js | 10 +- .../src/__tests__/ReactTransition-test.js | 30 ++- .../useMutableSource-test.internal.js | 11 +- packages/shared/ReactFeatureFlags.js | 2 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.native.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.testing.js | 1 + .../forks/ReactFeatureFlags.testing.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + .../src/__tests__/useSubscription-test.js | 8 +- 26 files changed, 414 insertions(+), 267 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js index a94cbe367a8ae..5ed762afb86a1 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js @@ -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.enableUnifiedSyncLane)) { + 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.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']); + } // Flush the async updates Scheduler.unstable_flushAll(); expect(container.textContent).toEqual('ABCD'); - expect(ops).toEqual(['BC', 'ABCD']); + if (gate(flags => flags.enableUnifiedSyncLane)) { + expect(ops).toEqual(['ABC', 'ABCD']); + } else { + expect(ops).toEqual(['BC', 'ABCD']); + } }); // @gate www diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index f2bec4ec6a689..6da64f9cda992 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -449,10 +449,9 @@ describe('ReactDOMServerPartialHydration', () => { expect(deleted.length).toBe(0); // Performing an update should force it to delete the boundary - root.render(); - - Scheduler.unstable_flushAll(); - jest.runAllTimers(); + await act(async () => { + root.render(); + }); expect(hydrated.length).toBe(1); expect(deleted.length).toBe(1); @@ -945,13 +944,12 @@ describe('ReactDOMServerPartialHydration', () => { root.render(); // At the same time, resolving the promise so that rendering can complete. - suspend = false; - resolve(); - await promise; - // This should first complete the hydration and then flush the update onto the hydrated state. - Scheduler.unstable_flushAll(); - jest.runAllTimers(); + await act(async () => { + suspend = false; + resolve(); + await promise; + }); // The new span should be the same since we should have successfully hydrated // before changing it. @@ -1093,9 +1091,9 @@ describe('ReactDOMServerPartialHydration', () => { expect(ref.current).toBe(null); // Render an update, but leave it still suspended. - root.render(); - Scheduler.unstable_flushAll(); - jest.runAllTimers(); + await act(async () => { + root.render(); + }); // Flushing now should delete the existing content and show the fallback. @@ -1104,12 +1102,11 @@ describe('ReactDOMServerPartialHydration', () => { expect(container.textContent).toBe('Loading...'); // Unsuspending shows the content. - suspend = false; - resolve(); - await promise; - - Scheduler.unstable_flushAll(); - jest.runAllTimers(); + await act(async () => { + suspend = false; + resolve(); + await promise; + }); const span = container.getElementsByTagName('span')[0]; expect(span.textContent).toBe('Hi'); @@ -1174,23 +1171,21 @@ describe('ReactDOMServerPartialHydration', () => { expect(ref.current).toBe(span); // Render an update, but leave it still suspended. - root.render(); - // Flushing now should delete the existing content and show the fallback. - Scheduler.unstable_flushAll(); - jest.runAllTimers(); + await act(async () => { + root.render(); + }); expect(container.getElementsByTagName('span').length).toBe(1); expect(ref.current).toBe(span); expect(container.textContent).toBe(''); // Unsuspending shows the content. - suspend = false; - resolve(); - await promise; - - Scheduler.unstable_flushAll(); - jest.runAllTimers(); + await act(async () => { + suspend = false; + resolve(); + await promise; + }); expect(span.textContent).toBe('Hi'); expect(span.className).toBe('hi'); @@ -1252,20 +1247,21 @@ describe('ReactDOMServerPartialHydration', () => { expect(ref.current).toBe(null); // Render an update, but leave it still suspended. - root.render(); - // Flushing now should delete the existing content and show the fallback. - Scheduler.unstable_flushAll(); - jest.runAllTimers(); + await act(async () => { + root.render(); + }); expect(container.getElementsByTagName('span').length).toBe(0); expect(ref.current).toBe(null); expect(container.textContent).toBe('Loading...'); // Unsuspending shows the content. - suspend = false; - resolve(); - await promise; + await act(async () => { + suspend = false; + resolve(); + await promise; + }); Scheduler.unstable_flushAll(); jest.runAllTimers(); @@ -1490,13 +1486,12 @@ describe('ReactDOMServerPartialHydration', () => { ); // At the same time, resolving the promise so that rendering can complete. - suspend = false; - resolve(); - await promise; - // This should first complete the hydration and then flush the update onto the hydrated state. - Scheduler.unstable_flushAll(); - jest.runAllTimers(); + await act(async () => { + suspend = false; + resolve(); + await promise; + }); // Since this should have been hydrated, this should still be the same span. const newSpan = container.getElementsByTagName('span')[0]; @@ -1569,27 +1564,25 @@ describe('ReactDOMServerPartialHydration', () => { expect(ref.current).toBe(null); // Render an update, but leave it still suspended. - root.render( - - - , - ); - // Flushing now should delete the existing content and show the fallback. - Scheduler.unstable_flushAll(); - jest.runAllTimers(); + await act(async () => { + root.render( + + + , + ); + }); expect(container.getElementsByTagName('span').length).toBe(0); expect(ref.current).toBe(null); expect(container.textContent).toBe('Loading...'); // Unsuspending shows the content. - suspend = false; - resolve(); - await promise; - - Scheduler.unstable_flushAll(); - jest.runAllTimers(); + await act(async () => { + suspend = false; + resolve(); + await promise; + }); const span = container.getElementsByTagName('span')[0]; expect(span.textContent).toBe('Hi'); @@ -2320,16 +2313,15 @@ describe('ReactDOMServerPartialHydration', () => { // Render an update, which will be higher or the same priority as pinging the hydration. // The new update doesn't suspend. - root.render( - - - , - ); - // Since we're still suspended on the original data, we can't hydrate. // This will force all expiration times to flush. - Scheduler.unstable_flushAll(); - jest.runAllTimers(); + await act(async () => { + root.render( + + + , + ); + }); // This will now be a new span because we weren't able to hydrate before const newSpan = container.getElementsByTagName('span')[0]; diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index c6da651a813aa..ca795e98d1410 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1786,7 +1786,7 @@ describe('ReactDOMServerSelectiveHydration', () => { document.body.removeChild(container); }); - it('can force hydration in response to sync update', () => { + it('can force hydration in response to sync update', async () => { function Child({text}) { Scheduler.unstable_yieldValue(`Child ${text}`); return (spanRef = ref)}>{text}; @@ -1812,15 +1812,17 @@ describe('ReactDOMServerSelectiveHydration', () => { const root = ReactDOMClient.hydrateRoot(container, ); expect(Scheduler).toFlushUntilNextPaint(['App A']); - ReactDOM.flushSync(() => { - root.render(); + await act(async () => { + ReactDOM.flushSync(() => { + root.render(); + }); }); expect(Scheduler).toHaveYielded(['App B', 'Child A', 'App B', 'Child B']); expect(initialSpan).toBe(spanRef); }); // @gate experimental || www - it('can force hydration in response to continuous update', () => { + it('can force hydration in response to continuous update', async () => { function Child({text}) { Scheduler.unstable_yieldValue(`Child ${text}`); return (spanRef = ref)}>{text}; @@ -1846,14 +1848,17 @@ describe('ReactDOMServerSelectiveHydration', () => { const root = ReactDOMClient.hydrateRoot(container, ); expect(Scheduler).toFlushUntilNextPaint(['App A']); - TODO_scheduleContinuousSchedulerTask(() => { - root.render(); + await act(async () => { + TODO_scheduleContinuousSchedulerTask(() => { + root.render(); + }); }); - expect(Scheduler).toFlushAndYield(['App B', 'Child A', 'App B', 'Child B']); + + expect(Scheduler).toHaveYielded(['App B', 'Child A', 'App B', 'Child B']); expect(initialSpan).toBe(spanRef); }); - it('can force hydration in response to default update', () => { + it('can force hydration in response to default update', async () => { function Child({text}) { Scheduler.unstable_yieldValue(`Child ${text}`); return (spanRef = ref)}>{text}; @@ -1878,11 +1883,10 @@ describe('ReactDOMServerSelectiveHydration', () => { const initialSpan = container.getElementsByTagName('span')[0]; const root = ReactDOMClient.hydrateRoot(container, ); expect(Scheduler).toFlushUntilNextPaint(['App A']); - - ReactDOM.unstable_batchedUpdates(() => { + await act(async () => { root.render(); }); - expect(Scheduler).toFlushAndYield(['App B', 'Child A', 'App B', 'Child B']); + expect(Scheduler).toHaveYielded(['App B', 'Child A', 'App B', 'Child B']); expect(initialSpan).toBe(spanRef); }); diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 97a28f88099e0..42b2fc6f9a3d6 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -23,6 +23,7 @@ import { enableUpdaterTracking, allowConcurrentByDefault, enableTransitionTracing, + enableUnifiedSyncLane, } from 'shared/ReactFeatureFlags'; import {isDevToolsPresent} from './ReactFiberDevToolsHook'; import {ConcurrentUpdatesByDefaultMode, NoMode} from './ReactTypeOfMode'; @@ -45,6 +46,8 @@ export const InputContinuousLane: Lane = /* */ 0b0000000000000000000 export const DefaultHydrationLane: Lane = /* */ 0b0000000000000000000000000010000; export const DefaultLane: Lane = /* */ 0b0000000000000000000000000100000; +export const SyncUpdateLanes: Lane = /* */ 0b0000000000000000000000000101010; + const TransitionHydrationLane: Lane = /* */ 0b0000000000000000000000001000000; const TransitionLanes: Lanes = /* */ 0b0000000011111111111111110000000; const TransitionLane1: Lane = /* */ 0b0000000000000000000000010000000; @@ -133,6 +136,12 @@ 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; + } + } switch (getHighestPriorityLane(lanes)) { case SyncHydrationLane: return SyncHydrationLane; @@ -754,46 +763,50 @@ export function getBumpedLaneForHydration( const renderLane = getHighestPriorityLane(renderLanes); let lane; - switch (renderLane) { - case SyncLane: - lane = SyncHydrationLane; - break; - case InputContinuousLane: - lane = InputContinuousHydrationLane; - break; - case DefaultLane: - lane = DefaultHydrationLane; - break; - case TransitionLane1: - case TransitionLane2: - case TransitionLane3: - case TransitionLane4: - case TransitionLane5: - case TransitionLane6: - case TransitionLane7: - case TransitionLane8: - case TransitionLane9: - case TransitionLane10: - case TransitionLane11: - case TransitionLane12: - case TransitionLane13: - case TransitionLane14: - case TransitionLane15: - case TransitionLane16: - case RetryLane1: - case RetryLane2: - case RetryLane3: - case RetryLane4: - lane = TransitionHydrationLane; - break; - case IdleLane: - lane = IdleHydrationLane; - break; - default: - // Everything else is already either a hydration lane, or shouldn't - // be retried at a hydration lane. - lane = NoLane; - break; + if (enableUnifiedSyncLane && (renderLane & SyncUpdateLanes) !== NoLane) { + lane = SyncHydrationLane; + } else { + switch (renderLane) { + case SyncLane: + lane = SyncHydrationLane; + break; + case InputContinuousLane: + lane = InputContinuousHydrationLane; + break; + case DefaultLane: + lane = DefaultHydrationLane; + break; + case TransitionLane1: + case TransitionLane2: + case TransitionLane3: + case TransitionLane4: + case TransitionLane5: + case TransitionLane6: + case TransitionLane7: + case TransitionLane8: + case TransitionLane9: + case TransitionLane10: + case TransitionLane11: + case TransitionLane12: + case TransitionLane13: + case TransitionLane14: + case TransitionLane15: + case TransitionLane16: + case RetryLane1: + case RetryLane2: + case RetryLane3: + case RetryLane4: + lane = TransitionHydrationLane; + break; + case IdleLane: + lane = IdleHydrationLane; + break; + default: + // Everything else is already either a hydration lane, or shouldn't + // be retried at a hydration lane. + lane = NoLane; + break; + } } // Check if the lane we chose is suspended. If so, that indicates that we diff --git a/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js b/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js index 7998d2a23177e..f9938a79b53d9 100644 --- a/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js @@ -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.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'); + } }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactClassSetStateCallback-test.js b/packages/react-reconciler/src/__tests__/ReactClassSetStateCallback-test.js index 0f97bb5997186..1342bd7310baa 100644 --- a/packages/react-reconciler/src/__tests__/ReactClassSetStateCallback-test.js +++ b/packages/react-reconciler/src/__tests__/ReactClassSetStateCallback-test.js @@ -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.enableUnifiedSyncLane)) { + 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'), diff --git a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js index 24127589dfd90..72ca257fefa8d 100644 --- a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js @@ -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]', diff --git a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js index 07cc3437a5e24..5802253e1c1f1 100644 --- a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js +++ b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js @@ -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.enableUnifiedSyncLane) ? ['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.enableUnifiedSyncLane)) { + expect(Scheduler).toFlushUntilNextPaint([]); + } else { + // Now flush it. + expect(Scheduler).toFlushUntilNextPaint(['1, 1']); + } }); expect(root).toMatchRenderedOutput('1, 1'); }); diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 63dc3c04d0a37..ebf4a25446929 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -568,9 +568,13 @@ describe('ReactHooks', () => { }); }; - // Update at normal priority - ReactTestRenderer.unstable_batchedUpdates(() => update(n => n * 100)); - + 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)); + } // The new state is eagerly computed. expect(Scheduler).toHaveYielded(['Compute state (1 -> 100)']); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 25d19be6b48d3..8bbf49c2cc514 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -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(); @@ -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.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')]); + } }); }); @@ -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.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(ReactNoop.getChildren()).toEqual([span('Count: 1')]); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js index 4d667b0ab6707..b56fdc85361b2 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js @@ -47,7 +47,7 @@ describe('ReactIncrementalUpdates', () => { state = {}; componentDidMount() { Scheduler.unstable_yieldValue('commit'); - ReactNoop.deferredUpdates(() => { + React.startTransition(() => { // Has low priority this.setState({b: 'b'}); this.setState({c: 'c'}); @@ -111,13 +111,13 @@ describe('ReactIncrementalUpdates', () => { expect(Scheduler).toFlushAndYield(['render', 'componentDidMount']); ReactNoop.flushSync(() => { - ReactNoop.deferredUpdates(() => { + React.startTransition(() => { instance.setState({x: 'x'}); instance.setState({y: 'y'}); }); instance.setState({a: 'a'}); instance.setState({b: 'b'}); - ReactNoop.deferredUpdates(() => { + React.startTransition(() => { instance.updater.enqueueReplaceState(instance, {c: 'c'}); instance.setState({d: 'd'}); }); @@ -162,7 +162,11 @@ describe('ReactIncrementalUpdates', () => { } // Schedule some async updates - if (gate(flags => flags.enableSyncDefaultUpdates)) { + if ( + gate( + flags => flags.enableSyncDefaultUpdates || flags.enableUnifiedSyncLane, + ) + ) { React.startTransition(() => { instance.setState(createUpdate('a')); instance.setState(createUpdate('b')); @@ -179,23 +183,37 @@ describe('ReactIncrementalUpdates', () => { expect(ReactNoop.getChildren()).toEqual([span('')]); // Schedule some more updates at different priorities - if (gate(flags => flags.enableSyncDefaultUpdates)) { - instance.setState(createUpdate('d')); - ReactNoop.flushSync(() => { - instance.setState(createUpdate('e')); - instance.setState(createUpdate('f')); - }); - React.startTransition(() => { - instance.setState(createUpdate('g')); - }); + instance.setState(createUpdate('d')); + ReactNoop.flushSync(() => { + instance.setState(createUpdate('e')); + instance.setState(createUpdate('f')); + }); + React.startTransition(() => { + instance.setState(createUpdate('g')); + }); - // The sync updates should have flushed, but not the async ones + // The sync updates should have flushed, but not the async ones. + if ( + gate( + flags => flags.enableSyncDefaultUpdates && flags.enableUnifiedSyncLane, + ) + ) { + expect(Scheduler).toHaveYielded(['d', 'e', 'f']); + expect(ReactNoop.getChildren()).toEqual([span('def')]); + } else { + // Update d was dropped and replaced by e. expect(Scheduler).toHaveYielded(['e', 'f']); expect(ReactNoop.getChildren()).toEqual([span('ef')]); + } - // 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. + // 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. @@ -211,25 +229,19 @@ describe('ReactIncrementalUpdates', () => { 'f', 'g', ]); - expect(ReactNoop.getChildren()).toEqual([span('abcdefg')]); } else { - instance.setState(createUpdate('d')); - ReactNoop.flushSync(() => { - instance.setState(createUpdate('e')); - instance.setState(createUpdate('f')); - }); - instance.setState(createUpdate('g')); - - // The sync updates should have flushed, but not the async ones - expect(Scheduler).toHaveYielded(['e', 'f']); - expect(ReactNoop.getChildren()).toEqual([span('ef')]); - - // 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. - expect(Scheduler).toFlushAndYield(['a', 'b', 'c', 'd', 'e', 'f', 'g']); - expect(ReactNoop.getChildren()).toEqual([span('abcdefg')]); + expect(Scheduler).toFlushAndYield([ + // Then we'll re-process everything for 'g'. + 'a', + 'b', + 'c', + 'd', + 'e', + 'f', + 'g', + ]); } + expect(ReactNoop.getChildren()).toEqual([span('abcdefg')]); }); it('can abort an update, schedule a replaceState, and resume', () => { @@ -261,7 +273,11 @@ describe('ReactIncrementalUpdates', () => { } // Schedule some async updates - if (gate(flags => flags.enableSyncDefaultUpdates)) { + if ( + gate( + flags => flags.enableSyncDefaultUpdates || flags.enableUnifiedSyncLane, + ) + ) { React.startTransition(() => { instance.setState(createUpdate('a')); instance.setState(createUpdate('b')); @@ -278,26 +294,39 @@ describe('ReactIncrementalUpdates', () => { expect(ReactNoop.getChildren()).toEqual([span('')]); // Schedule some more updates at different priorities - if (gate(flags => flags.enableSyncDefaultUpdates)) { - instance.setState(createUpdate('d')); + instance.setState(createUpdate('d')); - ReactNoop.flushSync(() => { - instance.setState(createUpdate('e')); - // No longer a public API, but we can test that it works internally by - // reaching into the updater. - instance.updater.enqueueReplaceState(instance, createUpdate('f')); - }); - React.startTransition(() => { - instance.setState(createUpdate('g')); - }); + ReactNoop.flushSync(() => { + instance.setState(createUpdate('e')); + // No longer a public API, but we can test that it works internally by + // reaching into the updater. + instance.updater.enqueueReplaceState(instance, createUpdate('f')); + }); + React.startTransition(() => { + instance.setState(createUpdate('g')); + }); - // The sync updates should have flushed, but not the async ones. + // The sync updates should have flushed, but not the async ones. + if ( + gate( + flags => flags.enableSyncDefaultUpdates && flags.enableUnifiedSyncLane, + ) + ) { + expect(Scheduler).toHaveYielded(['d', 'e', 'f']); + } else { + // Update d was dropped and replaced by e. expect(Scheduler).toHaveYielded(['e', 'f']); - expect(ReactNoop.getChildren()).toEqual([span('f')]); - - // 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. + } + expect(ReactNoop.getChildren()).toEqual([span('f')]); + + // 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. @@ -313,28 +342,19 @@ describe('ReactIncrementalUpdates', () => { 'f', 'g', ]); - expect(ReactNoop.getChildren()).toEqual([span('fg')]); } else { - instance.setState(createUpdate('d')); - ReactNoop.flushSync(() => { - instance.setState(createUpdate('e')); - // No longer a public API, but we can test that it works internally by - // reaching into the updater. - instance.updater.enqueueReplaceState(instance, createUpdate('f')); - }); - instance.setState(createUpdate('g')); - - // The sync updates should have flushed, but not the async ones. Update d - // was dropped and replaced by e. - expect(Scheduler).toHaveYielded(['e', 'f']); - expect(ReactNoop.getChildren()).toEqual([span('f')]); - - // 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. - expect(Scheduler).toFlushAndYield(['a', 'b', 'c', 'd', 'e', 'f', 'g']); - expect(ReactNoop.getChildren()).toEqual([span('fg')]); + expect(Scheduler).toFlushAndYield([ + // Then we'll re-process everything for 'g'. + 'a', + 'b', + 'c', + 'd', + 'e', + 'f', + 'g', + ]); } + expect(ReactNoop.getChildren()).toEqual([span('fg')]); }); it('passes accumulation of previous updates to replaceState updater function', () => { @@ -688,21 +708,29 @@ describe('ReactIncrementalUpdates', () => { pushToLog('B'), ); }); - 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', - ]); + 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(root).toMatchRenderedOutput('ABCD'); }); @@ -748,21 +776,29 @@ describe('ReactIncrementalUpdates', () => { pushToLog('B'), ); }); - 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', - ]); + 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(root).toMatchRenderedOutput('ABCD'); }); diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js index d723c44a5d308..cffcf617824ae 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js @@ -690,8 +690,15 @@ describe('ReactOffscreen', () => { ); // Before the inner update can finish, we receive another pair of updates. - setOuter(2); - setInner(2); + if (gate(flags => flags.enableUnifiedSyncLane)) { + React.startTransition(() => { + setOuter(2); + setInner(2); + }); + } else { + setOuter(2); + setInner(2); + } // Also, before either of these new updates are processed, the hidden // tree is revealed at high priority. diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreenSuspense-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreenSuspense-test.js index e1a6ca49fd64c..272979fab6444 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreenSuspense-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreenSuspense-test.js @@ -381,7 +381,9 @@ describe('ReactOffscreen', () => { expect(root).toMatchRenderedOutput(); await act(async () => { - setStep(1); + React.startTransition(() => { + setStep(1); + }); ReactNoop.flushSync(() => { setText('B'); }); @@ -513,8 +515,10 @@ describe('ReactOffscreen', () => { // Before the tree commits, schedule a concurrent event. The inner update // is to a tree that's just about to be hidden. - setOuter(2); - setInner(2); + startTransition(() => { + setOuter(2); + setInner(2); + }); // Commit the previous render. jest.runAllTimers(); diff --git a/packages/react-reconciler/src/__tests__/ReactTransition-test.js b/packages/react-reconciler/src/__tests__/ReactTransition-test.js index 560bb527b1b0e..dd0b9e0d0fedc 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransition-test.js +++ b/packages/react-reconciler/src/__tests__/ReactTransition-test.js @@ -934,16 +934,28 @@ describe('ReactTransition', () => { updateNormalPri(); }); - expect(Scheduler).toHaveYielded([ - // Finish transition update. - 'Normal pri: 0', - 'Commit', + if (gate(flags => flags.enableUnifiedSyncLane)) { + expect(Scheduler).toHaveYielded([ + 'Normal pri: 0', + 'Commit', - // Normal pri update. - 'Transition pri: 1', - 'Normal pri: 1', - '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', + ]); + } expect(root).toMatchRenderedOutput('Transition pri: 1, Normal pri: 1'); }); diff --git a/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js b/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js index 6321168d70aa3..c4ca0ae1de995 100644 --- a/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js +++ b/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js @@ -1558,8 +1558,15 @@ 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. - mutateA('a1'); - mutateB('b1'); + if (gate(flags => flags.enableUnifiedSyncLane)) { + React.startTransition(() => { + mutateA('a1'); + mutateB('b1'); + }); + } else { + 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 diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 8fcd13894c33a..24dc67d374692 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -151,6 +151,8 @@ 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; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 6d84e76612421..c43847ff08df5 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -72,6 +72,7 @@ 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; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 9b5c301b7d5ba..401362abc7f90 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -63,6 +63,7 @@ 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; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 45561ee4c5fec..96fd0dd2cca18 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -63,6 +63,7 @@ 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; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index dace39942cf2a..bcdcd974509a8 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -62,6 +62,7 @@ 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; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 47ed0529927c6..f2dcfc00f9b55 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -63,6 +63,7 @@ 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; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index bfcce69fe7308..1dda88f6bc176 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -63,6 +63,7 @@ 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; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index e45ef9d1af50c..cd4535e761a10 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -63,6 +63,7 @@ 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; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 882871de6f507..12b4ba8d35fde 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -24,6 +24,7 @@ export const enableProfilerNestedUpdateScheduledHook = __VARIANT__; export const disableSchedulerTimeoutInWorkLoop = __VARIANT__; export const enableLazyContextPropagation = __VARIANT__; export const enableSyncDefaultUpdates = __VARIANT__; +export const enableUnifiedSyncLane = __VARIANT__; export const consoleManagedByDevToolsDuringStrictMode = __VARIANT__; export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = __VARIANT__; export const enableClientRenderFallbackOnTextMismatch = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 89fae798d37b2..73a592286cb88 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -31,6 +31,7 @@ export const { disableSchedulerTimeoutInWorkLoop, enableLazyContextPropagation, enableSyncDefaultUpdates, + enableUnifiedSyncLane, enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay, enableClientRenderFallbackOnTextMismatch, enableTransitionTracing, diff --git a/packages/use-subscription/src/__tests__/useSubscription-test.js b/packages/use-subscription/src/__tests__/useSubscription-test.js index 985312abd1d64..ee89c3dbb9562 100644 --- a/packages/use-subscription/src/__tests__/useSubscription-test.js +++ b/packages/use-subscription/src/__tests__/useSubscription-test.js @@ -454,7 +454,13 @@ describe('useSubscription', () => { observableA.next('a-2'); // Update again - renderer.update(); + if (gate(flags => flags.enableUnifiedSyncLane)) { + React.startTransition(() => { + renderer.update(); + }); + } else { + renderer.update(); + } // Flush everything and ensure that the correct subscribable is used expect(Scheduler).toFlushAndYield([