Skip to content

Commit a6924d7

Browse files
sebmarkbagesebmarkbage
authored
Change .model getter to .readRoot method (facebook#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.
1 parent bd57819 commit a6924d7

File tree

12 files changed

+122
-135
lines changed

12 files changed

+122
-135
lines changed

‎fixtures/flight-browser/index.html

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,20 +70,20 @@ <h1>Flight Example</h1>
7070
let blob = await responseToDisplay.blob();
7171
let url = URL.createObjectURL(blob);
7272

73-
let data = ReactFlightDOMClient.readFromFetch(
73+
let data = ReactFlightDOMClient.createFromFetch(
7474
fetch(url)
7575
);
7676
// The client also supports XHR streaming.
7777
// var xhr = new XMLHttpRequest();
7878
// xhr.open('GET', url);
79-
// let data = ReactFlightDOMClient.readFromXHR(xhr);
79+
// let data = ReactFlightDOMClient.createFromXHR(xhr);
8080
// xhr.send();
8181

8282
renderResult(data);
8383
}
8484

8585
function Shell({ data }) {
86-
let model = data.model;
86+
let model = data.readRoot();
8787
return <div>
8888
<Suspense fallback="...">
8989
<h1>{model.title}</h1>

‎fixtures/flight/src/App.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React, {Suspense} from 'react';
22

33
function Content({data}) {
4-
return data.model.content;
4+
return data.readRoot().content;
55
}
66

77
function App({data}) {

‎fixtures/flight/src/index.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,6 @@ import ReactDOM from 'react-dom';
33
import ReactFlightDOMClient from 'react-flight-dom-webpack';
44
import App from './App';
55

6-
let data = ReactFlightDOMClient.readFromFetch(fetch('http://localhost:3001'));
6+
let data = ReactFlightDOMClient.createFromFetch(fetch('http://localhost:3001'));
7+
78
ReactDOM.render(<App data={data} />, document.getElementById('root'));

‎packages/react-client/src/ReactFlightClient.js

Lines changed: 28 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,6 @@ import {
2727
REACT_ELEMENT_TYPE,
2828
} from 'shared/ReactSymbols';
2929

30-
export type ReactModelRoot<T> = {|
31-
model: T,
32-
|};
33-
3430
export type JSONValue =
3531
| number
3632
| null
@@ -65,22 +61,32 @@ type ErroredChunk = {|
6561
|};
6662
type Chunk<T> = PendingChunk | ResolvedChunk<T> | ErroredChunk;
6763

68-
export type Response = {
64+
export type Response<T> = {
6965
partialRow: string,
70-
modelRoot: ReactModelRoot<any>,
66+
rootChunk: Chunk<T>,
7167
chunks: Map<number, Chunk<any>>,
68+
readRoot(): T,
7269
};
7370

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> {
7682
let rootChunk: Chunk<any> = createPendingChunk();
77-
definePendingProperty(modelRoot, 'model', rootChunk);
7883
let chunks: Map<number, Chunk<any>> = new Map();
7984
chunks.set(0, rootChunk);
8085
let response = {
8186
partialRow: '',
82-
modelRoot,
87+
rootChunk,
8388
chunks: chunks,
89+
readRoot: readRoot,
8490
};
8591
return response;
8692
}
@@ -142,7 +148,10 @@ function resolveChunk<T>(chunk: Chunk<T>, value: T): void {
142148

143149
// Report that any missing chunks in the model is now going to throw this
144150
// 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 {
146155
response.chunks.forEach(chunk => {
147156
// If this chunk was already resolved or errored, it won't
148157
// trigger an error but if it wasn't then we need to
@@ -164,24 +173,6 @@ function readMaybeChunk<T>(maybeChunk: Chunk<T> | T): T {
164173
}
165174
}
166175

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-
185176
function createElement(type, key, props): React$Element<any> {
186177
const element: any = {
187178
// This tag allows us to uniquely identify this as a React Element
@@ -272,8 +263,8 @@ function createLazyBlock<Props, Data>(
272263
return lazyType;
273264
}
274265

275-
export function parseModelFromJSON(
276-
response: Response,
266+
export function parseModelFromJSON<T>(
267+
response: Response<T>,
277268
targetObj: Object,
278269
key: string,
279270
value: JSONValue,
@@ -317,10 +308,10 @@ export function parseModelFromJSON(
317308
return value;
318309
}
319310

320-
export function resolveModelChunk<T>(
321-
response: Response,
311+
export function resolveModelChunk<T, M>(
312+
response: Response<T>,
322313
id: number,
323-
model: T,
314+
model: M,
324315
): void {
325316
let chunks = response.chunks;
326317
let chunk = chunks.get(id);
@@ -331,8 +322,8 @@ export function resolveModelChunk<T>(
331322
}
332323
}
333324

334-
export function resolveErrorChunk(
335-
response: Response,
325+
export function resolveErrorChunk<T>(
326+
response: Response<T>,
336327
id: number,
337328
message: string,
338329
stack: string,
@@ -348,14 +339,10 @@ export function resolveErrorChunk(
348339
}
349340
}
350341

351-
export function close(response: Response): void {
342+
export function close<T>(response: Response<T>): void {
352343
// In case there are any remaining unresolved chunks, they won't
353344
// be resolved now. So we need to issue an error to those.
354345
// Ideally we should be able to early bail out if we kept a
355346
// ref count of pending chunks.
356347
reportGlobalError(response, new Error('Connection closed.'));
357348
}
358-
359-
export function getModelRoot<T>(response: Response): ReactModelRoot<T> {
360-
return response.modelRoot;
361-
}

‎packages/react-client/src/ReactFlightClientStream.js

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,13 @@ import {
2525
readFinalStringChunk,
2626
} from './ReactFlightClientHostConfig';
2727

28-
export type ReactModelRoot<T> = {|
29-
model: T,
30-
|};
31-
32-
type Response = ResponseBase & {
28+
export type Response<T> = ResponseBase<T> & {
3329
fromJSON: (key: string, value: JSONValue) => any,
3430
stringDecoder: StringDecoder,
3531
};
3632

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);
3935
response.fromJSON = function(key: string, value: JSONValue) {
4036
return parseModelFromJSON(response, this, key, value);
4137
};
@@ -45,7 +41,7 @@ export function createResponse(): Response {
4541
return response;
4642
}
4743

48-
function processFullRow(response: Response, row: string): void {
44+
function processFullRow<T>(response: Response<T>, row: string): void {
4945
if (row === '') {
5046
return;
5147
}
@@ -76,8 +72,8 @@ function processFullRow(response: Response, row: string): void {
7672
}
7773
}
7874

79-
export function processStringChunk(
80-
response: Response,
75+
export function processStringChunk<T>(
76+
response: Response<T>,
8177
chunk: string,
8278
offset: number,
8379
): void {
@@ -92,8 +88,8 @@ export function processStringChunk(
9288
response.partialRow += chunk.substring(offset);
9389
}
9490

95-
export function processBinaryChunk(
96-
response: Response,
91+
export function processBinaryChunk<T>(
92+
response: Response<T>,
9793
chunk: Uint8Array,
9894
): void {
9995
if (!supportsBinaryStreams) {
@@ -113,4 +109,4 @@ export function processBinaryChunk(
113109
response.partialRow += readPartialStringChunk(stringDecoder, chunk);
114110
}
115111

116-
export {reportGlobalError, close, getModelRoot} from './ReactFlightClient';
112+
export {reportGlobalError, close} from './ReactFlightClient';

‎packages/react-client/src/__tests__/ReactFlight-test.js

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ describe('ReactFlight', () => {
5757
let transport = ReactNoopFlightServer.render({
5858
foo: <Foo />,
5959
});
60-
let root = ReactNoopFlightClient.read(transport);
61-
let model = root.model;
60+
let model = ReactNoopFlightClient.read(transport);
6261
expect(model).toEqual({
6362
foo: {
6463
bar: (
@@ -87,10 +86,10 @@ describe('ReactFlight', () => {
8786
};
8887

8988
let transport = ReactNoopFlightServer.render(model);
90-
let root = ReactNoopFlightClient.read(transport);
9189

9290
act(() => {
93-
let UserClient = root.model.User;
91+
let rootModel = ReactNoopFlightClient.read(transport);
92+
let UserClient = rootModel.User;
9493
ReactNoop.render(<UserClient greeting="Hello" />);
9594
});
9695

@@ -114,10 +113,10 @@ describe('ReactFlight', () => {
114113
};
115114

116115
let transport = ReactNoopFlightServer.render(model);
117-
let root = ReactNoopFlightClient.read(transport);
118116

119117
act(() => {
120-
let UserClient = root.model.User;
118+
let rootModel = ReactNoopFlightClient.read(transport);
119+
let UserClient = rootModel.User;
121120
ReactNoop.render(<UserClient greeting="Hello" />);
122121
});
123122

‎packages/react-flight-dom-relay/src/ReactFlightDOMRelayClient.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,13 @@ import type {Response, JSONValue} from 'react-client/src/ReactFlightClient';
1111

1212
import {
1313
createResponse,
14-
getModelRoot,
1514
parseModelFromJSON,
1615
resolveModelChunk,
1716
resolveErrorChunk,
1817
close,
1918
} from 'react-client/src/ReactFlightClient';
2019

21-
function parseModel(response, targetObj, key, value) {
20+
function parseModel<T>(response: Response<T>, targetObj, key, value) {
2221
if (typeof value === 'object' && value !== null) {
2322
if (Array.isArray(value)) {
2423
for (let i = 0; i < value.length; i++) {
@@ -38,14 +37,18 @@ function parseModel(response, targetObj, key, value) {
3837
return parseModelFromJSON(response, targetObj, key, value);
3938
}
4039

41-
export {createResponse, getModelRoot, close};
40+
export {createResponse, close};
4241

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+
) {
4447
resolveModelChunk(response, id, parseModel(response, {}, '', json));
4548
}
4649

47-
export function resolveError(
48-
response: Response,
50+
export function resolveError<T>(
51+
response: Response<T>,
4952
id: number,
5053
message: string,
5154
stack: string,

‎packages/react-flight-dom-relay/src/__tests__/ReactFlightDOMRelay-test.internal.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ describe('ReactFlightDOMRelay', () => {
3939
);
4040
}
4141
}
42-
let model = ReactDOMFlightRelayClient.getModelRoot(response).model;
4342
ReactDOMFlightRelayClient.close(response);
43+
let model = response.readRoot();
4444
return model;
4545
}
4646

‎packages/react-flight-dom-webpack/src/ReactFlightDOMClient.js

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,20 @@
77
* @flow
88
*/
99

10-
import type {ReactModelRoot} from 'react-client/src/ReactFlightClientStream';
10+
import type {Response as FlightResponse} from 'react-client/src/ReactFlightClientStream';
1111

1212
import {
1313
createResponse,
14-
getModelRoot,
1514
reportGlobalError,
1615
processStringChunk,
1716
processBinaryChunk,
1817
close,
1918
} from 'react-client/src/ReactFlightClientStream';
2019

21-
function startReadingFromStream(response, stream: ReadableStream): void {
20+
function startReadingFromStream<T>(
21+
response: FlightResponse<T>,
22+
stream: ReadableStream,
23+
): void {
2224
let reader = stream.getReader();
2325
function progress({done, value}) {
2426
if (done) {
@@ -35,16 +37,18 @@ function startReadingFromStream(response, stream: ReadableStream): void {
3537
reader.read().then(progress, error);
3638
}
3739

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();
4044
startReadingFromStream(response, stream);
41-
return getModelRoot(response);
45+
return response;
4246
}
4347

44-
function readFromFetch<T>(
48+
function createFromFetch<T>(
4549
promiseForResponse: Promise<Response>,
46-
): ReactModelRoot<T> {
47-
let response = createResponse();
50+
): FlightResponse<T> {
51+
let response: FlightResponse<T> = createResponse();
4852
promiseForResponse.then(
4953
function(r) {
5054
startReadingFromStream(response, (r.body: any));
@@ -53,11 +57,11 @@ function readFromFetch<T>(
5357
reportGlobalError(response, e);
5458
},
5559
);
56-
return getModelRoot(response);
60+
return response;
5761
}
5862

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();
6165
let processedLength = 0;
6266
function progress(e: ProgressEvent): void {
6367
let chunk = request.responseText;
@@ -76,7 +80,7 @@ function readFromXHR<T>(request: XMLHttpRequest): ReactModelRoot<T> {
7680
request.addEventListener('error', error);
7781
request.addEventListener('abort', error);
7882
request.addEventListener('timeout', error);
79-
return getModelRoot(response);
83+
return response;
8084
}
8185

82-
export {readFromXHR, readFromFetch, readFromReadableStream};
86+
export {createFromXHR, createFromFetch, createFromReadableStream};

0 commit comments

Comments
 (0)