@samitouri / QOS-React / commits / c21bcd627b

Clean up enableUnifiedSyncLane flag (#30062)

`enableUnifiedSyncLane` now passes everywhere. Let's clean it up Implemented with https://github.com/facebook/react/pull/27646 Flag enabled with https://github.com/facebook/react/pull/27646, https://github.com/facebook/react/pull/28269, https://github.com/facebook/react/pull/29052

Jack Pope committed Jun 24, 2024 at 11:18 UTC c21bcd627b6a8f31548edfc149dd3b879fea6558
20 files changed +62 -276
packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
+3 -12
@@ -313,21 +313,12 @@ describe('ReactDOMFiberAsync', () => {
313 assertLog([]);
314 });
315 // Only the active updates have flushed
316 - if (gate(flags => flags.enableUnifiedSyncLane)) {
317 - expect(container.textContent).toEqual('ABC');
318 - assertLog(['ABC']);
319 - } else {
320 - expect(container.textContent).toEqual('BC');
321 - assertLog(['BC']);
322 - }
316 + expect(container.textContent).toEqual('ABC');
317 + assertLog(['ABC']);
318
319 await act(() => {
320 instance.push('D');
326 - if (gate(flags => flags.enableUnifiedSyncLane)) {
327 - expect(container.textContent).toEqual('ABC');
328 - } else {
329 - expect(container.textContent).toEqual('BC');
330 - }
321 + expect(container.textContent).toEqual('ABC');
322 assertLog([]);
323 });
324 assertLog(['ABCD']);
packages/react-reconciler/src/ReactFiberLane.js
+6 -10
@@ -23,7 +23,6 @@ import {
23 enableRetryLaneExpiration,
24 enableSchedulingProfiler,
25 enableTransitionTracing,
26 - enableUnifiedSyncLane,
26 enableUpdaterTracking,
27 syncLaneExpirationMs,
28 transitionLaneExpirationMs,
@@ -51,9 +50,8 @@ export const InputContinuousLane: Lane = /* */ 0b0000000000000000000
50 export const DefaultHydrationLane: Lane = /* */ 0b0000000000000000000000000010000;
51 export const DefaultLane: Lane = /* */ 0b0000000000000000000000000100000;
52
54 -export const SyncUpdateLanes: Lane = enableUnifiedSyncLane
55 - ? SyncLane | InputContinuousLane | DefaultLane
56 - : SyncLane;
53 +export const SyncUpdateLanes: Lane =
54 + SyncLane | InputContinuousLane | DefaultLane;
55
56 const TransitionHydrationLane: Lane = /* */ 0b0000000000000000000000001000000;
57 const TransitionLanes: Lanes = /* */ 0b0000000001111111111111110000000;
@@ -151,11 +149,9 @@ let nextTransitionLane: Lane = TransitionLane1;
149 let nextRetryLane: Lane = RetryLane1;
150
151 function getHighestPriorityLanes(lanes: Lanes | Lane): Lanes {
154 - if (enableUnifiedSyncLane) {
155 - const pendingSyncLanes = lanes & SyncUpdateLanes;
156 - if (pendingSyncLanes !== 0) {
157 - return pendingSyncLanes;
158 - }
152 + const pendingSyncLanes = lanes & SyncUpdateLanes;
153 + if (pendingSyncLanes !== 0) {
154 + return pendingSyncLanes;
155 }
156 switch (getHighestPriorityLane(lanes)) {
157 case SyncHydrationLane:
@@ -826,7 +822,7 @@ export function getBumpedLaneForHydration(
822 const renderLane = getHighestPriorityLane(renderLanes);
823
824 let lane;
829 - if (enableUnifiedSyncLane && (renderLane & SyncUpdateLanes) !== NoLane) {
825 + if ((renderLane & SyncUpdateLanes) !== NoLane) {
826 lane = SyncHydrationLane;
827 } else {
828 switch (renderLane) {
packages/react-reconciler/src/__tests__/Activity-test.js
+2 -7
@@ -698,15 +698,10 @@ describe('Activity', () => {
698 );
699
700 // Before the inner update can finish, we receive another pair of updates.
701 - if (gate(flags => flags.enableUnifiedSyncLane)) {
702 - React.startTransition(() => {
703 - setOuter(2);
704 - setInner(2);
705 - });
706 - } else {
701 + React.startTransition(() => {
702 setOuter(2);
703 setInner(2);
709 - }
704 + });
705
706 // Also, before either of these new updates are processed, the hidden
707 // tree is revealed at high priority.
packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js
+2 -12
@@ -159,17 +159,7 @@ describe('ReactBlockingMode', () => {
159 );
160
161 // Now flush the first update
162 - if (gate(flags => flags.enableUnifiedSyncLane)) {
163 - assertLog(['A1', 'B1']);
164 - expect(root).toMatchRenderedOutput('A1B1');
165 - } else {
166 - // Only the second update should have flushed synchronously
167 - assertLog(['B1']);
168 - expect(root).toMatchRenderedOutput('A0B1');
169 -
170 - // Now flush the first update
171 - await waitForAll(['A1']);
172 - expect(root).toMatchRenderedOutput('A1B1');
173 - }
162 + assertLog(['A1', 'B1']);
163 + expect(root).toMatchRenderedOutput('A1B1');
164 });
165 });
packages/react-reconciler/src/__tests__/ReactClassSetStateCallback-test.js
+2 -6
@@ -39,13 +39,9 @@ describe('ReactClassSetStateCallback', () => {
39 assertLog([0]);
40
41 await act(() => {
42 - if (gate(flags => flags.enableUnifiedSyncLane)) {
43 - React.startTransition(() => {
44 - app.setState({step: 1}, () => Scheduler.log('Callback 1'));
45 - });
46 - } else {
42 + React.startTransition(() => {
43 app.setState({step: 1}, () => Scheduler.log('Callback 1'));
48 - }
44 + });
45 ReactNoop.flushSync(() => {
46 app.setState({step: 2}, () => Scheduler.log('Callback 2'));
47 });
packages/react-reconciler/src/__tests__/ReactFlushSync-test.js
+2 -9
@@ -102,20 +102,13 @@ describe('ReactFlushSync', () => {
102
103 // The passive effect will schedule a sync update and a normal update.
104 // They should commit in two separate batches. First the sync one.
105 - await waitForPaint(
106 - gate(flags => flags.enableUnifiedSyncLane) ? ['1, 1'] : ['1, 0'],
107 - );
105 + await waitForPaint(['1, 1']);
106
107 // The remaining update is not sync
108 ReactDOM.flushSync();
109 assertLog([]);
110
113 - if (gate(flags => flags.enableUnifiedSyncLane)) {
114 - await waitForPaint([]);
115 - } else {
116 - // Now flush it.
117 - await waitForPaint(['1, 1']);
118 - }
111 + await waitForPaint([]);
112 });
113 expect(getVisibleChildren(container)).toEqual('1, 1');
114
packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+2 -7
@@ -541,13 +541,8 @@ describe('ReactHooks', () => {
541 });
542 };
543
544 - if (gate(flags => flags.enableUnifiedSyncLane)) {
545 - // Update at transition priority
546 - React.startTransition(() => update(n => n * 100));
547 - } else {
548 - // Update at normal priority
549 - ReactTestRenderer.unstable_batchedUpdates(() => update(n => n * 100));
550 - }
544 + // Update at transition priority
545 + React.startTransition(() => update(n => n * 100));
546 // The new state is eagerly computed.
547 assertLog(['Compute state (1 -> 100)']);
548
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
+3 -14
@@ -899,15 +899,8 @@ describe('ReactHooksWithNoopRenderer', () => {
899 ReactNoop.flushSync(() => {
900 counter.current.dispatch(INCREMENT);
901 });
902 - if (gate(flags => flags.enableUnifiedSyncLane)) {
903 - assertLog(['Count: 4']);
904 - expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 4" />);
905 - } else {
906 - assertLog(['Count: 1']);
907 - expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 1" />);
908 - await waitForAll(['Count: 4']);
909 - expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 4" />);
910 - }
902 + assertLog(['Count: 4']);
903 + expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 4" />);
904 });
905 });
906
@@ -1613,11 +1606,7 @@ describe('ReactHooksWithNoopRenderer', () => {
1606 // As a result we, somewhat surprisingly, commit them in the opposite order.
1607 // This should be fine because any non-discrete set of work doesn't guarantee order
1608 // and easily could've happened slightly later too.
1616 - if (gate(flags => flags.enableUnifiedSyncLane)) {
1617 - assertLog(['Will set count to 1', 'Count: 1']);
1618 - } else {
1619 - assertLog(['Will set count to 1', 'Count: 2', 'Count: 1']);
1620 - }
1609 + assertLog(['Will set count to 1', 'Count: 1']);
1610
1611 expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 1" />);
1612 });
packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js
+29 -160
@@ -156,23 +156,11 @@ describe('ReactIncrementalUpdates', () => {
156 }
157
158 // Schedule some async updates
159 - if (
160 - gate(
161 - flags =>
162 - !flags.forceConcurrentByDefaultForTesting ||
163 - flags.enableUnifiedSyncLane,
164 - )
165 - ) {
166 - React.startTransition(() => {
167 - instance.setState(createUpdate('a'));
168 - instance.setState(createUpdate('b'));
169 - instance.setState(createUpdate('c'));
170 - });
171 - } else {
159 + React.startTransition(() => {
160 instance.setState(createUpdate('a'));
161 instance.setState(createUpdate('b'));
162 instance.setState(createUpdate('c'));
175 - }
163 + });
164
165 // Begin the updates but don't flush them yet
166 await waitFor(['a', 'b', 'c']);
@@ -189,58 +177,22 @@ describe('ReactIncrementalUpdates', () => {
177 });
178
179 // The sync updates should have flushed, but not the async ones.
192 - if (
193 - gate(
194 - flags =>
195 - !flags.forceConcurrentByDefaultForTesting &&
196 - flags.enableUnifiedSyncLane,
197 - )
198 - ) {
199 - assertLog(['d', 'e', 'f']);
200 - expect(ReactNoop).toMatchRenderedOutput(<span prop="def" />);
201 - } else {
202 - // Update d was dropped and replaced by e.
203 - assertLog(['e', 'f']);
204 - expect(ReactNoop).toMatchRenderedOutput(<span prop="ef" />);
205 - }
180 + assertLog(['d', 'e', 'f']);
181 + expect(ReactNoop).toMatchRenderedOutput(<span prop="def" />);
182
183 // Now flush the remaining work. Even though e and f were already processed,
184 // they should be processed again, to ensure that the terminal state
185 // is deterministic.
210 - if (
211 - gate(
212 - flags =>
213 - !flags.forceConcurrentByDefaultForTesting &&
214 - !flags.enableUnifiedSyncLane,
215 - )
216 - ) {
217 - await waitForAll([
218 - // Since 'g' is in a transition, we'll process 'd' separately first.
219 - // That causes us to process 'd' with 'e' and 'f' rebased.
220 - 'd',
221 - 'e',
222 - 'f',
223 - // Then we'll re-process everything for 'g'.
224 - 'a',
225 - 'b',
226 - 'c',
227 - 'd',
228 - 'e',
229 - 'f',
230 - 'g',
231 - ]);
232 - } else {
233 - await waitForAll([
234 - // Then we'll re-process everything for 'g'.
235 - 'a',
236 - 'b',
237 - 'c',
238 - 'd',
239 - 'e',
240 - 'f',
241 - 'g',
242 - ]);
243 - }
186 + await waitForAll([
187 + // Then we'll re-process everything for 'g'.
188 + 'a',
189 + 'b',
190 + 'c',
191 + 'd',
192 + 'e',
193 + 'f',
194 + 'g',
195 + ]);
196 expect(ReactNoop).toMatchRenderedOutput(<span prop="abcdefg" />);
197 });
198
@@ -267,23 +219,11 @@ describe('ReactIncrementalUpdates', () => {
219 }
220
221 // Schedule some async updates
270 - if (
271 - gate(
272 - flags =>
273 - !flags.forceConcurrentByDefaultForTesting ||
274 - flags.enableUnifiedSyncLane,
275 - )
276 - ) {
277 - React.startTransition(() => {
278 - instance.setState(createUpdate('a'));
279 - instance.setState(createUpdate('b'));
280 - instance.setState(createUpdate('c'));
281 - });
282 - } else {
222 + React.startTransition(() => {
223 instance.setState(createUpdate('a'));
224 instance.setState(createUpdate('b'));
225 instance.setState(createUpdate('c'));
286 - }
226 + });
227
228 // Begin the updates but don't flush them yet
229 await waitFor(['a', 'b', 'c']);
@@ -303,57 +243,22 @@ describe('ReactIncrementalUpdates', () => {
243 });
244
245 // The sync updates should have flushed, but not the async ones.
306 - if (
307 - gate(
308 - flags =>
309 - !flags.forceConcurrentByDefaultForTesting &&
310 - flags.enableUnifiedSyncLane,
311 - )
312 - ) {
313 - assertLog(['d', 'e', 'f']);
314 - } else {
315 - // Update d was dropped and replaced by e.
316 - assertLog(['e', 'f']);
317 - }
246 + assertLog(['d', 'e', 'f']);
247 expect(ReactNoop).toMatchRenderedOutput(<span prop="f" />);
248
249 // Now flush the remaining work. Even though e and f were already processed,
250 // they should be processed again, to ensure that the terminal state
251 // is deterministic.
323 - if (
324 - gate(
325 - flags =>
326 - !flags.forceConcurrentByDefaultForTesting &&
327 - !flags.enableUnifiedSyncLane,
328 - )
329 - ) {
330 - await waitForAll([
331 - // Since 'g' is in a transition, we'll process 'd' separately first.
332 - // That causes us to process 'd' with 'e' and 'f' rebased.
333 - 'd',
334 - 'e',
335 - 'f',
336 - // Then we'll re-process everything for 'g'.
337 - 'a',
338 - 'b',
339 - 'c',
340 - 'd',
341 - 'e',
342 - 'f',
343 - 'g',
344 - ]);
345 - } else {
346 - await waitForAll([
347 - // Then we'll re-process everything for 'g'.
348 - 'a',
349 - 'b',
350 - 'c',
351 - 'd',
352 - 'e',
353 - 'f',
354 - 'g',
355 - ]);
356 - }
252 + await waitForAll([
253 + // Then we'll re-process everything for 'g'.
254 + 'a',
255 + 'b',
256 + 'c',
257 + 'd',
258 + 'e',
259 + 'f',
260 + 'g',
261 + ]);
262 expect(ReactNoop).toMatchRenderedOutput(<span prop="fg" />);
263 });
264
@@ -684,25 +589,7 @@ describe('ReactIncrementalUpdates', () => {
589 pushToLog('B'),
590 );
591 });
687 - if (gate(flags => flags.enableUnifiedSyncLane)) {
688 - assertLog(['Committed: B', 'Committed: BCD', 'Committed: ABCD']);
689 - } else {
690 - assertLog([
691 - // A and B are pending. B is higher priority, so we'll render that first.
692 - 'Committed: B',
693 - // Because A comes first in the queue, we're now in rebase mode. B must
694 - // be rebased on top of A. Also, in a layout effect, we received two new
695 - // updates: C and D. C is user-blocking and D is synchronous.
696 - //
697 - // First render the synchronous update. What we're testing here is that
698 - // B *is not dropped* even though it has lower than sync priority. That's
699 - // because we already committed it. However, this render should not
700 - // include C, because that update wasn't already committed.
701 - 'Committed: BD',
702 - 'Committed: BCD',
703 - 'Committed: ABCD',
704 - ]);
705 - }
592 + assertLog(['Committed: B', 'Committed: BCD', 'Committed: ABCD']);
593 expect(root).toMatchRenderedOutput('ABCD');
594 });
595
@@ -744,25 +631,7 @@ describe('ReactIncrementalUpdates', () => {
631 pushToLog('B'),
632 );
633 });
747 - if (gate(flags => flags.enableUnifiedSyncLane)) {
748 - assertLog(['Committed: B', 'Committed: BCD', 'Committed: ABCD']);
749 - } else {
750 - assertLog([
751 - // A and B are pending. B is higher priority, so we'll render that first.
752 - 'Committed: B',
753 - // Because A comes first in the queue, we're now in rebase mode. B must
754 - // be rebased on top of A. Also, in a layout effect, we received two new
755 - // updates: C and D. C is user-blocking and D is synchronous.
756 - //
757 - // First render the synchronous update. What we're testing here is that
758 - // B *is not dropped* even though it has lower than sync priority. That's
759 - // because we already committed it. However, this render should not
760 - // include C, because that update wasn't already committed.
761 - 'Committed: BD',
762 - 'Committed: BCD',
763 - 'Committed: ABCD',
764 - ]);
765 - }
634 + assertLog(['Committed: B', 'Committed: BCD', 'Committed: ABCD']);
635 expect(root).toMatchRenderedOutput('ABCD');
636 });
637
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
+1 -3
@@ -3506,7 +3506,6 @@ describe('ReactSuspenseWithNoopRenderer', () => {
3506 });
3507
3508 // @gate enableLegacyCache
3509 - // @gate forceConcurrentByDefaultForTesting
3509 it('regression: ping at high priority causes update to be dropped', async () => {
3510 const {useState, useTransition} = React;
3511
@@ -3573,10 +3572,9 @@ describe('ReactSuspenseWithNoopRenderer', () => {
3572 });
3573
3574 await waitFor([
3576 - 'B',
3575 'Suspend! [A1]',
3576 'Loading...',
3579 -
3577 + 'B',
3578 'Suspend! [A2]',
3579 'Loading...',
3580 'Suspend! [B2]',
packages/react-reconciler/src/__tests__/ReactTransition-test.js
+8 -21
@@ -925,28 +925,15 @@ describe('ReactTransition', () => {
925 updateNormalPri();
926 });
927
928 - if (gate(flags => flags.enableUnifiedSyncLane)) {
929 - assertLog([
930 - 'Normal pri: 0',
931 - 'Commit',
932 -
933 - // Normal pri update.
934 - 'Transition pri: 1',
935 - 'Normal pri: 1',
936 - 'Commit',
937 - ]);
938 - } else {
939 - assertLog([
940 - // Finish transition update.
941 - 'Normal pri: 0',
942 - 'Commit',
928 + assertLog([
929 + 'Normal pri: 0',
930 + 'Commit',
931
944 - // Normal pri update.
945 - 'Transition pri: 1',
946 - 'Normal pri: 1',
947 - 'Commit',
948 - ]);
949 - }
932 + // Normal pri update.
933 + 'Transition pri: 1',
934 + 'Normal pri: 1',
935 + 'Commit',
936 + ]);
937
938 expect(root).toMatchRenderedOutput('Transition pri: 1, Normal pri: 1');
939 });
packages/shared/ReactFeatureFlags.js
-2
@@ -210,8 +210,6 @@ export const enableUseDeferredValueInitialArg = true;
210 // Enables time slicing for updates that aren't wrapped in startTransition.
211 export const forceConcurrentByDefaultForTesting = false;
212
213 -export const enableUnifiedSyncLane = true;
214 -
213 // Adds an opt-in to time slicing for updates that aren't wrapped in startTransition.
214 export const allowConcurrentByDefault = false;
215
packages/shared/forks/ReactFeatureFlags.native-fb.js
-1
@@ -79,7 +79,6 @@ export const enableSuspenseCallback = false;
79 export const enableTaint = true;
80 export const enableTransitionTracing = false;
81 export const enableTrustedTypesIntegration = false;
82 -export const enableUnifiedSyncLane = true;
82 export const enableUpdaterTracking = __PROFILE__;
83 export const enableUseDeferredValueInitialArg = true;
84 export const enableUseEffectEventHook = false;
packages/shared/forks/ReactFeatureFlags.native-oss.js
-1
@@ -71,7 +71,6 @@ export const enableSuspenseCallback = false;
71 export const enableTaint = true;
72 export const enableTransitionTracing = false;
73 export const enableTrustedTypesIntegration = false;
74 -export const enableUnifiedSyncLane = true;
74 export const enableUseDeferredValueInitialArg = true;
75 export const enableUseEffectEventHook = false;
76 export const enableUseMemoCacheHook = true;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
-1
@@ -54,7 +54,6 @@ export const disableSchedulerTimeoutInWorkLoop = false;
54 export const enableLazyContextPropagation = false;
55 export const enableLegacyHidden = false;
56 export const forceConcurrentByDefaultForTesting = false;
57 -export const enableUnifiedSyncLane = __EXPERIMENTAL__;
57 export const allowConcurrentByDefault = false;
58
59 export const consoleManagedByDevToolsDuringStrictMode = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js
-1
@@ -66,7 +66,6 @@ export const enableSuspenseCallback = false;
66 export const enableTaint = true;
67 export const enableTransitionTracing = false;
68 export const enableTrustedTypesIntegration = false;
69 -export const enableUnifiedSyncLane = true;
69 export const enableUpdaterTracking = false;
70 export const enableUseDeferredValueInitialArg = __EXPERIMENTAL__;
71 export const enableUseEffectEventHook = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
-1
@@ -56,7 +56,6 @@ export const disableSchedulerTimeoutInWorkLoop = false;
56 export const enableLazyContextPropagation = false;
57 export const enableLegacyHidden = false;
58 export const forceConcurrentByDefaultForTesting = false;
59 -export const enableUnifiedSyncLane = true;
59 export const allowConcurrentByDefault = true;
60
61 export const consoleManagedByDevToolsDuringStrictMode = false;
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
-1
@@ -16,7 +16,6 @@
16 export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
17 export const enableLazyContextPropagation = __VARIANT__;
18 export const forceConcurrentByDefaultForTesting = __VARIANT__;
19 -export const enableUnifiedSyncLane = __VARIANT__;
19 export const enableTransitionTracing = __VARIANT__;
20 export const enableDeferRootSchedulingToMicrotask = __VARIANT__;
21 export const alwaysThrottleRetries = true;
packages/shared/forks/ReactFeatureFlags.www.js
-1
@@ -18,7 +18,6 @@ export const {
18 enableTrustedTypesIntegration,
19 enableDebugTracing,
20 enableLazyContextPropagation,
21 - enableUnifiedSyncLane,
21 enableRetryLaneExpiration,
22 enableTransitionTracing,
23 enableDeferRootSchedulingToMicrotask,
packages/use-subscription/src/__tests__/useSubscription-test.js
+2 -6
@@ -434,13 +434,9 @@ describe('useSubscription', () => {
434 observableA.next('a-2');
435
436 // Update again
437 - if (gate(flags => flags.enableUnifiedSyncLane)) {
438 - React.startTransition(() => {
439 - root.render(<Parent observed={observableA} />);
440 - });
441 - } else {
437 + React.startTransition(() => {
438 root.render(<Parent observed={observableA} />);
443 - }
439 + });
440
441 // Flush everything and ensure that the correct subscribable is used
442 await waitForAll([