From 5619560f0c129f48e73a0c4fcf6f1c708ef6b6e9 Mon Sep 17 00:00:00 2001
From: Josh Larson <josh.larson@shopify.com>
Date: Wed, 19 Jan 2022 08:50:44 -0600
Subject: [PATCH 1/2] tests: add failing test to demonstrate bug in
 ReadableStream implementation

---
 .../__tests__/ReactFlightDOMBrowser-test.js   | 173 +++++++++++++++---
 1 file changed, 143 insertions(+), 30 deletions(-)

diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js
index 8b82caac6bc74..09ea1515c9997 100644
--- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js
+++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js
@@ -69,6 +69,31 @@ describe('ReactFlightDOMBrowser', () => {
     }
   }
 
+  function makeDelayedText(Model) {
+    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 <Model>{children}</Model>;
+    }
+    return [DelayedText, _resolve, _reject];
+  }
+
   it('should resolve HTML using W3C streams', async () => {
     function Text({children}) {
       return <span>{children}</span>;
@@ -174,36 +199,11 @@ describe('ReactFlightDOMBrowser', () => {
       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();
+    const [Friends, resolveFriends] = makeDelayedText(Text);
+    const [Name, resolveName] = makeDelayedText(Text);
+    const [Posts, resolvePosts] = makeDelayedText(Text);
+    const [Photos, resolvePhotos] = makeDelayedText(Text);
+    const [Games, , rejectGames] = makeDelayedText(Text);
 
     // View
     function ProfileDetails({avatar}) {
@@ -340,4 +340,117 @@ describe('ReactFlightDOMBrowser', () => {
 
     expect(reportedErrors).toEqual([]);
   });
+
+  it('should close the stream upon completion when rendering to W3C streams', async () => {
+    const {Suspense} = React;
+
+    // Model
+    function Text({children}) {
+      return children;
+    }
+
+    const [Friends, resolveFriends] = makeDelayedText(Text);
+    const [Name, resolveName] = makeDelayedText(Text);
+    const [Posts, resolvePosts] = makeDelayedText(Text);
+    const [Photos, resolvePhotos] = makeDelayedText(Text);
+
+    // 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 ProfileContent() {
+      return (
+        <Suspense fallback="(loading everything)">
+          <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>
+        </Suspense>
+      );
+    }
+
+    const model = {
+      rootContent: <ProfileContent />,
+    };
+
+    const stream = ReactServerDOMWriter.renderToReadableStream(
+      model,
+      webpackMap,
+    );
+
+    const reader = stream.getReader();
+    const decoder = new TextDecoder();
+
+    let flightResponse = '';
+    let isDone = false;
+
+    reader.read().then(function progress({done, value}) {
+      if (done) {
+        isDone = true;
+        return;
+      }
+
+      flightResponse += decoder.decode(value);
+
+      return reader.read().then(progress);
+    });
+
+    // Advance time enough to trigger a nested fallback.
+    jest.advanceTimersByTime(500);
+
+    await act(async () => {});
+
+    expect(flightResponse).toContain('(loading everything)');
+    expect(flightResponse).toContain('(loading sidebar)');
+    expect(flightResponse).toContain('(loading posts)');
+    expect(flightResponse).not.toContain(':friends:');
+    expect(flightResponse).not.toContain(':name:');
+
+    await act(async () => {
+      resolveFriends();
+    });
+
+    expect(flightResponse).toContain(':friends:');
+
+    await act(async () => {
+      resolveName();
+    });
+
+    expect(flightResponse).toContain(':name:');
+
+    await act(async () => {
+      resolvePhotos();
+    });
+
+    expect(flightResponse).toContain(':photos:');
+
+    await act(async () => {
+      resolvePosts();
+    });
+
+    expect(flightResponse).toContain(':posts:');
+
+    // Final pending chunk is written; stream should be closed.
+    expect(isDone).toBeTruthy();
+  });
 });

From 8ecc536e206fe5ffec1713874ae091a37262d133 Mon Sep 17 00:00:00 2001
From: Sebastian Markbage <sebastian@calyptus.eu>
Date: Wed, 23 Feb 2022 00:10:22 -0500
Subject: [PATCH 2/2] Re-add reentrancy avoidance

I removed the equivalency of this in #22446. However, I didn't fully
understand the intended semantics of the spec but I understand this better
now.

The spec is not actually recursive. It won't call pull again inside of a
pull. It might not call it inside startWork neither which the original
issue avoided. However, it will call pull if you enqueue to the controller
without filling up the desired size outside any call.

We could avoid that by returning a Promise from pull that we wait to
resolve until we've performed all our pending tasks. That would be the
more idiomatic solution. That's a bit more involved but since we know
understand it, we can readd the reentrancy hack since we have an easy place
to detect it. If anything, it should probably throw or log here otherwise.

I believe this fixes #22772.

This includes the test from #22889 but should ideally have one for Fizz.
---
 packages/react-server/src/ReactFizzServer.js   | 4 ++++
 packages/react-server/src/ReactFlightServer.js | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js
index e46aee9a45cd2..39fcdf3168205 100644
--- a/packages/react-server/src/ReactFizzServer.js
+++ b/packages/react-server/src/ReactFizzServer.js
@@ -1950,6 +1950,10 @@ export function startFlowing(request: Request, destination: Destination): void {
   if (request.status === CLOSED) {
     return;
   }
+  if (request.destination !== null) {
+    // We're already flowing.
+    return;
+  }
   request.destination = destination;
   try {
     flushCompletedQueues(request, destination);
diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js
index 052fa730fd91e..5ad7ed2ea041c 100644
--- a/packages/react-server/src/ReactFlightServer.js
+++ b/packages/react-server/src/ReactFlightServer.js
@@ -790,6 +790,10 @@ export function startFlowing(request: Request, destination: Destination): void {
   if (request.status === CLOSED) {
     return;
   }
+  if (request.destination !== null) {
+    // We're already flowing.
+    return;
+  }
   request.destination = destination;
   try {
     flushCompletedChunks(request, destination);