@samitouri / QOS-React-2 / commits / eba248c390

[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.

Sebastian Markbåge committed Sep 27, 2021 at 20:47 UTC eba248c390a5e32488536a100e2f7c0e55d43da6
12 files changed +300 -29
package.json
+1 -1
@@ -35,7 +35,7 @@
35 "@babel/preset-flow": "^7.10.4",
36 "@babel/preset-react": "^7.10.4",
37 "@babel/traverse": "^7.11.0",
38 - "@mattiasbuelens/web-streams-polyfill": "^0.3.2",
38 + "web-streams-polyfill": "^3.1.1",
39 "abort-controller": "^3.0.0",
40 "art": "0.10.1",
41 "babel-eslint": "^10.0.3",
packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
+1 -1
@@ -10,7 +10,7 @@
10 'use strict';
11
12 // 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;
14 global.TextEncoder = require('util').TextEncoder;
15 global.AbortController = require('abort-controller');
16
packages/react-noop-renderer/src/ReactNoopFlightServer.js
+1
@@ -68,6 +68,7 @@ function render(model: ReactModel, options?: Options): Destination {
68 options ? options.onError : undefined,
69 );
70 ReactNoopFlightServer.startWork(request);
71 + ReactNoopFlightServer.startFlowing(request);
72 return destination;
73 }
74
packages/react-server-dom-relay/src/ReactFlightDOMRelayServer.js
+6 -1
@@ -13,7 +13,11 @@ import type {
13 Destination,
14 } from './ReactFlightDOMRelayServerHostConfig';
15
16 -import {createRequest, startWork} from 'react-server/src/ReactFlightServer';
16 +import {
17 + createRequest,
18 + startWork,
19 + startFlowing,
20 +} from 'react-server/src/ReactFlightServer';
21
22 type Options = {
23 onError?: (error: mixed) => void,
@@ -32,6 +36,7 @@ function render(
36 options ? options.onError : undefined,
37 );
38 startWork(request);
39 + startFlowing(request);
40 }
41
42 export {render};
packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js
+9 -2
@@ -26,7 +26,7 @@ function renderToReadableStream(
26 options?: Options,
27 ): ReadableStream {
28 let request;
29 - return new ReadableStream({
29 + const stream = new ReadableStream({
30 start(controller) {
31 request = createRequest(
32 model,
@@ -37,10 +37,17 @@ function renderToReadableStream(
37 startWork(request);
38 },
39 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 + }
47 },
48 cancel(reason) {},
49 });
50 + return stream;
51 }
52
53 export {renderToReadableStream};
packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js
+2 -1
@@ -37,8 +37,9 @@ function pipeToNodeWritable(
37 webpackMap,
38 options ? options.onError : undefined,
39 );
40 - destination.on('drain', createDrainHandler(destination, request));
40 startWork(request);
41 + startFlowing(request);
42 + destination.on('drain', createDrainHandler(destination, request));
43 }
44
45 export {pipeToNodeWritable};
packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js
+1 -1
@@ -10,7 +10,7 @@
10 'use strict';
11
12 // 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;
14 global.TextDecoder = require('util').TextDecoder;
15
16 // Don't wait before processing work on the server.
packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js
+267 -2
@@ -5,28 +5,56 @@
5 * LICENSE file in the root directory of this source tree.
6 *
7 * @emails react-core
8 - * @jest-environment node
8 */
9
10 'use strict';
11
12 // 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;
14 global.TextEncoder = require('util').TextEncoder;
15 global.TextDecoder = require('util').TextDecoder;
16
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;
25 let React;
26 +let ReactDOM;
27 let ReactServerDOMWriter;
28 let ReactServerDOMReader;
29
30 describe('ReactFlightDOMBrowser', () => {
31 beforeEach(() => {
32 jest.resetModules();
33 + webpackModules = {};
34 + webpackMap = {};
35 + act = require('jest-react').act;
36 React = require('react');
37 + ReactDOM = require('react-dom');
38 ReactServerDOMWriter = require('react-server-dom-webpack/writer.browser.server');
39 ReactServerDOMReader = require('react-server-dom-webpack');
40 });
41
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 +
58 async function waitForSuspense(fn) {
59 while (true) {
60 try {
@@ -75,4 +103,241 @@ describe('ReactFlightDOMBrowser', () => {
103 });
104 });
105 });
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 + });
343 });
packages/react-server-native-relay/src/ReactFlightNativeRelayServer.js
+6 -1
@@ -13,7 +13,11 @@ import type {
13 Destination,
14 } from './ReactFlightNativeRelayServerHostConfig';
15
16 -import {createRequest, startWork} from 'react-server/src/ReactFlightServer';
16 +import {
17 + createRequest,
18 + startWork,
19 + startFlowing,
20 +} from 'react-server/src/ReactFlightServer';
21
22 function render(
23 model: ReactModel,
@@ -22,6 +26,7 @@ function render(
26 ): void {
27 const request = createRequest(model, destination, config);
28 startWork(request);
29 + startFlowing(request);
30 }
31
32 export {render};
packages/react-server/src/ReactFizzServer.js
-7
@@ -1748,13 +1748,7 @@ function flushPartiallyCompletedSegment(
1748 }
1749 }
1750
1751 -let reentrant = false;
1751 function flushCompletedQueues(request: Request): void {
1753 - if (reentrant) {
1754 - return;
1755 - }
1756 - reentrant = true;
1757 -
1752 const destination = request.destination;
1753 beginWriting(destination);
1754 try {
@@ -1840,7 +1834,6 @@ function flushCompletedQueues(request: Request): void {
1834 }
1835 largeBoundaries.splice(0, i);
1836 } finally {
1843 - reentrant = false;
1837 completeWriting(destination);
1838 flushBuffered(destination);
1839 if (
packages/react-server/src/ReactFlightServer.js
-7
@@ -706,12 +706,7 @@ function performWork(request: Request): void {
706 }
707 }
708
709 -let reentrant = false;
709 function flushCompletedChunks(request: Request): void {
711 - if (reentrant) {
712 - return;
713 - }
714 - reentrant = true;
710 const destination = request.destination;
711 beginWriting(destination);
712 try {
@@ -758,7 +753,6 @@ function flushCompletedChunks(request: Request): void {
753 }
754 errorChunks.splice(0, i);
755 } finally {
761 - reentrant = false;
756 completeWriting(destination);
757 }
758 flushBuffered(destination);
@@ -769,7 +763,6 @@ function flushCompletedChunks(request: Request): void {
763 }
764
765 export function startWork(request: Request): void {
772 - request.flowing = true;
766 scheduleWork(() => performWork(request));
767 }
768
yarn.lock
+6 -5
@@ -1853,11 +1853,6 @@
1853 "@types/yargs" "^15.0.0"
1854 chalk "^4.0.0"
1855
1856 -"@mattiasbuelens/web-streams-polyfill@^0.3.2":
1857 - version "0.3.2"
1858 - resolved "https://registry.yarnpkg.com/@mattiasbuelens/web-streams-polyfill/-/web-streams-polyfill-0.3.2.tgz#d7d180e769ac38f30c4a8e1dd9bd4412affb7f42"
1859 - integrity sha512-ANZvP8lC9IXiaPM3rwM8BGMbFIZbbj0goZT/xP2IA95UIZjEToyHXT/k8G0MmSAnxKRMh5E6oLVE6jmOt5zZ/g==
1860 -
1856 "@nodelib/fs.scandir@2.1.3":
1857 version "2.1.3"
1858 resolved "https://registry.yarnpkg.com/@nodelib/fs.scandir/-/fs.scandir-2.1.3.tgz#3a582bdb53804c6ba6d146579c46e52130cf4a3b"
@@ -6447,6 +6442,7 @@ eslint-plugin-no-unsanitized@3.1.2:
6442
6443 "eslint-plugin-react-internal@link:./scripts/eslint-rules":
6444 version "0.0.0"
6445 + uid ""
6446
6447 eslint-plugin-react@^6.7.1:
6448 version "6.10.3"
@@ -15951,6 +15947,11 @@ web-ext@^4:
15947 yargs "15.3.1"
15948 zip-dir "1.0.2"
15949
15950 +web-streams-polyfill@^3.1.1:
15951 + version "3.1.1"
15952 + resolved "https://registry.yarnpkg.com/web-streams-polyfill/-/web-streams-polyfill-3.1.1.tgz#1516f2d4ea8f1bdbfed15eb65cb2df87098c8364"
15953 + integrity sha512-Czi3fG883e96T4DLEPRvufrF2ydhOOW1+1a6c3gNjH2aIh50DNFBdfwh2AKoOf1rXvpvavAoA11Qdq9+BKjE0Q==
15954 +
15955 webidl-conversions@^4.0.2:
15956 version "4.0.2"
15957 resolved "https://registry.yarnpkg.com/webidl-conversions/-/webidl-conversions-4.0.2.tgz#a855980b1f0b6b359ba1d5d9fb39ae941faa63ad"