[Fizz] Fix Client Render after Postpone (#27905)

If we end up client rendering a boundary due to an error after we have already injected a postponed hole in that boundary we'll end up trying to target a missing segment. Since we never insert segments for an already errored boundary into the HTML. Normally an errored prerender wouldn't be used but if it is, such as if it was an intentional client error it triggers this case. Those should really be replaced with postpones though. This is a bit annoying since we eagerly build up the postponed path. I took the easy route here and just cleared out the suspense boundary itself from having any postponed slots. However, this still creates an unnecessary replay path along the way to the boundary. We could probably walk the path and remove any empty parent nodes. What is worse is that if this is the only thing that postponed, we'd still generate a postponed state even though there's actually nothing to resume. Since this is a bit of an edge case already maybe it's fine. In my test I added a check for the `error` event on `window` since this error only surfaces by throwing an ignored error. We should really do that globally for all tests. Our tests should fail by default if there's an error logged to the window. DiffTrain build for [f9dddcb](f9dddcb)
facebook · Jan 9, 2024 · 3d4df0a · 3d4df0a
1 parent f748f73
commit 3d4df0a
Show file tree

Hide file tree

Showing 9 changed files with 145 additions and 16 deletions.
diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION
@@ -1 +1 @@
-f1039be4a48384e7e4b0a87d4d92c48e900053b9
+f9dddcbbb1c0b73f974e78b9488927b778630682
diff --git a/compiled/facebook-www/React-dev.modern.js b/compiled/facebook-www/React-dev.modern.js
@@ -24,7 +24,7 @@ if (__DEV__) {
     ) {
       __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStart(new Error());
     }
-    var ReactVersion = "18.3.0-www-modern-b06e722a";
+    var ReactVersion = "18.3.0-www-modern-5ca2fa17";
 
     // ATTENTION
     // When adding new symbols to this file,

diff --git a/compiled/facebook-www/ReactDOMServer-dev.classic.js b/compiled/facebook-www/ReactDOMServer-dev.classic.js
@@ -19,7 +19,7 @@ if (__DEV__) {
     var React = require("react");
     var ReactDOM = require("react-dom");
 
-    var ReactVersion = "18.3.0-www-classic-350ad947";
+    var ReactVersion = "18.3.0-www-classic-df9a06ca";
 
     // This refers to a WWW module.
     var warningWWW = require("warning");
@@ -11222,7 +11222,8 @@ if (__DEV__) {
           errorDigest = logRecoverableError(request, error, thrownInfo);
         }
 
-        encodeErrorForBoundary(newBoundary, errorDigest, error, thrownInfo); // We don't need to decrement any task numbers because we didn't spawn any new task.
+        encodeErrorForBoundary(newBoundary, errorDigest, error, thrownInfo);
+        untrackBoundary(request, newBoundary); // We don't need to decrement any task numbers because we didn't spawn any new task.
         // We don't need to schedule any task because we know the parent has written yet.
         // We do need to fallthrough to create the fallback though.
       } finally {
@@ -12709,6 +12710,34 @@ if (__DEV__) {
       task.treeContext = prevTreeContext;
       task.keyPath = prevKeyPath;
     }
+    // resume it.
+
+    function untrackBoundary(request, boundary) {
+      var trackedPostpones = request.trackedPostpones;
+
+      if (trackedPostpones === null) {
+        return;
+      }
+
+      var boundaryKeyPath = boundary.trackedContentKeyPath;
+
+      if (boundaryKeyPath === null) {
+        return;
+      }
+
+      var boundaryNode = trackedPostpones.workingMap.get(boundaryKeyPath);
+
+      if (boundaryNode === undefined) {
+        return;
+      } // Downgrade to plain ReplayNode since we won't replay through it.
+      // $FlowFixMe[cannot-write]: We intentionally downgrade this to the other tuple.
+
+      boundaryNode.length = 4; // Remove any resumable slots.
+
+      boundaryNode[2] = [];
+      boundaryNode[3] = null; // TODO: We should really just remove the boundary from all parent paths too so
+      // we don't replay the path to it.
+    }
 
     function spawnNewSuspendedReplayTask(request, task, thenableState, x) {
       var newTask = createReplayTask(
@@ -12932,7 +12961,8 @@ if (__DEV__) {
 
         if (boundary.status !== CLIENT_RENDERED) {
           boundary.status = CLIENT_RENDERED;
-          encodeErrorForBoundary(boundary, errorDigest, error, errorInfo); // Regardless of what happens next, this boundary won't be displayed,
+          encodeErrorForBoundary(boundary, errorDigest, error, errorInfo);
+          untrackBoundary(request, boundary); // Regardless of what happens next, this boundary won't be displayed,
           // so we can flush it, if the parent already flushed.
 
           if (boundary.parentFlushed) {
@@ -13144,6 +13174,7 @@ if (__DEV__) {
             errorMessage,
             _errorInfo
           );
+          untrackBoundary(request, boundary);
 
           if (boundary.parentFlushed) {
             request.clientRenderedBoundaries.push(boundary);

diff --git a/compiled/facebook-www/ReactDOMServer-dev.modern.js b/compiled/facebook-www/ReactDOMServer-dev.modern.js
@@ -19,7 +19,7 @@ if (__DEV__) {
     var React = require("react");
     var ReactDOM = require("react-dom");
 
-    var ReactVersion = "18.3.0-www-modern-73b83dcd";
+    var ReactVersion = "18.3.0-www-modern-9282fd09";
 
     // This refers to a WWW module.
     var warningWWW = require("warning");
@@ -10961,7 +10961,8 @@ if (__DEV__) {
           errorDigest = logRecoverableError(request, error, thrownInfo);
         }
 
-        encodeErrorForBoundary(newBoundary, errorDigest, error, thrownInfo); // We don't need to decrement any task numbers because we didn't spawn any new task.
+        encodeErrorForBoundary(newBoundary, errorDigest, error, thrownInfo);
+        untrackBoundary(request, newBoundary); // We don't need to decrement any task numbers because we didn't spawn any new task.
         // We don't need to schedule any task because we know the parent has written yet.
         // We do need to fallthrough to create the fallback though.
       } finally {
@@ -12437,6 +12438,34 @@ if (__DEV__) {
       task.treeContext = prevTreeContext;
       task.keyPath = prevKeyPath;
     }
+    // resume it.
+
+    function untrackBoundary(request, boundary) {
+      var trackedPostpones = request.trackedPostpones;
+
+      if (trackedPostpones === null) {
+        return;
+      }
+
+      var boundaryKeyPath = boundary.trackedContentKeyPath;
+
+      if (boundaryKeyPath === null) {
+        return;
+      }
+
+      var boundaryNode = trackedPostpones.workingMap.get(boundaryKeyPath);
+
+      if (boundaryNode === undefined) {
+        return;
+      } // Downgrade to plain ReplayNode since we won't replay through it.
+      // $FlowFixMe[cannot-write]: We intentionally downgrade this to the other tuple.
+
+      boundaryNode.length = 4; // Remove any resumable slots.
+
+      boundaryNode[2] = [];
+      boundaryNode[3] = null; // TODO: We should really just remove the boundary from all parent paths too so
+      // we don't replay the path to it.
+    }
 
     function spawnNewSuspendedReplayTask(request, task, thenableState, x) {
       var newTask = createReplayTask(
@@ -12660,7 +12689,8 @@ if (__DEV__) {
 
         if (boundary.status !== CLIENT_RENDERED) {
           boundary.status = CLIENT_RENDERED;
-          encodeErrorForBoundary(boundary, errorDigest, error, errorInfo); // Regardless of what happens next, this boundary won't be displayed,
+          encodeErrorForBoundary(boundary, errorDigest, error, errorInfo);
+          untrackBoundary(request, boundary); // Regardless of what happens next, this boundary won't be displayed,
           // so we can flush it, if the parent already flushed.
 
           if (boundary.parentFlushed) {
@@ -12872,6 +12902,7 @@ if (__DEV__) {
             errorMessage,
             _errorInfo
           );
+          untrackBoundary(request, boundary);
 
           if (boundary.parentFlushed) {
             request.clientRenderedBoundaries.push(boundary);

diff --git a/compiled/facebook-www/ReactDOMServer-prod.classic.js b/compiled/facebook-www/ReactDOMServer-prod.classic.js
@@ -3981,7 +3981,8 @@ function renderElement(
                 error,
                 previousComponentStack
               )),
-              (ref.errorDigest = JSCompiler_inline_result);
+              (ref.errorDigest = JSCompiler_inline_result),
+              untrackBoundary(request, ref);
           } finally {
             (request.renderState.boundaryResources = contextKey
               ? contextKey.resources
@@ -4527,6 +4528,15 @@ function renderChildrenArray(request, task, children, childIndex) {
   task.treeContext = replay;
   task.keyPath = prevKeyPath;
 }
+function untrackBoundary(request, boundary) {
+  request = request.trackedPostpones;
+  null !== request &&
+    ((boundary = boundary.trackedContentKeyPath),
+    null !== boundary &&
+      ((boundary = request.workingMap.get(boundary)),
+      void 0 !== boundary &&
+        ((boundary.length = 4), (boundary[2] = []), (boundary[3] = null))));
+}
 function renderNode(request, task, node, childIndex) {
   var previousFormatContext = task.formatContext,
     previousLegacyContext = task.legacyContext,
@@ -4723,6 +4733,7 @@ function abortTask(task, request, error) {
         (task = getThrownInfo(request, task.componentStack)),
         (task = logRecoverableError(request, error, task)),
         (boundary.errorDigest = task),
+        untrackBoundary(request, boundary),
         boundary.parentFlushed &&
           request.clientRenderedBoundaries.push(boundary)),
       boundary.fallbackAbortableTasks.forEach(function (fallbackTask) {
@@ -5011,6 +5022,7 @@ function performWork(request$jscomp$2) {
                   4 !== boundary$jscomp$0.status &&
                     ((boundary$jscomp$0.status = 4),
                     (boundary$jscomp$0.errorDigest = request$jscomp$0),
+                    untrackBoundary(request, boundary$jscomp$0),
                     boundary$jscomp$0.parentFlushed &&
                       request.clientRenderedBoundaries.push(
                         boundary$jscomp$0
@@ -5664,4 +5676,4 @@ exports.renderToString = function (children, options) {
     'The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server'
   );
 };
-exports.version = "18.3.0-www-classic-ef6a5528";
+exports.version = "18.3.0-www-classic-3dabb062";
diff --git a/compiled/facebook-www/ReactDOMServer-prod.modern.js b/compiled/facebook-www/ReactDOMServer-prod.modern.js
@@ -3918,7 +3918,8 @@ function renderElement(
               (contextType$jscomp$0.status = 4),
               (contextType = getThrownInfo(request, task.componentStack)),
               (initialState = logRecoverableError(request, error, contextType)),
-              (contextType$jscomp$0.errorDigest = initialState);
+              (contextType$jscomp$0.errorDigest = initialState),
+              untrackBoundary(request, contextType$jscomp$0);
           } finally {
             (request.renderState.boundaryResources = prevThenableState
               ? prevThenableState.resources
@@ -4452,6 +4453,15 @@ function renderChildrenArray(request, task, children, childIndex) {
   task.treeContext = replay;
   task.keyPath = prevKeyPath;
 }
+function untrackBoundary(request, boundary) {
+  request = request.trackedPostpones;
+  null !== request &&
+    ((boundary = boundary.trackedContentKeyPath),
+    null !== boundary &&
+      ((boundary = request.workingMap.get(boundary)),
+      void 0 !== boundary &&
+        ((boundary.length = 4), (boundary[2] = []), (boundary[3] = null))));
+}
 function renderNode(request, task, node, childIndex) {
   var previousFormatContext = task.formatContext,
     previousLegacyContext = task.legacyContext,
@@ -4648,6 +4658,7 @@ function abortTask(task, request, error) {
         (task = getThrownInfo(request, task.componentStack)),
         (task = logRecoverableError(request, error, task)),
         (boundary.errorDigest = task),
+        untrackBoundary(request, boundary),
         boundary.parentFlushed &&
           request.clientRenderedBoundaries.push(boundary)),
       boundary.fallbackAbortableTasks.forEach(function (fallbackTask) {
@@ -4936,6 +4947,7 @@ function performWork(request$jscomp$2) {
                   4 !== boundary$jscomp$0.status &&
                     ((boundary$jscomp$0.status = 4),
                     (boundary$jscomp$0.errorDigest = request$jscomp$0),
+                    untrackBoundary(request, boundary$jscomp$0),
                     boundary$jscomp$0.parentFlushed &&
                       request.clientRenderedBoundaries.push(
                         boundary$jscomp$0
@@ -5589,4 +5601,4 @@ exports.renderToString = function (children, options) {
     'The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server'
   );
 };
-exports.version = "18.3.0-www-modern-5f7638f4";
+exports.version = "18.3.0-www-modern-c6ac8c72";
diff --git a/compiled/facebook-www/ReactDOMServerStreaming-dev.modern.js b/compiled/facebook-www/ReactDOMServerStreaming-dev.modern.js
@@ -10840,7 +10840,8 @@ if (__DEV__) {
           errorDigest = logRecoverableError(request, error, thrownInfo);
         }
 
-        encodeErrorForBoundary(newBoundary, errorDigest, error, thrownInfo); // We don't need to decrement any task numbers because we didn't spawn any new task.
+        encodeErrorForBoundary(newBoundary, errorDigest, error, thrownInfo);
+        untrackBoundary(request, newBoundary); // We don't need to decrement any task numbers because we didn't spawn any new task.
         // We don't need to schedule any task because we know the parent has written yet.
         // We do need to fallthrough to create the fallback though.
       } finally {
@@ -12316,6 +12317,34 @@ if (__DEV__) {
       task.treeContext = prevTreeContext;
       task.keyPath = prevKeyPath;
     }
+    // resume it.
+
+    function untrackBoundary(request, boundary) {
+      var trackedPostpones = request.trackedPostpones;
+
+      if (trackedPostpones === null) {
+        return;
+      }
+
+      var boundaryKeyPath = boundary.trackedContentKeyPath;
+
+      if (boundaryKeyPath === null) {
+        return;
+      }
+
+      var boundaryNode = trackedPostpones.workingMap.get(boundaryKeyPath);
+
+      if (boundaryNode === undefined) {
+        return;
+      } // Downgrade to plain ReplayNode since we won't replay through it.
+      // $FlowFixMe[cannot-write]: We intentionally downgrade this to the other tuple.
+
+      boundaryNode.length = 4; // Remove any resumable slots.
+
+      boundaryNode[2] = [];
+      boundaryNode[3] = null; // TODO: We should really just remove the boundary from all parent paths too so
+      // we don't replay the path to it.
+    }
 
     function spawnNewSuspendedReplayTask(request, task, thenableState, x) {
       var newTask = createReplayTask(
@@ -12539,7 +12568,8 @@ if (__DEV__) {
 
         if (boundary.status !== CLIENT_RENDERED) {
           boundary.status = CLIENT_RENDERED;
-          encodeErrorForBoundary(boundary, errorDigest, error, errorInfo); // Regardless of what happens next, this boundary won't be displayed,
+          encodeErrorForBoundary(boundary, errorDigest, error, errorInfo);
+          untrackBoundary(request, boundary); // Regardless of what happens next, this boundary won't be displayed,
           // so we can flush it, if the parent already flushed.
 
           if (boundary.parentFlushed) {
@@ -12751,6 +12781,7 @@ if (__DEV__) {
             errorMessage,
             _errorInfo
           );
+          untrackBoundary(request, boundary);
 
           if (boundary.parentFlushed) {
             request.clientRenderedBoundaries.push(boundary);

diff --git a/compiled/facebook-www/ReactDOMServerStreaming-prod.modern.js b/compiled/facebook-www/ReactDOMServerStreaming-prod.modern.js
@@ -3736,7 +3736,8 @@ function renderElement(
               (contextType$jscomp$0.status = 4),
               (contextType = getThrownInfo(request, task.componentStack)),
               (initialState = logRecoverableError(request, error, contextType)),
-              (contextType$jscomp$0.errorDigest = initialState);
+              (contextType$jscomp$0.errorDigest = initialState),
+              untrackBoundary(request, contextType$jscomp$0);
           } finally {
             (request.renderState.boundaryResources = prevThenableState
               ? prevThenableState.resources
@@ -4285,6 +4286,15 @@ function renderChildrenArray(request, task, children, childIndex) {
   task.treeContext = replay;
   task.keyPath = prevKeyPath;
 }
+function untrackBoundary(request, boundary) {
+  request = request.trackedPostpones;
+  null !== request &&
+    ((boundary = boundary.trackedContentKeyPath),
+    null !== boundary &&
+      ((boundary = request.workingMap.get(boundary)),
+      void 0 !== boundary &&
+        ((boundary.length = 4), (boundary[2] = []), (boundary[3] = null))));
+}
 function renderNode(request, task, node, childIndex) {
   var previousFormatContext = task.formatContext,
     previousLegacyContext = task.legacyContext,
@@ -4484,6 +4494,7 @@ function abortTask(task, request, error) {
         (task = getThrownInfo(request, task.componentStack)),
         (task = logRecoverableError(request, error, task)),
         (boundary.errorDigest = task),
+        untrackBoundary(request, boundary),
         boundary.parentFlushed &&
           request.clientRenderedBoundaries.push(boundary)),
       boundary.fallbackAbortableTasks.forEach(function (fallbackTask) {
@@ -5320,6 +5331,7 @@ exports.renderNextChunk = function (stream) {
                     4 !== boundary$jscomp$0.status &&
                       ((boundary$jscomp$0.status = 4),
                       (boundary$jscomp$0.errorDigest = errorDigest),
+                      untrackBoundary(request, boundary$jscomp$0),
                       boundary$jscomp$0.parentFlushed &&
                         request.clientRenderedBoundaries.push(
                           boundary$jscomp$0

diff --git a/compiled/facebook-www/ReactSharedSubset-prod.modern.js b/compiled/facebook-www/ReactSharedSubset-prod.modern.js
@@ -489,4 +489,4 @@ exports.useId = function () {
 exports.useMemo = function (create, deps) {
   return ReactCurrentDispatcher.current.useMemo(create, deps);
 };
-exports.version = "18.3.0-www-modern-73b83dcd";
+exports.version = "18.3.0-www-modern-9282fd09";