[Fizz] Don't pop the replay stack if we've already rendered past an e…

…lement (#27513) This is the same problem as we had with keyPath before where if the element itself suspends, we have to restore the replay node to what it was before, however, if something below the element suspends we shouldn't pop it because that will pop it back up the stack. Instead of passing replay as an argument to every renderElement function, I use a hack to compare if the node is still the same as the one we tried to render, then that means we haven't stepped down into the child yet. Maybe this is not quite correct because in theory you could have a recursive node that just renders itself over and over until some context bails out. This solves an issue where if you suspended in an element it would retry trying to replay from that element but using the postponed state from the root. DiffTrain build for [09fbee8](09fbee8)
facebook · Oct 13, 2023 · bd95626 · bd95626
1 parent 46a8f79
commit bd95626
Show file tree

Hide file tree

Showing 9 changed files with 275 additions and 227 deletions.
diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION
@@ -1 +1 @@
-bb778528d1ca22b44dad832f0258aaa4c0e6d4a4
+09fbee89d62bc1c4a00e64474346cb9bc87682cb
diff --git a/compiled/facebook-www/ReactART-dev.modern.js b/compiled/facebook-www/ReactART-dev.modern.js
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
   return self;
 }
 
-var ReactVersion = "18.3.0-www-modern-70e823a6";
+var ReactVersion = "18.3.0-www-modern-88adeddd";
 
 var LegacyRoot = 0;
 var ConcurrentRoot = 1;

diff --git a/compiled/facebook-www/ReactART-prod.modern.js b/compiled/facebook-www/ReactART-prod.modern.js
@@ -9780,7 +9780,7 @@ var slice = Array.prototype.slice,
       return null;
     },
     bundleType: 0,
-    version: "18.3.0-www-modern-7f660f7a",
+    version: "18.3.0-www-modern-0b7b822d",
     rendererPackageName: "react-art"
   };
 var internals$jscomp$inline_1286 = {
@@ -9811,7 +9811,7 @@ var internals$jscomp$inline_1286 = {
   scheduleRoot: null,
   setRefreshHandler: null,
   getCurrentFiber: null,
-  reconcilerVersion: "18.3.0-www-modern-7f660f7a"
+  reconcilerVersion: "18.3.0-www-modern-0b7b822d"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
   var hook$jscomp$inline_1287 = __REACT_DEVTOOLS_GLOBAL_HOOK__;

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-c2645119";
+var ReactVersion = "18.3.0-www-classic-d0c392db";
 
 // This refers to a WWW module.
 var warningWWW = require("warning");
@@ -11437,6 +11437,7 @@ function replayElement(
 
       var childNodes = node[2];
       var childSlots = node[3];
+      var currentNode = task.node;
       task.replay = {
         nodes: childNodes,
         slots: childSlots,
@@ -11463,25 +11464,33 @@ function replayElement(
               "The tree doesn't match so React will fallback to client rendering."
           );
         }
+
+        task.replay.pendingTasks--;
       } catch (x) {
         if (
           typeof x === "object" &&
           x !== null &&
           (x === SuspenseException || typeof x.then === "function")
         ) {
           // Suspend
+          if (task.node === currentNode) {
+            // This same element suspended so we need to pop the replay we just added.
+            task.replay = replay;
+          }
+
           throw x;
-        } // Unlike regular render, we don't terminate the siblings if we error
+        }
+
+        task.replay.pendingTasks--; // Unlike regular render, we don't terminate the siblings if we error
         // during a replay. That's because this component didn't actually error
         // in the original prerender. What's unable to complete is the child
         // replay nodes which might be Suspense boundaries which are able to
         // absorb the error and we can still continue with siblings.
 
         erroredReplay(request, task.blockedBoundary, x, childNodes, childSlots);
-      } finally {
-        task.replay.pendingTasks--;
-        task.replay = replay;
       }
+
+      task.replay = replay;
     } else {
       // Let's double check that the component type matches.
       if (type !== REACT_SUSPENSE_TYPE) {
@@ -11843,6 +11852,8 @@ function replayFragment(request, task, children, childIndex) {
             "The tree doesn't match so React will fallback to client rendering."
         );
       }
+
+      task.replay.pendingTasks--;
     } catch (x) {
       if (
         typeof x === "object" &&
@@ -11851,18 +11862,19 @@ function replayFragment(request, task, children, childIndex) {
       ) {
         // Suspend
         throw x;
-      } // Unlike regular render, we don't terminate the siblings if we error
+      }
+
+      task.replay.pendingTasks--; // Unlike regular render, we don't terminate the siblings if we error
       // during a replay. That's because this component didn't actually error
       // in the original prerender. What's unable to complete is the child
       // replay nodes which might be Suspense boundaries which are able to
       // absorb the error and we can still continue with siblings.
       // This is an error, stash the component stack if it is null.
 
       erroredReplay(request, task.blockedBoundary, x, childNodes, childSlots);
-    } finally {
-      task.replay.pendingTasks--;
-      task.replay = replay;
-    } // We finished rendering this node, so now we can consume this
+    }
+
+    task.replay = replay; // We finished rendering this node, so now we can consume this
     // slot. This must happen after in case we rerender this task.
 
     replayNodes.splice(j, 1);
@@ -11902,7 +11914,7 @@ function renderChildrenArray(request, task, children, childIndex) {
         task.treeContext = pushTreeContext(prevTreeContext, totalChildren, i); // We need to use the non-destructive form so that we can safely pop back
         // up and render the sibling if something suspends.
 
-        var resumeSegmentID = resumeSlots[i];
+        var resumeSegmentID = resumeSlots[i]; // TODO: If this errors we should still continue with the next sibling.
 
         if (typeof resumeSegmentID === "number") {
           resumeNode(request, task, resumeSegmentID, node, i); // We finished rendering this node, so now we can consume this

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-70e823a6";
+var ReactVersion = "18.3.0-www-modern-88adeddd";
 
 // This refers to a WWW module.
 var warningWWW = require("warning");
@@ -11185,6 +11185,7 @@ function replayElement(
 
       var childNodes = node[2];
       var childSlots = node[3];
+      var currentNode = task.node;
       task.replay = {
         nodes: childNodes,
         slots: childSlots,
@@ -11211,25 +11212,33 @@ function replayElement(
               "The tree doesn't match so React will fallback to client rendering."
           );
         }
+
+        task.replay.pendingTasks--;
       } catch (x) {
         if (
           typeof x === "object" &&
           x !== null &&
           (x === SuspenseException || typeof x.then === "function")
         ) {
           // Suspend
+          if (task.node === currentNode) {
+            // This same element suspended so we need to pop the replay we just added.
+            task.replay = replay;
+          }
+
           throw x;
-        } // Unlike regular render, we don't terminate the siblings if we error
+        }
+
+        task.replay.pendingTasks--; // Unlike regular render, we don't terminate the siblings if we error
         // during a replay. That's because this component didn't actually error
         // in the original prerender. What's unable to complete is the child
         // replay nodes which might be Suspense boundaries which are able to
         // absorb the error and we can still continue with siblings.
 
         erroredReplay(request, task.blockedBoundary, x, childNodes, childSlots);
-      } finally {
-        task.replay.pendingTasks--;
-        task.replay = replay;
       }
+
+      task.replay = replay;
     } else {
       // Let's double check that the component type matches.
       if (type !== REACT_SUSPENSE_TYPE) {
@@ -11591,6 +11600,8 @@ function replayFragment(request, task, children, childIndex) {
             "The tree doesn't match so React will fallback to client rendering."
         );
       }
+
+      task.replay.pendingTasks--;
     } catch (x) {
       if (
         typeof x === "object" &&
@@ -11599,18 +11610,19 @@ function replayFragment(request, task, children, childIndex) {
       ) {
         // Suspend
         throw x;
-      } // Unlike regular render, we don't terminate the siblings if we error
+      }
+
+      task.replay.pendingTasks--; // Unlike regular render, we don't terminate the siblings if we error
       // during a replay. That's because this component didn't actually error
       // in the original prerender. What's unable to complete is the child
       // replay nodes which might be Suspense boundaries which are able to
       // absorb the error and we can still continue with siblings.
       // This is an error, stash the component stack if it is null.
 
       erroredReplay(request, task.blockedBoundary, x, childNodes, childSlots);
-    } finally {
-      task.replay.pendingTasks--;
-      task.replay = replay;
-    } // We finished rendering this node, so now we can consume this
+    }
+
+    task.replay = replay; // We finished rendering this node, so now we can consume this
     // slot. This must happen after in case we rerender this task.
 
     replayNodes.splice(j, 1);
@@ -11650,7 +11662,7 @@ function renderChildrenArray(request, task, children, childIndex) {
         task.treeContext = pushTreeContext(prevTreeContext, totalChildren, i); // We need to use the non-destructive form so that we can safely pop back
         // up and render the sibling if something suspends.
 
-        var resumeSegmentID = resumeSlots[i];
+        var resumeSegmentID = resumeSlots[i]; // TODO: If this errors we should still continue with the next sibling.
 
         if (typeof resumeSegmentID === "number") {
           resumeNode(request, task, resumeSegmentID, node, i); // We finished rendering this node, so now we can consume this
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		bb778528d1ca22b44dad832f0258aaa4c0e6d4a4
		09fbee89d62bc1c4a00e64474346cb9bc87682cb