Allow suspending in the shell during hydration (#23304)
* Allow suspending in the shell during hydration Builds on behavior added in #23267. Initial hydration should be allowed to suspend in the shell. In practice, this happens because the code for the outer shell hasn't loaded yet. Currently if you try to do this, it errors because it expects there to be a parent Suspense boundary, because without a fallback we can't produce a consistent tree. However, for non-sync updates, we don't need to produce a consistent tree immediately — we can delay the commit until the data resolves. In #23267, I added support for suspending without a parent boundary if the update was wrapped with `startTransition`. Here, I've expanded this to include hydration, too. I wonder if we should expand this even further to include all non-sync/ discrete updates. * Allow suspending in shell for all non-sync updates Instead of erroring, we can delay the commit. The only time we'll continue to error when there's no parent Suspense boundary is during sync/discrete updates, because those are expected to produce a complete tree synchronously to maintain consistency with external state.
Andrew Clark committed
Feb 16, 2022 at 11:47 UTC
f7f7ed089eb04a3504762425baf1d673701c736b
7 files changed
+259
-40
packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js
new
+216
@@ -0,0 +1,216 @@
1
+/**
2
+ * Copyright (c) Facebook, Inc. and its affiliates.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ *
7
+ * @emails react-core
8
+ */
9
+
10
+let JSDOM;
11
+let React;
12
+let ReactDOM;
13
+let Scheduler;
14
+let clientAct;
15
+let ReactDOMFizzServer;
16
+let Stream;
17
+let document;
18
+let writable;
19
+let container;
20
+let buffer = '';
21
+let hasErrored = false;
22
+let fatalError = undefined;
23
+let textCache;
24
+
25
+describe('ReactDOMFizzShellHydration', () => {
26
+ beforeEach(() => {
27
+ jest.resetModules();
28
+ JSDOM = require('jsdom').JSDOM;
29
+ React = require('react');
30
+ ReactDOM = require('react-dom');
31
+ Scheduler = require('scheduler');
32
+ clientAct = require('jest-react').act;
33
+ ReactDOMFizzServer = require('react-dom/server');
34
+ Stream = require('stream');
35
+
36
+ textCache = new Map();
37
+
38
+ // Test Environment
39
+ const jsdom = new JSDOM(
40
+ '<!DOCTYPE html><html><head></head><body><div id="container">',
41
+ {
42
+ runScripts: 'dangerously',
43
+ },
44
+ );
45
+ document = jsdom.window.document;
46
+ container = document.getElementById('container');
47
+
48
+ buffer = '';
49
+ hasErrored = false;
50
+
51
+ writable = new Stream.PassThrough();
52
+ writable.setEncoding('utf8');
53
+ writable.on('data', chunk => {
54
+ buffer += chunk;
55
+ });
56
+ writable.on('error', error => {
57
+ hasErrored = true;
58
+ fatalError = error;
59
+ });
60
+ });
61
+
62
+ async function serverAct(callback) {
63
+ await callback();
64
+ // Await one turn around the event loop.
65
+ // This assumes that we'll flush everything we have so far.
66
+ await new Promise(resolve => {
67
+ setImmediate(resolve);
68
+ });
69
+ if (hasErrored) {
70
+ throw fatalError;
71
+ }
72
+ // JSDOM doesn't support stream HTML parser so we need to give it a proper fragment.
73
+ // We also want to execute any scripts that are embedded.
74
+ // We assume that we have now received a proper fragment of HTML.
75
+ const bufferedContent = buffer;
76
+ buffer = '';
77
+ const fakeBody = document.createElement('body');
78
+ fakeBody.innerHTML = bufferedContent;
79
+ while (fakeBody.firstChild) {
80
+ const node = fakeBody.firstChild;
81
+ if (node.nodeName === 'SCRIPT') {
82
+ const script = document.createElement('script');
83
+ script.textContent = node.textContent;
84
+ fakeBody.removeChild(node);
85
+ container.appendChild(script);
86
+ } else {
87
+ container.appendChild(node);
88
+ }
89
+ }
90
+ }
91
+
92
+ function resolveText(text) {
93
+ const record = textCache.get(text);
94
+ if (record === undefined) {
95
+ const newRecord = {
96
+ status: 'resolved',
97
+ value: text,
98
+ };
99
+ textCache.set(text, newRecord);
100
+ } else if (record.status === 'pending') {
101
+ const thenable = record.value;
102
+ record.status = 'resolved';
103
+ record.value = text;
104
+ thenable.pings.forEach(t => t());
105
+ }
106
+ }
107
+
108
+ function readText(text) {
109
+ const record = textCache.get(text);
110
+ if (record !== undefined) {
111
+ switch (record.status) {
112
+ case 'pending':
113
+ throw record.value;
114
+ case 'rejected':
115
+ throw record.value;
116
+ case 'resolved':
117
+ return record.value;
118
+ }
119
+ } else {
120
+ Scheduler.unstable_yieldValue(`Suspend! [${text}]`);
121
+
122
+ const thenable = {
123
+ pings: [],
124
+ then(resolve) {
125
+ if (newRecord.status === 'pending') {
126
+ thenable.pings.push(resolve);
127
+ } else {
128
+ Promise.resolve().then(() => resolve(newRecord.value));
129
+ }
130
+ },
131
+ };
132
+
133
+ const newRecord = {
134
+ status: 'pending',
135
+ value: thenable,
136
+ };
137
+ textCache.set(text, newRecord);
138
+
139
+ throw thenable;
140
+ }
141
+ }
142
+
143
+ // function Text({text}) {
144
+ // Scheduler.unstable_yieldValue(text);
145
+ // return text;
146
+ // }
147
+
148
+ function AsyncText({text}) {
149
+ readText(text);
150
+ Scheduler.unstable_yieldValue(text);
151
+ return text;
152
+ }
153
+
154
+ function resetTextCache() {
155
+ textCache = new Map();
156
+ }
157
+
158
+ test('suspending in the shell during hydration', async () => {
159
+ const div = React.createRef(null);
160
+
161
+ function App() {
162
+ return (
163
+ <div ref={div}>
164
+ <AsyncText text="Shell" />
165
+ </div>
166
+ );
167
+ }
168
+
169
+ // Server render
170
+ await resolveText('Shell');
171
+ await serverAct(async () => {
172
+ const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
173
+ pipe(writable);
174
+ });
175
+ expect(Scheduler).toHaveYielded(['Shell']);
176
+ const dehydratedDiv = container.getElementsByTagName('div')[0];
177
+
178
+ // Clear the cache and start rendering on the client
179
+ resetTextCache();
180
+
181
+ // Hydration suspends because the data for the shell hasn't loaded yet
182
+ await clientAct(async () => {
183
+ ReactDOM.hydrateRoot(container, <App />);
184
+ });
185
+ expect(Scheduler).toHaveYielded(['Suspend! [Shell]']);
186
+ expect(div.current).toBe(null);
187
+ expect(container.textContent).toBe('Shell');
188
+
189
+ // The shell loads and hydration finishes
190
+ await clientAct(async () => {
191
+ await resolveText('Shell');
192
+ });
193
+ expect(Scheduler).toHaveYielded(['Shell']);
194
+ expect(div.current).toBe(dehydratedDiv);
195
+ expect(container.textContent).toBe('Shell');
196
+ });
197
+
198
+ test('suspending in the shell during a normal client render', async () => {
199
+ // Same as previous test but during a normal client render, no hydration
200
+ function App() {
201
+ return <AsyncText text="Shell" />;
202
+ }
203
+
204
+ const root = ReactDOM.createRoot(container);
205
+ await clientAct(async () => {
206
+ root.render(<App />);
207
+ });
208
+ expect(Scheduler).toHaveYielded(['Suspend! [Shell]']);
209
+
210
+ await clientAct(async () => {
211
+ await resolveText('Shell');
212
+ });
213
+ expect(Scheduler).toHaveYielded(['Shell']);
214
+ expect(container.textContent).toBe('Shell');
215
+ });
216
+});
packages/react-reconciler/src/ReactFiberLane.new.js
+4
@@ -443,6 +443,10 @@ export function getLanesToRetrySynchronouslyOnError(root: FiberRoot): Lanes {
443
return NoLanes;
444
}
445
446
+export function includesSyncLane(lanes: Lanes) {
447
+ return (lanes & SyncLane) !== NoLanes;
448
+}
449
+
450
export function includesNonIdleWork(lanes: Lanes) {
451
return (lanes & NonIdleLanes) !== NoLanes;
452
}
packages/react-reconciler/src/ReactFiberLane.old.js
+4
@@ -443,6 +443,10 @@ export function getLanesToRetrySynchronouslyOnError(root: FiberRoot): Lanes {
443
return NoLanes;
444
}
445
446
+export function includesSyncLane(lanes: Lanes) {
447
+ return (lanes & SyncLane) !== NoLanes;
448
+}
449
+
450
export function includesNonIdleWork(lanes: Lanes) {
451
return (lanes & NonIdleLanes) !== NoLanes;
452
}
packages/react-reconciler/src/ReactFiberThrow.new.js
+13
-13
@@ -79,7 +79,7 @@ import {
79
includesSomeLane,
80
mergeLanes,
81
pickArbitraryLane,
82
- includesOnlyTransitions,
82
+ includesSyncLane,
83
} from './ReactFiberLane.new';
84
import {
85
getIsHydrating,
@@ -480,25 +480,25 @@ function throwException(
480
attachRetryListener(suspenseBoundary, root, wakeable, rootRenderLanes);
481
return;
482
} else {
483
- // No boundary was found. If we're inside startTransition, this is OK.
483
+ // No boundary was found. Unless this is a sync update, this is OK.
484
// We can suspend and wait for more data to arrive.
485
486
- if (includesOnlyTransitions(rootRenderLanes)) {
487
- // This is a transition. Suspend. Since we're not activating a Suspense
488
- // boundary, this will unwind all the way to the root without performing
489
- // a second pass to render a fallback. (This is arguably how refresh
490
- // transitions should work, too, since we're not going to commit the
491
- // fallbacks anyway.)
486
+ if (!includesSyncLane(rootRenderLanes)) {
487
+ // This is not a sync update. Suspend. Since we're not activating a
488
+ // Suspense boundary, this will unwind all the way to the root without
489
+ // performing a second pass to render a fallback. (This is arguably how
490
+ // refresh transitions should work, too, since we're not going to commit
491
+ // the fallbacks anyway.)
492
+ //
493
+ // This case also applies to initial hydration.
494
attachPingListener(root, wakeable, rootRenderLanes);
495
renderDidSuspendDelayIfPossible();
496
return;
497
}
498
497
- // We're not in a transition. We treat this case like an error because
498
- // discrete renders are expected to finish synchronously to maintain
499
- // consistency with external state.
500
- // TODO: This will error during non-transition concurrent renders, too.
501
- // But maybe it shouldn't?
499
+ // This is a sync/discrete update. We treat this case like an error
500
+ // because discrete renders are expected to produce a complete tree
501
+ // synchronously to maintain consistency with external state.
502
503
// TODO: We should never call getComponentNameFromFiber in production.
504
// Log a warning or something to prevent us from accidentally bundling it.
packages/react-reconciler/src/ReactFiberThrow.old.js
+13
-13
@@ -79,7 +79,7 @@ import {
79
includesSomeLane,
80
mergeLanes,
81
pickArbitraryLane,
82
- includesOnlyTransitions,
82
+ includesSyncLane,
83
} from './ReactFiberLane.old';
84
import {
85
getIsHydrating,
@@ -480,25 +480,25 @@ function throwException(
480
attachRetryListener(suspenseBoundary, root, wakeable, rootRenderLanes);
481
return;
482
} else {
483
- // No boundary was found. If we're inside startTransition, this is OK.
483
+ // No boundary was found. Unless this is a sync update, this is OK.
484
// We can suspend and wait for more data to arrive.
485
486
- if (includesOnlyTransitions(rootRenderLanes)) {
487
- // This is a transition. Suspend. Since we're not activating a Suspense
488
- // boundary, this will unwind all the way to the root without performing
489
- // a second pass to render a fallback. (This is arguably how refresh
490
- // transitions should work, too, since we're not going to commit the
491
- // fallbacks anyway.)
486
+ if (!includesSyncLane(rootRenderLanes)) {
487
+ // This is not a sync update. Suspend. Since we're not activating a
488
+ // Suspense boundary, this will unwind all the way to the root without
489
+ // performing a second pass to render a fallback. (This is arguably how
490
+ // refresh transitions should work, too, since we're not going to commit
491
+ // the fallbacks anyway.)
492
+ //
493
+ // This case also applies to initial hydration.
494
attachPingListener(root, wakeable, rootRenderLanes);
495
renderDidSuspendDelayIfPossible();
496
return;
497
}
498
497
- // We're not in a transition. We treat this case like an error because
498
- // discrete renders are expected to finish synchronously to maintain
499
- // consistency with external state.
500
- // TODO: This will error during non-transition concurrent renders, too.
501
- // But maybe it shouldn't?
499
+ // This is a sync/discrete update. We treat this case like an error
500
+ // because discrete renders are expected to produce a complete tree
501
+ // synchronously to maintain consistency with external state.
502
503
// TODO: We should never call getComponentNameFromFiber in production.
504
// Log a warning or something to prevent us from accidentally bundling it.
packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
-11
@@ -389,17 +389,6 @@ describe('ReactSuspense', () => {
389
expect(root).toMatchRenderedOutput('Hi');
390
});
391
392
- it('throws if tree suspends and none of the Suspense ancestors have a boundary', () => {
393
- ReactTestRenderer.create(<AsyncText text="Hi" ms={1000} />, {
394
- unstable_isConcurrent: true,
395
- });
396
-
397
- expect(Scheduler).toFlushAndThrow(
398
- 'AsyncText suspended while rendering, but no fallback UI was specified.',
399
- );
400
- expect(Scheduler).toHaveYielded(['Suspend! [Hi]', 'Suspend! [Hi]']);
401
- });
402
-
392
it('updates memoized child of suspense component when context updates (simple memo)', () => {
393
const {useContext, createContext, useState, memo} = React;
394
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
+9
-3
@@ -1003,9 +1003,15 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1003
});
1004
1005
// @gate enableCache
1006
- it('throws a helpful error when an update is suspends without a placeholder', () => {
1007
- ReactNoop.render(<AsyncText text="Async" />);
1008
- expect(Scheduler).toFlushAndThrow(
1006
+ it('errors when an update suspends without a placeholder during a sync update', () => {
1007
+ // This is an error because sync/discrete updates are expected to produce
1008
+ // a complete tree immediately to maintain consistency with external state
1009
+ // — we can't delay the commit.
1010
+ expect(() => {
1011
+ ReactNoop.flushSync(() => {
1012
+ ReactNoop.render(<AsyncText text="Async" />);
1013
+ });
1014
+ }).toThrow(
1015
'AsyncText suspended while rendering, but no fallback UI was specified.',
1016
);
1017
});