Skip to content

Commit 018c58c

Browse files
authored
Clean up enableSyncDefaultUpdates flag a bit (facebook#26858)
## Overview Does a few things: - Renames `enableSyncDefaultUpdates` to `forceConcurrentByDefaultForTesting` - Changes the way it's used so it's dead-code eliminated separate from `allowConcurrentByDefault` - Deletes a bunch of the gated code The gates that are deleted are unnecessary now. We were keeping them when we originally thought we would come back to being concurrent by default. But we've shifted and now sync-by default is the desired behavior long term, so there's no need to keep all these forked tests around. I'll follow up to delete more of the forked behavior if possible. Ideally we wouldn't need this flag even if we're still using `allowConcurrentByDefault`.
1 parent ae31d2e commit 018c58c

35 files changed

+316
-882
lines changed

packages/react-art/src/__tests__/ReactART-test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ describe('ReactART', () => {
364364
expect(onClick2).toBeCalled();
365365
});
366366

367-
// @gate !enableSyncDefaultUpdates
367+
// @gate forceConcurrentByDefaultForTesting
368368
it('can concurrently render with a "primary" renderer while sharing context', async () => {
369369
const CurrentRendererContext = React.createContext(null);
370370

packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -312,10 +312,10 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
312312
expect(container.textContent).toEqual('not hovered');
313313

314314
await waitFor(['hovered']);
315-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
316-
expect(container.textContent).toEqual('hovered');
317-
} else {
315+
if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
318316
expect(container.textContent).toEqual('not hovered');
317+
} else {
318+
expect(container.textContent).toEqual('hovered');
319319
}
320320
});
321321
expect(container.textContent).toEqual('hovered');

packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -2036,13 +2036,13 @@ describe('ReactDOMServerPartialHydration', () => {
20362036
suspend = true;
20372037

20382038
await act(async () => {
2039-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
2040-
await waitFor(['Before', 'After']);
2041-
} else {
2039+
if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
20422040
await waitFor(['Before']);
20432041
// This took a long time to render.
20442042
Scheduler.unstable_advanceTime(1000);
20452043
await waitFor(['After']);
2044+
} else {
2045+
await waitFor(['Before', 'After']);
20462046
}
20472047

20482048
// This will cause us to skip the second row completely.

packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js

+2-6
Original file line numberDiff line numberDiff line change
@@ -1984,13 +1984,9 @@ describe('DOMPluginEventSystem', () => {
19841984
log.length = 0;
19851985

19861986
// Increase counter
1987-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
1988-
React.startTransition(() => {
1989-
root.render(<Test counter={1} />);
1990-
});
1991-
} else {
1987+
React.startTransition(() => {
19921988
root.render(<Test counter={1} />);
1993-
}
1989+
});
19941990
// Yield before committing
19951991
await waitFor(['Test']);
19961992

packages/react-reconciler/src/ReactFiber.js

+7-4
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import {
3333
enableProfilerTimer,
3434
enableScopeAPI,
3535
enableLegacyHidden,
36-
enableSyncDefaultUpdates,
36+
forceConcurrentByDefaultForTesting,
3737
allowConcurrentByDefault,
3838
enableTransitionTracing,
3939
enableDebugTracing,
@@ -460,10 +460,13 @@ export function createHostRootFiber(
460460
}
461461
if (
462462
// We only use this flag for our repo tests to check both behaviors.
463-
// TODO: Flip this flag and rename it something like "forceConcurrentByDefaultForTesting"
464-
!enableSyncDefaultUpdates ||
463+
forceConcurrentByDefaultForTesting
464+
) {
465+
mode |= ConcurrentUpdatesByDefaultMode;
466+
} else if (
465467
// Only for internal experiments.
466-
(allowConcurrentByDefault && concurrentUpdatesByDefaultOverride)
468+
allowConcurrentByDefault &&
469+
concurrentUpdatesByDefaultOverride
467470
) {
468471
mode |= ConcurrentUpdatesByDefaultMode;
469472
}

packages/react-reconciler/src/__tests__/ReactExpiration-test.js

+62-98
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,22 @@ describe('ReactExpiration', () => {
125125
}
126126

127127
it('increases priority of updates as time progresses', async () => {
128-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
128+
if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
129+
ReactNoop.render(<span prop="done" />);
130+
expect(ReactNoop).toMatchRenderedOutput(null);
131+
132+
// Nothing has expired yet because time hasn't advanced.
133+
flushNextRenderIfExpired();
134+
expect(ReactNoop).toMatchRenderedOutput(null);
135+
// Advance time a bit, but not enough to expire the low pri update.
136+
ReactNoop.expire(4500);
137+
flushNextRenderIfExpired();
138+
expect(ReactNoop).toMatchRenderedOutput(null);
139+
// Advance by another second. Now the update should expire and flush.
140+
ReactNoop.expire(500);
141+
flushNextRenderIfExpired();
142+
expect(ReactNoop).toMatchRenderedOutput(<span prop="done" />);
143+
} else {
129144
ReactNoop.render(<Text text="Step 1" />);
130145
React.startTransition(() => {
131146
ReactNoop.render(<Text text="Step 2" />);
@@ -147,21 +162,6 @@ describe('ReactExpiration', () => {
147162
ReactNoop.expire(500);
148163
await unstable_waitForExpired(['Step 2']);
149164
expect(ReactNoop).toMatchRenderedOutput('Step 2');
150-
} else {
151-
ReactNoop.render(<span prop="done" />);
152-
expect(ReactNoop).toMatchRenderedOutput(null);
153-
154-
// Nothing has expired yet because time hasn't advanced.
155-
flushNextRenderIfExpired();
156-
expect(ReactNoop).toMatchRenderedOutput(null);
157-
// Advance time a bit, but not enough to expire the low pri update.
158-
ReactNoop.expire(4500);
159-
flushNextRenderIfExpired();
160-
expect(ReactNoop).toMatchRenderedOutput(null);
161-
// Advance by another second. Now the update should expire and flush.
162-
ReactNoop.expire(500);
163-
flushNextRenderIfExpired();
164-
expect(ReactNoop).toMatchRenderedOutput(<span prop="done" />);
165165
}
166166
});
167167

@@ -187,13 +187,9 @@ describe('ReactExpiration', () => {
187187

188188
// First, show what happens for updates in two separate events.
189189
// Schedule an update.
190-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
191-
React.startTransition(() => {
192-
ReactNoop.render(<TextClass text="A" />);
193-
});
194-
} else {
190+
React.startTransition(() => {
195191
ReactNoop.render(<TextClass text="A" />);
196-
}
192+
});
197193
// Advance the timer.
198194
Scheduler.unstable_advanceTime(2000);
199195
// Partially flush the first update, then interrupt it.
@@ -248,13 +244,10 @@ describe('ReactExpiration', () => {
248244

249245
// First, show what happens for updates in two separate events.
250246
// Schedule an update.
251-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
252-
React.startTransition(() => {
253-
ReactNoop.render(<TextClass text="A" />);
254-
});
255-
} else {
247+
React.startTransition(() => {
256248
ReactNoop.render(<TextClass text="A" />);
257-
}
249+
});
250+
258251
// Advance the timer.
259252
Scheduler.unstable_advanceTime(2000);
260253
// Partially flush the first update, then interrupt it.
@@ -320,13 +313,10 @@ describe('ReactExpiration', () => {
320313
}
321314

322315
// Initial mount
323-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
324-
React.startTransition(() => {
325-
ReactNoop.render(<App />);
326-
});
327-
} else {
316+
React.startTransition(() => {
328317
ReactNoop.render(<App />);
329-
}
318+
});
319+
330320
await waitForAll([
331321
'initial [A] [render]',
332322
'initial [B] [render]',
@@ -339,13 +329,10 @@ describe('ReactExpiration', () => {
339329
]);
340330

341331
// Partial update
342-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
343-
React.startTransition(() => {
344-
subscribers.forEach(s => s.setState({text: '1'}));
345-
});
346-
} else {
332+
React.startTransition(() => {
347333
subscribers.forEach(s => s.setState({text: '1'}));
348-
}
334+
});
335+
349336
await waitFor(['1 [A] [render]', '1 [B] [render]']);
350337

351338
// Before the update can finish, update again. Even though no time has
@@ -371,13 +358,9 @@ describe('ReactExpiration', () => {
371358
);
372359
}
373360

374-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
375-
React.startTransition(() => {
376-
root.render(<App />);
377-
});
378-
} else {
361+
React.startTransition(() => {
379362
root.render(<App />);
380-
}
363+
});
381364

382365
await waitFor(['A']);
383366
await waitFor(['B']);
@@ -404,13 +387,9 @@ describe('ReactExpiration', () => {
404387
</>
405388
);
406389
}
407-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
408-
React.startTransition(() => {
409-
root.render(<App />);
410-
});
411-
} else {
390+
React.startTransition(() => {
412391
root.render(<App />);
413-
}
392+
});
414393

415394
await waitFor(['A']);
416395
await waitFor(['B']);
@@ -429,7 +408,26 @@ describe('ReactExpiration', () => {
429408
jest.resetModules();
430409
Scheduler = require('scheduler');
431410

432-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
411+
if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
412+
// Before importing the renderer, advance the current time by a number
413+
// larger than the maximum allowed for bitwise operations.
414+
const maxSigned31BitInt = 1073741823;
415+
Scheduler.unstable_advanceTime(maxSigned31BitInt * 100);
416+
// Now import the renderer. On module initialization, it will read the
417+
// current time.
418+
ReactNoop = require('react-noop-renderer');
419+
ReactNoop.render('Hi');
420+
421+
// The update should not have expired yet.
422+
flushNextRenderIfExpired();
423+
await waitFor([]);
424+
expect(ReactNoop).toMatchRenderedOutput(null);
425+
// Advance the time some more to expire the update.
426+
Scheduler.unstable_advanceTime(10000);
427+
flushNextRenderIfExpired();
428+
await waitFor([]);
429+
expect(ReactNoop).toMatchRenderedOutput('Hi');
430+
} else {
433431
const InternalTestUtils = require('internal-test-utils');
434432
waitFor = InternalTestUtils.waitFor;
435433
assertLog = InternalTestUtils.assertLog;
@@ -446,14 +444,10 @@ describe('ReactExpiration', () => {
446444
React = require('react');
447445

448446
ReactNoop.render(<Text text="Step 1" />);
449-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
450-
React.startTransition(() => {
451-
ReactNoop.render(<Text text="Step 2" />);
452-
});
453-
await waitFor(['Step 1']);
454-
} else {
455-
ReactNoop.render('Hi');
456-
}
447+
React.startTransition(() => {
448+
ReactNoop.render(<Text text="Step 2" />);
449+
});
450+
await waitFor(['Step 1']);
457451

458452
// The update should not have expired yet.
459453
await unstable_waitForExpired([]);
@@ -464,25 +458,6 @@ describe('ReactExpiration', () => {
464458
Scheduler.unstable_advanceTime(10000);
465459
await unstable_waitForExpired(['Step 2']);
466460
expect(ReactNoop).toMatchRenderedOutput('Step 2');
467-
} else {
468-
// Before importing the renderer, advance the current time by a number
469-
// larger than the maximum allowed for bitwise operations.
470-
const maxSigned31BitInt = 1073741823;
471-
Scheduler.unstable_advanceTime(maxSigned31BitInt * 100);
472-
// Now import the renderer. On module initialization, it will read the
473-
// current time.
474-
ReactNoop = require('react-noop-renderer');
475-
ReactNoop.render('Hi');
476-
477-
// The update should not have expired yet.
478-
flushNextRenderIfExpired();
479-
await waitFor([]);
480-
expect(ReactNoop).toMatchRenderedOutput(null);
481-
// Advance the time some more to expire the update.
482-
Scheduler.unstable_advanceTime(10000);
483-
flushNextRenderIfExpired();
484-
await waitFor([]);
485-
expect(ReactNoop).toMatchRenderedOutput('Hi');
486461
}
487462
});
488463

@@ -494,13 +469,10 @@ describe('ReactExpiration', () => {
494469
// Before scheduling an update, advance the current time.
495470
Scheduler.unstable_advanceTime(10000);
496471

497-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
498-
React.startTransition(() => {
499-
ReactNoop.render('Hi');
500-
});
501-
} else {
472+
React.startTransition(() => {
502473
ReactNoop.render('Hi');
503-
}
474+
});
475+
504476
await unstable_waitForExpired([]);
505477
expect(ReactNoop).toMatchRenderedOutput(null);
506478

@@ -541,13 +513,9 @@ describe('ReactExpiration', () => {
541513

542514
// First demonstrate what happens when there's no starvation
543515
await act(async () => {
544-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
545-
React.startTransition(() => {
546-
updateNormalPri();
547-
});
548-
} else {
516+
React.startTransition(() => {
549517
updateNormalPri();
550-
}
518+
});
551519
await waitFor(['Sync pri: 0']);
552520
updateSyncPri();
553521
assertLog(['Sync pri: 1', 'Normal pri: 0']);
@@ -565,13 +533,9 @@ describe('ReactExpiration', () => {
565533

566534
// Do the same thing, but starve the first update
567535
await act(async () => {
568-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
569-
React.startTransition(() => {
570-
updateNormalPri();
571-
});
572-
} else {
536+
React.startTransition(() => {
573537
updateNormalPri();
574-
}
538+
});
575539
await waitFor(['Sync pri: 1']);
576540

577541
// This time, a lot of time has elapsed since the normal pri update

packages/react-reconciler/src/__tests__/ReactFlushSync-test.js

+2-6
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,9 @@ describe('ReactFlushSync', () => {
4949

5050
const root = ReactNoop.createRoot();
5151
await act(async () => {
52-
if (gate(flags => flags.enableSyncDefaultUpdates)) {
53-
React.startTransition(() => {
54-
root.render(<App />);
55-
});
56-
} else {
52+
React.startTransition(() => {
5753
root.render(<App />);
58-
}
54+
});
5955
// This will yield right before the passive effect fires
6056
await waitForPaint(['0, 0']);
6157

0 commit comments

Comments
 (0)