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

package.json

@@ -35,7 +35,7 @@
     "@babel/preset-flow": "^7.10.4",
     "@babel/preset-react": "^7.10.4",
     "@babel/traverse": "^7.11.0",
-    "@mattiasbuelens/web-streams-polyfill": "^0.3.2",
+    "web-streams-polyfill": "^3.1.1",
     "abort-controller": "^3.0.0",
     "art": "0.10.1",
     "babel-eslint": "^10.0.3",

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

@@ -10,7 +10,7 @@
 'use strict';
 
 // Polyfills for test environment
-global.ReadableStream = require('@mattiasbuelens/web-streams-polyfill/ponyfill/es6').ReadableStream;
+global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream;
 global.TextEncoder = require('util').TextEncoder;
 global.AbortController = require('abort-controller');
 

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

@@ -68,6 +68,7 @@ function render(model: ReactModel, options?: Options): Destination {
     options ? options.onError : undefined,
   );
   ReactNoopFlightServer.startWork(request);
+  ReactNoopFlightServer.startFlowing(request);
   return destination;
 }
 

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

@@ -13,7 +13,11 @@ import type {
   Destination,
 } from './ReactFlightDOMRelayServerHostConfig';
 
-import {createRequest, startWork} from 'react-server/src/ReactFlightServer';
+import {
+  createRequest,
+  startWork,
+  startFlowing,
+} from 'react-server/src/ReactFlightServer';
 
 type Options = {
   onError?: (error: mixed) => void,
@@ -32,6 +36,7 @@ function render(
     options ? options.onError : undefined,
   );
   startWork(request);
+  startFlowing(request);
 }
 
 export {render};

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

@@ -26,7 +26,7 @@ function renderToReadableStream(
   options?: Options,
 ): ReadableStream {
   let request;
-  return new ReadableStream({
+  const stream = new ReadableStream({
     start(controller) {
       request = createRequest(
         model,
@@ -37,10 +37,17 @@ function renderToReadableStream(
       startWork(request);
     },
     pull(controller) {
-      startFlowing(request);
+      // Pull is called immediately even if the stream is not passed to anything.
+      // That's buffering too early. We want to start buffering once the stream
+      // is actually used by something so we can give it the best result possible
+      // at that point.
+      if (stream.locked) {
+        startFlowing(request);
+      }
     },
     cancel(reason) {},
   });
+  return stream;
 }
 
 export {renderToReadableStream};

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

@@ -37,8 +37,9 @@ function pipeToNodeWritable(
     webpackMap,
     options ? options.onError : undefined,
   );
-  destination.on('drain', createDrainHandler(destination, request));
   startWork(request);
+  startFlowing(request);
+  destination.on('drain', createDrainHandler(destination, request));
 }
 
 export {pipeToNodeWritable};

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

@@ -10,7 +10,7 @@
 'use strict';
 
 // Polyfills for test environment
-global.ReadableStream = require('@mattiasbuelens/web-streams-polyfill/ponyfill/es6').ReadableStream;
+global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream;
 global.TextDecoder = require('util').TextDecoder;
 
 // Don't wait before processing work on the server.

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

@@ -5,28 +5,56 @@
  * LICENSE file in the root directory of this source tree.
  *
  * @emails react-core
- * @jest-environment node
  */
 
 'use strict';
 
 // Polyfills for test environment
-global.ReadableStream = require('@mattiasbuelens/web-streams-polyfill/ponyfill/es6').ReadableStream;
+global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream;
 global.TextEncoder = require('util').TextEncoder;
 global.TextDecoder = require('util').TextDecoder;
 
+let webpackModuleIdx = 0;
+let webpackModules = {};
+let webpackMap = {};
+global.__webpack_require__ = function(id) {
+  return webpackModules[id];
+};
+
+let act;
 let React;
+let ReactDOM;
 let ReactServerDOMWriter;
 let ReactServerDOMReader;
 
 describe('ReactFlightDOMBrowser', () => {
   beforeEach(() => {
     jest.resetModules();
+    webpackModules = {};
+    webpackMap = {};
+    act = require('jest-react').act;
     React = require('react');
+    ReactDOM = require('react-dom');
     ReactServerDOMWriter = require('react-server-dom-webpack/writer.browser.server');
     ReactServerDOMReader = require('react-server-dom-webpack');
   });
 
+  function moduleReference(moduleExport) {
+    const idx = webpackModuleIdx++;
+    webpackModules[idx] = {
+      d: moduleExport,
+    };
+    webpackMap['path/' + idx] = {
+      default: {
+        id: '' + idx,
+        chunks: [],
+        name: 'd',
+      },
+    };
+    const MODULE_TAG = Symbol.for('react.module.reference');
+    return {$$typeof: MODULE_TAG, filepath: 'path/' + idx, name: 'default'};
+  }
+
   async function waitForSuspense(fn) {
     while (true) {
       try {
@@ -75,4 +103,241 @@ describe('ReactFlightDOMBrowser', () => {
       });
     });
   });
+
+  it('should resolve HTML using W3C streams', async () => {
+    function Text({children}) {
+      return <span>{children}</span>;
+    }
+    function HTML() {
+      return (
+        <div>
+          <Text>hello</Text>
+          <Text>world</Text>
+        </div>
+      );
+    }
+
+    function App() {
+      const model = {
+        html: <HTML />,
+      };
+      return model;
+    }
+
+    const stream = ReactServerDOMWriter.renderToReadableStream(<App />);
+    const response = ReactServerDOMReader.createFromReadableStream(stream);
+    await waitForSuspense(() => {
+      const model = response.readRoot();
+      expect(model).toEqual({
+        html: (
+          <div>
+            <span>hello</span>
+            <span>world</span>
+          </div>
+        ),
+      });
+    });
+  });
+
+  it('should progressively reveal server components', async () => {
+    let reportedErrors = [];
+    const {Suspense} = React;
+
+    // Client Components
+
+    class ErrorBoundary extends React.Component {
+      state = {hasError: false, error: null};
+      static getDerivedStateFromError(error) {
+        return {
+          hasError: true,
+          error,
+        };
+      }
+      render() {
+        if (this.state.hasError) {
+          return this.props.fallback(this.state.error);
+        }
+        return this.props.children;
+      }
+    }
+
+    function MyErrorBoundary({children}) {
+      return (
+        <ErrorBoundary fallback={e => <p>{e.message}</p>}>
+          {children}
+        </ErrorBoundary>
+      );
+    }
+
+    // Model
+    function Text({children}) {
+      return children;
+    }
+
+    function makeDelayedText() {
+      let error, _resolve, _reject;
+      let promise = new Promise((resolve, reject) => {
+        _resolve = () => {
+          promise = null;
+          resolve();
+        };
+        _reject = e => {
+          error = e;
+          promise = null;
+          reject(e);
+        };
+      });
+      function DelayedText({children}, data) {
+        if (promise) {
+          throw promise;
+        }
+        if (error) {
+          throw error;
+        }
+        return <Text>{children}</Text>;
+      }
+      return [DelayedText, _resolve, _reject];
+    }
+
+    const [Friends, resolveFriends] = makeDelayedText();
+    const [Name, resolveName] = makeDelayedText();
+    const [Posts, resolvePosts] = makeDelayedText();
+    const [Photos, resolvePhotos] = makeDelayedText();
+    const [Games, , rejectGames] = makeDelayedText();
+
+    // View
+    function ProfileDetails({avatar}) {
+      return (
+        <div>
+          <Name>:name:</Name>
+          {avatar}
+        </div>
+      );
+    }
+    function ProfileSidebar({friends}) {
+      return (
+        <div>
+          <Photos>:photos:</Photos>
+          {friends}
+        </div>
+      );
+    }
+    function ProfilePosts({posts}) {
+      return <div>{posts}</div>;
+    }
+    function ProfileGames({games}) {
+      return <div>{games}</div>;
+    }
+
+    const MyErrorBoundaryClient = moduleReference(MyErrorBoundary);
+
+    function ProfileContent() {
+      return (
+        <>
+          <ProfileDetails avatar={<Text>:avatar:</Text>} />
+          <Suspense fallback={<p>(loading sidebar)</p>}>
+            <ProfileSidebar friends={<Friends>:friends:</Friends>} />
+          </Suspense>
+          <Suspense fallback={<p>(loading posts)</p>}>
+            <ProfilePosts posts={<Posts>:posts:</Posts>} />
+          </Suspense>
+          <MyErrorBoundaryClient>
+            <Suspense fallback={<p>(loading games)</p>}>
+              <ProfileGames games={<Games>:games:</Games>} />
+            </Suspense>
+          </MyErrorBoundaryClient>
+        </>
+      );
+    }
+
+    const model = {
+      rootContent: <ProfileContent />,
+    };
+
+    function ProfilePage({response}) {
+      return response.readRoot().rootContent;
+    }
+
+    const stream = ReactServerDOMWriter.renderToReadableStream(
+      model,
+      webpackMap,
+      {
+        onError(x) {
+          reportedErrors.push(x);
+        },
+      },
+    );
+    const response = ReactServerDOMReader.createFromReadableStream(stream);
+
+    const container = document.createElement('div');
+    const root = ReactDOM.createRoot(container);
+    await act(async () => {
+      root.render(
+        <Suspense fallback={<p>(loading)</p>}>
+          <ProfilePage response={response} />
+        </Suspense>,
+      );
+    });
+    expect(container.innerHTML).toBe('<p>(loading)</p>');
+
+    // This isn't enough to show anything.
+    await act(async () => {
+      resolveFriends();
+    });
+    expect(container.innerHTML).toBe('<p>(loading)</p>');
+
+    // We can now show the details. Sidebar and posts are still loading.
+    await act(async () => {
+      resolveName();
+    });
+    // Advance time enough to trigger a nested fallback.
+    jest.advanceTimersByTime(500);
+    expect(container.innerHTML).toBe(
+      '<div>:name::avatar:</div>' +
+        '<p>(loading sidebar)</p>' +
+        '<p>(loading posts)</p>' +
+        '<p>(loading games)</p>',
+    );
+
+    expect(reportedErrors).toEqual([]);
+
+    const theError = new Error('Game over');
+    // Let's *fail* loading games.
+    await act(async () => {
+      rejectGames(theError);
+    });
+    expect(container.innerHTML).toBe(
+      '<div>:name::avatar:</div>' +
+        '<p>(loading sidebar)</p>' +
+        '<p>(loading posts)</p>' +
+        '<p>Game over</p>', // TODO: should not have message in prod.
+    );
+
+    expect(reportedErrors).toEqual([theError]);
+    reportedErrors = [];
+
+    // We can now show the sidebar.
+    await act(async () => {
+      resolvePhotos();
+    });
+    expect(container.innerHTML).toBe(
+      '<div>:name::avatar:</div>' +
+        '<div>:photos::friends:</div>' +
+        '<p>(loading posts)</p>' +
+        '<p>Game over</p>', // TODO: should not have message in prod.
+    );
+
+    // Show everything.
+    await act(async () => {
+      resolvePosts();
+    });
+    expect(container.innerHTML).toBe(
+      '<div>:name::avatar:</div>' +
+        '<div>:photos::friends:</div>' +
+        '<div>:posts:</div>' +
+        '<p>Game over</p>', // TODO: should not have message in prod.
+    );
+
+    expect(reportedErrors).toEqual([]);
+  });
 });