Skip to content

Commit f84cfb3

Browse files
committed
Suspense in Batched Mode
Should have same semantics as Concurrent Mode.
1 parent 67e8030 commit f84cfb3

8 files changed

+86
-21
lines changed

packages/react-reconciler/src/ReactFiberBeginWork.js

+7-6
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ import {
8787
NoMode,
8888
ProfileMode,
8989
StrictMode,
90+
BatchedMode,
9091
} from './ReactTypeOfMode';
9192
import {
9293
shouldSetTextContent,
@@ -1493,8 +1494,8 @@ function updateSuspenseComponent(
14931494
null,
14941495
);
14951496

1496-
if ((workInProgress.mode & ConcurrentMode) === NoMode) {
1497-
// Outside of concurrent mode, we commit the effects from the
1497+
if ((workInProgress.mode & BatchedMode) === NoMode) {
1498+
// Outside of batched mode, we commit the effects from the
14981499
// partially completed, timed-out tree, too.
14991500
const progressedState: SuspenseState = workInProgress.memoizedState;
15001501
const progressedPrimaryChild: Fiber | null =
@@ -1546,8 +1547,8 @@ function updateSuspenseComponent(
15461547
NoWork,
15471548
);
15481549

1549-
if ((workInProgress.mode & ConcurrentMode) === NoMode) {
1550-
// Outside of concurrent mode, we commit the effects from the
1550+
if ((workInProgress.mode & BatchedMode) === NoMode) {
1551+
// Outside of batched mode, we commit the effects from the
15511552
// partially completed, timed-out tree, too.
15521553
const progressedState: SuspenseState = workInProgress.memoizedState;
15531554
const progressedPrimaryChild: Fiber | null =
@@ -1629,8 +1630,8 @@ function updateSuspenseComponent(
16291630
// schedule a placement.
16301631
// primaryChildFragment.effectTag |= Placement;
16311632

1632-
if ((workInProgress.mode & ConcurrentMode) === NoMode) {
1633-
// Outside of concurrent mode, we commit the effects from the
1633+
if ((workInProgress.mode & BatchedMode) === NoMode) {
1634+
// Outside of batched mode, we commit the effects from the
16341635
// partially completed, timed-out tree, too.
16351636
const progressedState: SuspenseState = workInProgress.memoizedState;
16361637
const progressedPrimaryChild: Fiber | null =

packages/react-reconciler/src/ReactFiberCompleteWork.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import {
4343
EventComponent,
4444
EventTarget,
4545
} from 'shared/ReactWorkTags';
46-
import {ConcurrentMode, NoMode} from './ReactTypeOfMode';
46+
import {NoMode, BatchedMode} from './ReactTypeOfMode';
4747
import {
4848
Placement,
4949
Ref,
@@ -716,12 +716,12 @@ function completeWork(
716716
}
717717

718718
if (nextDidTimeout && !prevDidTimeout) {
719-
// If this subtreee is running in concurrent mode we can suspend,
719+
// If this subtreee is running in batched mode we can suspend,
720720
// otherwise we won't suspend.
721721
// TODO: This will still suspend a synchronous tree if anything
722722
// in the concurrent tree already suspended during this render.
723723
// This is a known bug.
724-
if ((workInProgress.mode & ConcurrentMode) !== NoMode) {
724+
if ((workInProgress.mode & BatchedMode) !== NoMode) {
725725
renderDidSuspend();
726726
}
727727
}

packages/react-reconciler/src/ReactFiberUnwindWork.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import {
4141
enableSuspenseServerRenderer,
4242
enableEventAPI,
4343
} from 'shared/ReactFeatureFlags';
44-
import {ConcurrentMode, NoMode} from './ReactTypeOfMode';
44+
import {NoMode, BatchedMode} from './ReactTypeOfMode';
4545
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent';
4646

4747
import {createCapturedValue} from './ReactCapturedValue';
@@ -223,15 +223,15 @@ function throwException(
223223
thenables.add(thenable);
224224
}
225225

226-
// If the boundary is outside of concurrent mode, we should *not*
226+
// If the boundary is outside of batched mode, we should *not*
227227
// suspend the commit. Pretend as if the suspended component rendered
228228
// null and keep rendering. In the commit phase, we'll schedule a
229229
// subsequent synchronous update to re-render the Suspense.
230230
//
231231
// Note: It doesn't matter whether the component that suspended was
232-
// inside a concurrent mode tree. If the Suspense is outside of it, we
232+
// inside a batched mode tree. If the Suspense is outside of it, we
233233
// should *not* suspend the commit.
234-
if ((workInProgress.mode & ConcurrentMode) === NoMode) {
234+
if ((workInProgress.mode & BatchedMode) === NoMode) {
235235
workInProgress.effectTag |= DidCapture;
236236

237237
// We're going to commit this fiber even though it didn't complete.

packages/react-reconciler/src/__tests__/ReactBatchedMode.test-internal.js

+64
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ let React;
22
let ReactFeatureFlags;
33
let ReactNoop;
44
let Scheduler;
5+
let ReactCache;
6+
let Suspense;
7+
let TextResource;
58

69
describe('ReactBatchedMode', () => {
710
beforeEach(() => {
@@ -12,13 +15,40 @@ describe('ReactBatchedMode', () => {
1215
React = require('react');
1316
ReactNoop = require('react-noop-renderer');
1417
Scheduler = require('scheduler');
18+
ReactCache = require('react-cache');
19+
Suspense = React.Suspense;
20+
21+
TextResource = ReactCache.unstable_createResource(([text, ms = 0]) => {
22+
return new Promise((resolve, reject) =>
23+
setTimeout(() => {
24+
Scheduler.yieldValue(`Promise resolved [${text}]`);
25+
resolve(text);
26+
}, ms),
27+
);
28+
}, ([text, ms]) => text);
1529
});
1630

1731
function Text(props) {
1832
Scheduler.yieldValue(props.text);
1933
return props.text;
2034
}
2135

36+
function AsyncText(props) {
37+
const text = props.text;
38+
try {
39+
TextResource.read([props.text, props.ms]);
40+
Scheduler.yieldValue(text);
41+
return props.text;
42+
} catch (promise) {
43+
if (typeof promise.then === 'function') {
44+
Scheduler.yieldValue(`Suspend! [${text}]`);
45+
} else {
46+
Scheduler.yieldValue(`Error! [${text}]`);
47+
}
48+
throw promise;
49+
}
50+
}
51+
2252
it('updates flush without yielding in the next event', () => {
2353
const root = ReactNoop.createSyncRoot();
2454

@@ -55,4 +85,38 @@ describe('ReactBatchedMode', () => {
5585
expect(Scheduler).toFlushExpired(['Hi', 'Layout effect']);
5686
expect(root).toMatchRenderedOutput('Hi');
5787
});
88+
89+
it('uses proper Suspense semantics, not legacy ones', async () => {
90+
const root = ReactNoop.createSyncRoot();
91+
root.render(
92+
<Suspense fallback={<Text text="Loading..." />}>
93+
<span>
94+
<Text text="A" />
95+
</span>
96+
<span>
97+
<AsyncText text="B" />
98+
</span>
99+
<span>
100+
<Text text="C" />
101+
</span>
102+
</Suspense>,
103+
);
104+
105+
expect(Scheduler).toFlushExpired(['A', 'Suspend! [B]', 'C', 'Loading...']);
106+
// In Legacy Mode, A and B would mount in a hidden primary tree. In Batched
107+
// and Concurrent Mode, nothing in the primary tree should mount. But the
108+
// fallback should mount immediately.
109+
expect(root).toMatchRenderedOutput('Loading...');
110+
111+
await jest.advanceTimersByTime(1000);
112+
expect(Scheduler).toHaveYielded(['Promise resolved [B]']);
113+
expect(Scheduler).toFlushExpired(['A', 'B', 'C']);
114+
expect(root).toMatchRenderedOutput(
115+
<React.Fragment>
116+
<span>A</span>
117+
<span>B</span>
118+
<span>C</span>
119+
</React.Fragment>,
120+
);
121+
});
58122
});

packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -941,7 +941,7 @@ describe('ReactHooksWithNoopRenderer', () => {
941941
});
942942

943943
it(
944-
'in sync mode, useEffect is deferred and updates finish synchronously ' +
944+
'in legacy mode, useEffect is deferred and updates finish synchronously ' +
945945
'(in a single batch)',
946946
() => {
947947
function Counter(props) {

packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,8 @@ describe('ReactSuspenseFuzz', () => {
175175
resetCache();
176176
ReactNoop.renderLegacySyncRoot(children);
177177
resolveAllTasks();
178-
const syncOutput = ReactNoop.getChildrenAsJSX();
179-
expect(syncOutput).toEqual(expectedOutput);
178+
const legacyOutput = ReactNoop.getChildrenAsJSX();
179+
expect(legacyOutput).toEqual(expectedOutput);
180180
ReactNoop.renderLegacySyncRoot(null);
181181

182182
resetCache();

packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ describe('ReactSuspensePlaceholder', () => {
303303
});
304304

305305
describe('when suspending during mount', () => {
306-
it('properly accounts for base durations when a suspended times out in a sync tree', () => {
306+
it('properly accounts for base durations when a suspended times out in a legacy tree', () => {
307307
ReactNoop.renderLegacySyncRoot(<App shouldSuspend={true} />);
308308
expect(Scheduler).toHaveYielded([
309309
'App',
@@ -373,7 +373,7 @@ describe('ReactSuspensePlaceholder', () => {
373373
});
374374

375375
describe('when suspending during update', () => {
376-
it('properly accounts for base durations when a suspended times out in a sync tree', () => {
376+
it('properly accounts for base durations when a suspended times out in a legacy tree', () => {
377377
ReactNoop.renderLegacySyncRoot(
378378
<App shouldSuspend={false} textRenderDuration={5} />,
379379
);

packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -869,7 +869,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
869869
);
870870
});
871871

872-
describe('sync mode', () => {
872+
describe('legacy mode mode', () => {
873873
it('times out immediately', async () => {
874874
function App() {
875875
return (
@@ -977,7 +977,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
977977

978978
it(
979979
'continues rendering asynchronously even if a promise is captured by ' +
980-
'a sync boundary (default mode)',
980+
'a sync boundary (legacy mode)',
981981
async () => {
982982
class UpdatingText extends React.Component {
983983
state = {text: this.props.initialText};
@@ -1109,7 +1109,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
11091109

11101110
it(
11111111
'continues rendering asynchronously even if a promise is captured by ' +
1112-
'a sync boundary (strict, non-concurrent)',
1112+
'a sync boundary (strict, legacy)',
11131113
async () => {
11141114
class UpdatingText extends React.Component {
11151115
state = {text: this.props.initialText};

0 commit comments

Comments
 (0)