Change .model getter to .readRoot method (#18382)
Originally the idea was to hide all suspending behind getters or proxies. However, this has some issues with perf on hot code like React elements. It also makes it too easy to accidentally access it the first time in an effect or callback where things aren't allowed to suspend. Making it an explicit method call avoids this issue. All other suspending has moved to explicit lazy blocks (and soon elements). The only thing remaining is the root. We could require the root to be an element or block but that creates an unfortunate indirection unnecessarily. Instead, I expose a readRoot method on the response. Typically we try to avoid virtual dispatch but in this case, it's meant that you build abstractions on top of a Flight response so passing it a round is useful.
Sebastian Markbåge committed
Mar 25, 2020 at 11:47 UTC
a6924d77b10dcf4108f5ffdfd37078b0864c57d5
12 files changed
+122
-135
fixtures/flight-browser/index.html
+3
-3
@@ -70,20 +70,20 @@
70
let blob = await responseToDisplay.blob();
71
let url = URL.createObjectURL(blob);
72
73
- let data = ReactFlightDOMClient.readFromFetch(
73
+ let data = ReactFlightDOMClient.createFromFetch(
74
fetch(url)
75
);
76
// The client also supports XHR streaming.
77
// var xhr = new XMLHttpRequest();
78
// xhr.open('GET', url);
79
- // let data = ReactFlightDOMClient.readFromXHR(xhr);
79
+ // let data = ReactFlightDOMClient.createFromXHR(xhr);
80
// xhr.send();
81
82
renderResult(data);
83
}
84
85
function Shell({ data }) {
86
- let model = data.model;
86
+ let model = data.readRoot();
87
return <div>
88
<Suspense fallback="...">
89
<h1>{model.title}</h1>
fixtures/flight/src/App.js
+1
-1
@@ -1,7 +1,7 @@
1
import React, {Suspense} from 'react';
2
3
function Content({data}) {
4
- return data.model.content;
4
+ return data.readRoot().content;
5
}
6
7
function App({data}) {
fixtures/flight/src/index.js
+2
-1
@@ -3,5 +3,6 @@ import ReactDOM from 'react-dom';
3
import ReactFlightDOMClient from 'react-flight-dom-webpack';
4
import App from './App';
5
6
-let data = ReactFlightDOMClient.readFromFetch(fetch('http://localhost:3001'));
6
+let data = ReactFlightDOMClient.createFromFetch(fetch('http://localhost:3001'));
7
+
8
ReactDOM.render(<App data={data} />, document.getElementById('root'));
packages/react-client/src/ReactFlightClient.js
+28
-41
@@ -27,10 +27,6 @@ import {
27
REACT_ELEMENT_TYPE,
28
} from 'shared/ReactSymbols';
29
30
-export type ReactModelRoot<T> = {|
31
- model: T,
32
-|};
33
-
30
export type JSONValue =
31
| number
32
| null
@@ -65,22 +61,32 @@ type ErroredChunk = {|
61
|};
62
type Chunk<T> = PendingChunk | ResolvedChunk<T> | ErroredChunk;
63
68
-export type Response = {
64
+export type Response<T> = {
65
partialRow: string,
70
- modelRoot: ReactModelRoot<any>,
66
+ rootChunk: Chunk<T>,
67
chunks: Map<number, Chunk<any>>,
68
+ readRoot(): T,
69
};
70
74
-export function createResponse(): Response {
75
- let modelRoot: ReactModelRoot<any> = ({}: any);
71
+function readRoot<T>(): T {
72
+ let response: Response<T> = this;
73
+ let rootChunk = response.rootChunk;
74
+ if (rootChunk.status === RESOLVED) {
75
+ return rootChunk.value;
76
+ } else {
77
+ throw rootChunk.value;
78
+ }
79
+}
80
+
81
+export function createResponse<T>(): Response<T> {
82
let rootChunk: Chunk<any> = createPendingChunk();
77
- definePendingProperty(modelRoot, 'model', rootChunk);
83
let chunks: Map<number, Chunk<any>> = new Map();
84
chunks.set(0, rootChunk);
85
let response = {
86
partialRow: '',
82
- modelRoot,
87
+ rootChunk,
88
chunks: chunks,
89
+ readRoot: readRoot,
90
};
91
return response;
92
}
@@ -142,7 +148,10 @@ function resolveChunk<T>(chunk: Chunk<T>, value: T): void {
148
149
// Report that any missing chunks in the model is now going to throw this
150
// error upon read. Also notify any pending promises.
145
-export function reportGlobalError(response: Response, error: Error): void {
151
+export function reportGlobalError<T>(
152
+ response: Response<T>,
153
+ error: Error,
154
+): void {
155
response.chunks.forEach(chunk => {
156
// If this chunk was already resolved or errored, it won't
157
// trigger an error but if it wasn't then we need to
@@ -164,24 +173,6 @@ function readMaybeChunk<T>(maybeChunk: Chunk<T> | T): T {
173
}
174
}
175
167
-function definePendingProperty<T>(
168
- object: Object,
169
- key: string,
170
- chunk: Chunk<T>,
171
-): void {
172
- Object.defineProperty(object, key, {
173
- configurable: false,
174
- enumerable: true,
175
- get() {
176
- if (chunk.status === RESOLVED) {
177
- return chunk.value;
178
- } else {
179
- throw chunk.value;
180
- }
181
- },
182
- });
183
-}
184
-
176
function createElement(type, key, props): React$Element<any> {
177
const element: any = {
178
// This tag allows us to uniquely identify this as a React Element
@@ -272,8 +263,8 @@ function createLazyBlock<Props, Data>(
263
return lazyType;
264
}
265
275
-export function parseModelFromJSON(
276
- response: Response,
266
+export function parseModelFromJSON<T>(
267
+ response: Response<T>,
268
targetObj: Object,
269
key: string,
270
value: JSONValue,
@@ -317,10 +308,10 @@ export function parseModelFromJSON(
308
return value;
309
}
310
320
-export function resolveModelChunk<T>(
321
- response: Response,
311
+export function resolveModelChunk<T, M>(
312
+ response: Response<T>,
313
id: number,
323
- model: T,
314
+ model: M,
315
): void {
316
let chunks = response.chunks;
317
let chunk = chunks.get(id);
@@ -331,8 +322,8 @@ export function resolveModelChunk<T>(
322
}
323
}
324
334
-export function resolveErrorChunk(
335
- response: Response,
325
+export function resolveErrorChunk<T>(
326
+ response: Response<T>,
327
id: number,
328
message: string,
329
stack: string,
@@ -348,14 +339,10 @@ export function resolveErrorChunk(
339
}
340
}
341
351
-export function close(response: Response): void {
342
+export function close<T>(response: Response<T>): void {
343
// In case there are any remaining unresolved chunks, they won't
344
// be resolved now. So we need to issue an error to those.
345
// Ideally we should be able to early bail out if we kept a
346
// ref count of pending chunks.
347
reportGlobalError(response, new Error('Connection closed.'));
348
}
358
-
359
-export function getModelRoot<T>(response: Response): ReactModelRoot<T> {
360
- return response.modelRoot;
361
-}
packages/react-client/src/ReactFlightClientStream.js
+9
-13
@@ -25,17 +25,13 @@ import {
25
readFinalStringChunk,
26
} from './ReactFlightClientHostConfig';
27
28
-export type ReactModelRoot<T> = {|
29
- model: T,
30
-|};
31
-
32
-type Response = ResponseBase & {
28
+export type Response<T> = ResponseBase<T> & {
29
fromJSON: (key: string, value: JSONValue) => any,
30
stringDecoder: StringDecoder,
31
};
32
37
-export function createResponse(): Response {
38
- let response: Response = (createResponseImpl(): any);
33
+export function createResponse<T>(): Response<T> {
34
+ let response: Response<T> = (createResponseImpl(): any);
35
response.fromJSON = function(key: string, value: JSONValue) {
36
return parseModelFromJSON(response, this, key, value);
37
};
@@ -45,7 +41,7 @@ export function createResponse(): Response {
41
return response;
42
}
43
48
-function processFullRow(response: Response, row: string): void {
44
+function processFullRow<T>(response: Response<T>, row: string): void {
45
if (row === '') {
46
return;
47
}
@@ -76,8 +72,8 @@ function processFullRow(response: Response, row: string): void {
72
}
73
}
74
79
-export function processStringChunk(
80
- response: Response,
75
+export function processStringChunk<T>(
76
+ response: Response<T>,
77
chunk: string,
78
offset: number,
79
): void {
@@ -92,8 +88,8 @@ export function processStringChunk(
88
response.partialRow += chunk.substring(offset);
89
}
90
95
-export function processBinaryChunk(
96
- response: Response,
91
+export function processBinaryChunk<T>(
92
+ response: Response<T>,
93
chunk: Uint8Array,
94
): void {
95
if (!supportsBinaryStreams) {
@@ -113,4 +109,4 @@ export function processBinaryChunk(
109
response.partialRow += readPartialStringChunk(stringDecoder, chunk);
110
}
111
116
-export {reportGlobalError, close, getModelRoot} from './ReactFlightClient';
112
+export {reportGlobalError, close} from './ReactFlightClient';
packages/react-client/src/__tests__/ReactFlight-test.js
+5
-6
@@ -57,8 +57,7 @@ describe('ReactFlight', () => {
57
let transport = ReactNoopFlightServer.render({
58
foo: <Foo />,
59
});
60
- let root = ReactNoopFlightClient.read(transport);
61
- let model = root.model;
60
+ let model = ReactNoopFlightClient.read(transport);
61
expect(model).toEqual({
62
foo: {
63
bar: (
@@ -87,10 +86,10 @@ describe('ReactFlight', () => {
86
};
87
88
let transport = ReactNoopFlightServer.render(model);
90
- let root = ReactNoopFlightClient.read(transport);
89
90
act(() => {
93
- let UserClient = root.model.User;
91
+ let rootModel = ReactNoopFlightClient.read(transport);
92
+ let UserClient = rootModel.User;
93
ReactNoop.render(<UserClient greeting="Hello" />);
94
});
95
@@ -114,10 +113,10 @@ describe('ReactFlight', () => {
113
};
114
115
let transport = ReactNoopFlightServer.render(model);
117
- let root = ReactNoopFlightClient.read(transport);
116
117
act(() => {
120
- let UserClient = root.model.User;
118
+ let rootModel = ReactNoopFlightClient.read(transport);
119
+ let UserClient = rootModel.User;
120
ReactNoop.render(<UserClient greeting="Hello" />);
121
});
122
packages/react-flight-dom-relay/src/ReactFlightDOMRelayClient.js
+9
-6
@@ -11,14 +11,13 @@ import type {Response, JSONValue} from 'react-client/src/ReactFlightClient';
11
12
import {
13
createResponse,
14
- getModelRoot,
14
parseModelFromJSON,
15
resolveModelChunk,
16
resolveErrorChunk,
17
close,
18
} from 'react-client/src/ReactFlightClient';
19
21
-function parseModel(response, targetObj, key, value) {
20
+function parseModel<T>(response: Response<T>, targetObj, key, value) {
21
if (typeof value === 'object' && value !== null) {
22
if (Array.isArray(value)) {
23
for (let i = 0; i < value.length; i++) {
@@ -38,14 +37,18 @@ function parseModel(response, targetObj, key, value) {
37
return parseModelFromJSON(response, targetObj, key, value);
38
}
39
41
-export {createResponse, getModelRoot, close};
40
+export {createResponse, close};
41
43
-export function resolveModel(response: Response, id: number, json: JSONValue) {
42
+export function resolveModel<T>(
43
+ response: Response<T>,
44
+ id: number,
45
+ json: JSONValue,
46
+) {
47
resolveModelChunk(response, id, parseModel(response, {}, '', json));
48
}
49
47
-export function resolveError(
48
- response: Response,
50
+export function resolveError<T>(
51
+ response: Response<T>,
52
id: number,
53
message: string,
54
stack: string,
packages/react-flight-dom-relay/src/__tests__/ReactFlightDOMRelay-test.internal.js
+1
-1
@@ -39,8 +39,8 @@ describe('ReactFlightDOMRelay', () => {
39
);
40
}
41
}
42
- let model = ReactDOMFlightRelayClient.getModelRoot(response).model;
42
ReactDOMFlightRelayClient.close(response);
43
+ let model = response.readRoot();
44
return model;
45
}
46
packages/react-flight-dom-webpack/src/ReactFlightDOMClient.js
+18
-14
@@ -7,18 +7,20 @@
7
* @flow
8
*/
9
10
-import type {ReactModelRoot} from 'react-client/src/ReactFlightClientStream';
10
+import type {Response as FlightResponse} from 'react-client/src/ReactFlightClientStream';
11
12
import {
13
createResponse,
14
- getModelRoot,
14
reportGlobalError,
15
processStringChunk,
16
processBinaryChunk,
17
close,
18
} from 'react-client/src/ReactFlightClientStream';
19
21
-function startReadingFromStream(response, stream: ReadableStream): void {
20
+function startReadingFromStream<T>(
21
+ response: FlightResponse<T>,
22
+ stream: ReadableStream,
23
+): void {
24
let reader = stream.getReader();
25
function progress({done, value}) {
26
if (done) {
@@ -35,16 +37,18 @@ function startReadingFromStream(response, stream: ReadableStream): void {
37
reader.read().then(progress, error);
38
}
39
38
-function readFromReadableStream<T>(stream: ReadableStream): ReactModelRoot<T> {
39
- let response = createResponse();
40
+function createFromReadableStream<T>(
41
+ stream: ReadableStream,
42
+): FlightResponse<T> {
43
+ let response: FlightResponse<T> = createResponse();
44
startReadingFromStream(response, stream);
41
- return getModelRoot(response);
45
+ return response;
46
}
47
44
-function readFromFetch<T>(
48
+function createFromFetch<T>(
49
promiseForResponse: Promise<Response>,
46
-): ReactModelRoot<T> {
47
- let response = createResponse();
50
+): FlightResponse<T> {
51
+ let response: FlightResponse<T> = createResponse();
52
promiseForResponse.then(
53
function(r) {
54
startReadingFromStream(response, (r.body: any));
@@ -53,11 +57,11 @@ function readFromFetch<T>(
57
reportGlobalError(response, e);
58
},
59
);
56
- return getModelRoot(response);
60
+ return response;
61
}
62
59
-function readFromXHR<T>(request: XMLHttpRequest): ReactModelRoot<T> {
60
- let response = createResponse();
63
+function createFromXHR<T>(request: XMLHttpRequest): FlightResponse<T> {
64
+ let response: FlightResponse<T> = createResponse();
65
let processedLength = 0;
66
function progress(e: ProgressEvent): void {
67
let chunk = request.responseText;
@@ -76,7 +80,7 @@ function readFromXHR<T>(request: XMLHttpRequest): ReactModelRoot<T> {
80
request.addEventListener('error', error);
81
request.addEventListener('abort', error);
82
request.addEventListener('timeout', error);
79
- return getModelRoot(response);
83
+ return response;
84
}
85
82
-export {readFromXHR, readFromFetch, readFromReadableStream};
86
+export {createFromXHR, createFromFetch, createFromReadableStream};
packages/react-flight-dom-webpack/src/__tests__/ReactFlightDOM-test.js
+40
-37
@@ -119,9 +119,10 @@ describe('ReactFlightDOM', () => {
119
120
let {writable, readable} = getTestStream();
121
ReactFlightDOMServer.pipeToNodeWritable(<App />, writable, webpackMap);
122
- let result = ReactFlightDOMClient.readFromReadableStream(readable);
122
+ let response = ReactFlightDOMClient.createFromReadableStream(readable);
123
await waitForSuspense(() => {
124
- expect(result.model).toEqual({
124
+ let model = response.readRoot();
125
+ expect(model).toEqual({
126
html: (
127
<div>
128
<span>hello</span>
@@ -154,13 +155,13 @@ describe('ReactFlightDOM', () => {
155
}
156
157
// View
157
- function Message({result}) {
158
- return <section>{result.model.html}</section>;
158
+ function Message({response}) {
159
+ return <section>{response.readRoot().html}</section>;
160
}
160
- function App({result}) {
161
+ function App({response}) {
162
return (
163
<Suspense fallback={<h1>Loading...</h1>}>
163
- <Message result={result} />
164
+ <Message response={response} />
165
</Suspense>
166
);
167
}
@@ -171,12 +172,12 @@ describe('ReactFlightDOM', () => {
172
writable,
173
webpackMap,
174
);
174
- let result = ReactFlightDOMClient.readFromReadableStream(readable);
175
+ let response = ReactFlightDOMClient.createFromReadableStream(readable);
176
177
let container = document.createElement('div');
178
let root = ReactDOM.createRoot(container);
179
await act(async () => {
179
- root.render(<App result={result} />);
180
+ root.render(<App response={response} />);
181
});
182
expect(container.innerHTML).toBe(
183
'<section><div><span>hello</span><span>world</span></div></section>',
@@ -192,13 +193,13 @@ describe('ReactFlightDOM', () => {
193
}
194
195
// View
195
- function Message({result}) {
196
- return <p>{result.model.text}</p>;
196
+ function Message({response}) {
197
+ return <p>{response.readRoot().text}</p>;
198
}
198
- function App({result}) {
199
+ function App({response}) {
200
return (
201
<Suspense fallback={<h1>Loading...</h1>}>
201
- <Message result={result} />
202
+ <Message response={response} />
203
</Suspense>
204
);
205
}
@@ -209,12 +210,12 @@ describe('ReactFlightDOM', () => {
210
writable,
211
webpackMap,
212
);
212
- let result = ReactFlightDOMClient.readFromReadableStream(readable);
213
+ let response = ReactFlightDOMClient.createFromReadableStream(readable);
214
215
let container = document.createElement('div');
216
let root = ReactDOM.createRoot(container);
217
await act(async () => {
217
- root.render(<App result={result} />);
218
+ root.render(<App response={response} />);
219
});
220
expect(container.innerHTML).toBe('<p>$1</p>');
221
});
@@ -228,13 +229,13 @@ describe('ReactFlightDOM', () => {
229
}
230
231
// View
231
- function Message({result}) {
232
- return <p>{result.model.text}</p>;
232
+ function Message({response}) {
233
+ return <p>{response.readRoot().text}</p>;
234
}
234
- function App({result}) {
235
+ function App({response}) {
236
return (
237
<Suspense fallback={<h1>Loading...</h1>}>
237
- <Message result={result} />
238
+ <Message response={response} />
239
</Suspense>
240
);
241
}
@@ -245,12 +246,12 @@ describe('ReactFlightDOM', () => {
246
writable,
247
webpackMap,
248
);
248
- let result = ReactFlightDOMClient.readFromReadableStream(readable);
249
+ let response = ReactFlightDOMClient.createFromReadableStream(readable);
250
251
let container = document.createElement('div');
252
let root = ReactDOM.createRoot(container);
253
await act(async () => {
253
- root.render(<App result={result} />);
254
+ root.render(<App response={response} />);
255
});
256
expect(container.innerHTML).toBe('<p>@div</p>');
257
});
@@ -327,42 +328,44 @@ describe('ReactFlightDOM', () => {
328
};
329
330
// View
330
- function ProfileDetails({result}) {
331
+ function ProfileDetails({response}) {
332
+ let model = response.readRoot();
333
return (
334
<div>
333
- {result.model.name}
334
- {result.model.more.avatar}
335
+ {model.name}
336
+ {model.more.avatar}
337
</div>
338
);
339
}
338
- function ProfileSidebar({result}) {
340
+ function ProfileSidebar({response}) {
341
+ let model = response.readRoot();
342
return (
343
<div>
341
- {result.model.photos}
342
- {result.model.more.friends}
344
+ {model.photos}
345
+ {model.more.friends}
346
</div>
347
);
348
}
346
- function ProfilePosts({result}) {
347
- return <div>{result.model.more.posts}</div>;
349
+ function ProfilePosts({response}) {
350
+ return <div>{response.readRoot().more.posts}</div>;
351
}
349
- function ProfileGames({result}) {
350
- return <div>{result.model.more.games}</div>;
352
+ function ProfileGames({response}) {
353
+ return <div>{response.readRoot().more.games}</div>;
354
}
352
- function ProfilePage({result}) {
355
+ function ProfilePage({response}) {
356
return (
357
<>
358
<Suspense fallback={<p>(loading)</p>}>
356
- <ProfileDetails result={result} />
359
+ <ProfileDetails response={response} />
360
<Suspense fallback={<p>(loading sidebar)</p>}>
358
- <ProfileSidebar result={result} />
361
+ <ProfileSidebar response={response} />
362
</Suspense>
363
<Suspense fallback={<p>(loading posts)</p>}>
361
- <ProfilePosts result={result} />
364
+ <ProfilePosts response={response} />
365
</Suspense>
366
<ErrorBoundary fallback={e => <p>{e.message}</p>}>
367
<Suspense fallback={<p>(loading games)</p>}>
365
- <ProfileGames result={result} />
368
+ <ProfileGames response={response} />
369
</Suspense>
370
</ErrorBoundary>
371
</Suspense>
@@ -372,12 +375,12 @@ describe('ReactFlightDOM', () => {
375
376
let {writable, readable} = getTestStream();
377
ReactFlightDOMServer.pipeToNodeWritable(profileModel, writable, webpackMap);
375
- let result = ReactFlightDOMClient.readFromReadableStream(readable);
378
+ let response = ReactFlightDOMClient.createFromReadableStream(readable);
379
380
let container = document.createElement('div');
381
let root = ReactDOM.createRoot(container);
382
await act(async () => {
380
- root.render(<ProfilePage result={result} />);
383
+ root.render(<ProfilePage response={response} />);
384
});
385
expect(container.innerHTML).toBe('<p>(loading)</p>');
386
packages/react-flight-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js
+3
-2
@@ -62,9 +62,10 @@ describe('ReactFlightDOMBrowser', () => {
62
}
63
64
let stream = ReactFlightDOMServer.renderToReadableStream(<App />);
65
- let result = ReactFlightDOMClient.readFromReadableStream(stream);
65
+ let response = ReactFlightDOMClient.createFromReadableStream(stream);
66
await waitForSuspense(() => {
67
- expect(result.model).toEqual({
67
+ let model = response.readRoot();
68
+ expect(model).toEqual({
69
html: (
70
<div>
71
<span>hello</span>
packages/react-noop-renderer/src/ReactNoopFlightClient.js
+3
-10
@@ -14,20 +14,13 @@
14
* environment.
15
*/
16
17
-import type {ReactModelRoot} from 'react-client/flight';
18
-
17
import {readModule} from 'react-noop-renderer/flight-modules';
18
19
import ReactFlightClient from 'react-client/flight';
20
21
type Source = Array<string>;
22
25
-const {
26
- createResponse,
27
- getModelRoot,
28
- processStringChunk,
29
- close,
30
-} = ReactFlightClient({
23
+const {createResponse, processStringChunk, close} = ReactFlightClient({
24
supportsBinaryStreams: false,
25
resolveModuleReference(idx: string) {
26
return idx;
@@ -38,13 +31,13 @@ const {
31
},
32
});
33
41
-function read<T>(source: Source): ReactModelRoot<T> {
34
+function read<T>(source: Source): T {
35
let response = createResponse(source);
36
for (let i = 0; i < source.length; i++) {
37
processStringChunk(response, source[i], 0);
38
}
39
close(response);
47
- return getModelRoot(response);
40
+ return response.readRoot();
41
}
42
43
export {read};