Skip to content

Commit eba248c

Browse files
authored
[Fizz/Flight] Remove reentrancy hack (#22446)
* Remove reentrant check from Fizz/Flight * Make startFlowing explicit in Flight This is already an explicit call in Fizz. This moves flowing to be explicit. That way we can avoid calling it in start() for web streams and therefore avoid the reentrant call. * Add regression test This test doesn't actually error due to the streams polyfill not behaving like Chrome but rather according to spec. * Update the Web Streams polyfill Not that we need this but just in case there are differences that are fixed.
1 parent 6638815 commit eba248c

File tree

12 files changed

+300
-29
lines changed

12 files changed

+300
-29
lines changed

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
"@babel/preset-flow": "^7.10.4",
3636
"@babel/preset-react": "^7.10.4",
3737
"@babel/traverse": "^7.11.0",
38-
"@mattiasbuelens/web-streams-polyfill": "^0.3.2",
38+
"web-streams-polyfill": "^3.1.1",
3939
"abort-controller": "^3.0.0",
4040
"art": "0.10.1",
4141
"babel-eslint": "^10.0.3",

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
'use strict';
1111

1212
// Polyfills for test environment
13-
global.ReadableStream = require('@mattiasbuelens/web-streams-polyfill/ponyfill/es6').ReadableStream;
13+
global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream;
1414
global.TextEncoder = require('util').TextEncoder;
1515
global.AbortController = require('abort-controller');
1616

packages/react-noop-renderer/src/ReactNoopFlightServer.js

+1
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ function render(model: ReactModel, options?: Options): Destination {
6868
options ? options.onError : undefined,
6969
);
7070
ReactNoopFlightServer.startWork(request);
71+
ReactNoopFlightServer.startFlowing(request);
7172
return destination;
7273
}
7374

packages/react-server-dom-relay/src/ReactFlightDOMRelayServer.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ import type {
1313
Destination,
1414
} from './ReactFlightDOMRelayServerHostConfig';
1515

16-
import {createRequest, startWork} from 'react-server/src/ReactFlightServer';
16+
import {
17+
createRequest,
18+
startWork,
19+
startFlowing,
20+
} from 'react-server/src/ReactFlightServer';
1721

1822
type Options = {
1923
onError?: (error: mixed) => void,
@@ -32,6 +36,7 @@ function render(
3236
options ? options.onError : undefined,
3337
);
3438
startWork(request);
39+
startFlowing(request);
3540
}
3641

3742
export {render};

packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js

+9-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ function renderToReadableStream(
2626
options?: Options,
2727
): ReadableStream {
2828
let request;
29-
return new ReadableStream({
29+
const stream = new ReadableStream({
3030
start(controller) {
3131
request = createRequest(
3232
model,
@@ -37,10 +37,17 @@ function renderToReadableStream(
3737
startWork(request);
3838
},
3939
pull(controller) {
40-
startFlowing(request);
40+
// Pull is called immediately even if the stream is not passed to anything.
41+
// That's buffering too early. We want to start buffering once the stream
42+
// is actually used by something so we can give it the best result possible
43+
// at that point.
44+
if (stream.locked) {
45+
startFlowing(request);
46+
}
4147
},
4248
cancel(reason) {},
4349
});
50+
return stream;
4451
}
4552

4653
export {renderToReadableStream};

packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,9 @@ function pipeToNodeWritable(
3737
webpackMap,
3838
options ? options.onError : undefined,
3939
);
40-
destination.on('drain', createDrainHandler(destination, request));
4140
startWork(request);
41+
startFlowing(request);
42+
destination.on('drain', createDrainHandler(destination, request));
4243
}
4344

4445
export {pipeToNodeWritable};

packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
'use strict';
1111

1212
// Polyfills for test environment
13-
global.ReadableStream = require('@mattiasbuelens/web-streams-polyfill/ponyfill/es6').ReadableStream;
13+
global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream;
1414
global.TextDecoder = require('util').TextDecoder;
1515

1616
// Don't wait before processing work on the server.

packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js

+267-2
Original file line numberDiff line numberDiff line change
@@ -5,28 +5,56 @@
55
* LICENSE file in the root directory of this source tree.
66
*
77
* @emails react-core
8-
* @jest-environment node
98
*/
109

1110
'use strict';
1211

1312
// Polyfills for test environment
14-
global.ReadableStream = require('@mattiasbuelens/web-streams-polyfill/ponyfill/es6').ReadableStream;
13+
global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream;
1514
global.TextEncoder = require('util').TextEncoder;
1615
global.TextDecoder = require('util').TextDecoder;
1716

17+
let webpackModuleIdx = 0;
18+
let webpackModules = {};
19+
let webpackMap = {};
20+
global.__webpack_require__ = function(id) {
21+
return webpackModules[id];
22+
};
23+
24+
let act;
1825
let React;
26+
let ReactDOM;
1927
let ReactServerDOMWriter;
2028
let ReactServerDOMReader;
2129

2230
describe('ReactFlightDOMBrowser', () => {
2331
beforeEach(() => {
2432
jest.resetModules();
33+
webpackModules = {};
34+
webpackMap = {};
35+
act = require('jest-react').act;
2536
React = require('react');
37+
ReactDOM = require('react-dom');
2638
ReactServerDOMWriter = require('react-server-dom-webpack/writer.browser.server');
2739
ReactServerDOMReader = require('react-server-dom-webpack');
2840
});
2941

42+
function moduleReference(moduleExport) {
43+
const idx = webpackModuleIdx++;
44+
webpackModules[idx] = {
45+
d: moduleExport,
46+
};
47+
webpackMap['path/' + idx] = {
48+
default: {
49+
id: '' + idx,
50+
chunks: [],
51+
name: 'd',
52+
},
53+
};
54+
const MODULE_TAG = Symbol.for('react.module.reference');
55+
return {$$typeof: MODULE_TAG, filepath: 'path/' + idx, name: 'default'};
56+
}
57+
3058
async function waitForSuspense(fn) {
3159
while (true) {
3260
try {
@@ -75,4 +103,241 @@ describe('ReactFlightDOMBrowser', () => {
75103
});
76104
});
77105
});
106+
107+
it('should resolve HTML using W3C streams', async () => {
108+
function Text({children}) {
109+
return <span>{children}</span>;
110+
}
111+
function HTML() {
112+
return (
113+
<div>
114+
<Text>hello</Text>
115+
<Text>world</Text>
116+
</div>
117+
);
118+
}
119+
120+
function App() {
121+
const model = {
122+
html: <HTML />,
123+
};
124+
return model;
125+
}
126+
127+
const stream = ReactServerDOMWriter.renderToReadableStream(<App />);
128+
const response = ReactServerDOMReader.createFromReadableStream(stream);
129+
await waitForSuspense(() => {
130+
const model = response.readRoot();
131+
expect(model).toEqual({
132+
html: (
133+
<div>
134+
<span>hello</span>
135+
<span>world</span>
136+
</div>
137+
),
138+
});
139+
});
140+
});
141+
142+
it('should progressively reveal server components', async () => {
143+
let reportedErrors = [];
144+
const {Suspense} = React;
145+
146+
// Client Components
147+
148+
class ErrorBoundary extends React.Component {
149+
state = {hasError: false, error: null};
150+
static getDerivedStateFromError(error) {
151+
return {
152+
hasError: true,
153+
error,
154+
};
155+
}
156+
render() {
157+
if (this.state.hasError) {
158+
return this.props.fallback(this.state.error);
159+
}
160+
return this.props.children;
161+
}
162+
}
163+
164+
function MyErrorBoundary({children}) {
165+
return (
166+
<ErrorBoundary fallback={e => <p>{e.message}</p>}>
167+
{children}
168+
</ErrorBoundary>
169+
);
170+
}
171+
172+
// Model
173+
function Text({children}) {
174+
return children;
175+
}
176+
177+
function makeDelayedText() {
178+
let error, _resolve, _reject;
179+
let promise = new Promise((resolve, reject) => {
180+
_resolve = () => {
181+
promise = null;
182+
resolve();
183+
};
184+
_reject = e => {
185+
error = e;
186+
promise = null;
187+
reject(e);
188+
};
189+
});
190+
function DelayedText({children}, data) {
191+
if (promise) {
192+
throw promise;
193+
}
194+
if (error) {
195+
throw error;
196+
}
197+
return <Text>{children}</Text>;
198+
}
199+
return [DelayedText, _resolve, _reject];
200+
}
201+
202+
const [Friends, resolveFriends] = makeDelayedText();
203+
const [Name, resolveName] = makeDelayedText();
204+
const [Posts, resolvePosts] = makeDelayedText();
205+
const [Photos, resolvePhotos] = makeDelayedText();
206+
const [Games, , rejectGames] = makeDelayedText();
207+
208+
// View
209+
function ProfileDetails({avatar}) {
210+
return (
211+
<div>
212+
<Name>:name:</Name>
213+
{avatar}
214+
</div>
215+
);
216+
}
217+
function ProfileSidebar({friends}) {
218+
return (
219+
<div>
220+
<Photos>:photos:</Photos>
221+
{friends}
222+
</div>
223+
);
224+
}
225+
function ProfilePosts({posts}) {
226+
return <div>{posts}</div>;
227+
}
228+
function ProfileGames({games}) {
229+
return <div>{games}</div>;
230+
}
231+
232+
const MyErrorBoundaryClient = moduleReference(MyErrorBoundary);
233+
234+
function ProfileContent() {
235+
return (
236+
<>
237+
<ProfileDetails avatar={<Text>:avatar:</Text>} />
238+
<Suspense fallback={<p>(loading sidebar)</p>}>
239+
<ProfileSidebar friends={<Friends>:friends:</Friends>} />
240+
</Suspense>
241+
<Suspense fallback={<p>(loading posts)</p>}>
242+
<ProfilePosts posts={<Posts>:posts:</Posts>} />
243+
</Suspense>
244+
<MyErrorBoundaryClient>
245+
<Suspense fallback={<p>(loading games)</p>}>
246+
<ProfileGames games={<Games>:games:</Games>} />
247+
</Suspense>
248+
</MyErrorBoundaryClient>
249+
</>
250+
);
251+
}
252+
253+
const model = {
254+
rootContent: <ProfileContent />,
255+
};
256+
257+
function ProfilePage({response}) {
258+
return response.readRoot().rootContent;
259+
}
260+
261+
const stream = ReactServerDOMWriter.renderToReadableStream(
262+
model,
263+
webpackMap,
264+
{
265+
onError(x) {
266+
reportedErrors.push(x);
267+
},
268+
},
269+
);
270+
const response = ReactServerDOMReader.createFromReadableStream(stream);
271+
272+
const container = document.createElement('div');
273+
const root = ReactDOM.createRoot(container);
274+
await act(async () => {
275+
root.render(
276+
<Suspense fallback={<p>(loading)</p>}>
277+
<ProfilePage response={response} />
278+
</Suspense>,
279+
);
280+
});
281+
expect(container.innerHTML).toBe('<p>(loading)</p>');
282+
283+
// This isn't enough to show anything.
284+
await act(async () => {
285+
resolveFriends();
286+
});
287+
expect(container.innerHTML).toBe('<p>(loading)</p>');
288+
289+
// We can now show the details. Sidebar and posts are still loading.
290+
await act(async () => {
291+
resolveName();
292+
});
293+
// Advance time enough to trigger a nested fallback.
294+
jest.advanceTimersByTime(500);
295+
expect(container.innerHTML).toBe(
296+
'<div>:name::avatar:</div>' +
297+
'<p>(loading sidebar)</p>' +
298+
'<p>(loading posts)</p>' +
299+
'<p>(loading games)</p>',
300+
);
301+
302+
expect(reportedErrors).toEqual([]);
303+
304+
const theError = new Error('Game over');
305+
// Let's *fail* loading games.
306+
await act(async () => {
307+
rejectGames(theError);
308+
});
309+
expect(container.innerHTML).toBe(
310+
'<div>:name::avatar:</div>' +
311+
'<p>(loading sidebar)</p>' +
312+
'<p>(loading posts)</p>' +
313+
'<p>Game over</p>', // TODO: should not have message in prod.
314+
);
315+
316+
expect(reportedErrors).toEqual([theError]);
317+
reportedErrors = [];
318+
319+
// We can now show the sidebar.
320+
await act(async () => {
321+
resolvePhotos();
322+
});
323+
expect(container.innerHTML).toBe(
324+
'<div>:name::avatar:</div>' +
325+
'<div>:photos::friends:</div>' +
326+
'<p>(loading posts)</p>' +
327+
'<p>Game over</p>', // TODO: should not have message in prod.
328+
);
329+
330+
// Show everything.
331+
await act(async () => {
332+
resolvePosts();
333+
});
334+
expect(container.innerHTML).toBe(
335+
'<div>:name::avatar:</div>' +
336+
'<div>:photos::friends:</div>' +
337+
'<div>:posts:</div>' +
338+
'<p>Game over</p>', // TODO: should not have message in prod.
339+
);
340+
341+
expect(reportedErrors).toEqual([]);
342+
});
78343
});

0 commit comments

Comments
 (0)