[Fizz] Don't double replay elements when it's the postpone point (#27440

) The `resumeElement` function wasn't actually doing the correct thing because it was resuming the element itself but what the child slot means is that we're supposed to resume in the direct child of the element. This is difficult to check for since it's all the possible branches that the element can render into, so instead we just check this in renderNode. It means the hottest path always checks the task which is unfortunate. And also, resuming using the correct nextSegmentId. Fixes two bugs surfaced by this test. --------- Co-authored-by: Josh Story <josh.c.story@gmail.com> DiffTrain build for [a6ed60a](a6ed60a)
facebook · Sep 29, 2023 · 378167a · 378167a
1 parent a67bc42
commit 378167a
Show file tree

Hide file tree

Showing 9 changed files with 846 additions and 1,311 deletions.
diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION
@@ -1 +1 @@
-c7ba8c098889b6dc47fa9c807bbba3975a658584
+a6ed60a8eb0626e5f84d0bdbb62c0b61219150d3
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-10178652";
+var ReactVersion = "18.3.0-www-modern-5845cabd";
 
 var LegacyRoot = 0;
 var ConcurrentRoot = 1;

diff --git a/compiled/facebook-www/ReactART-prod.modern.js b/compiled/facebook-www/ReactART-prod.modern.js
@@ -9772,7 +9772,7 @@ var slice = Array.prototype.slice,
       return null;
     },
     bundleType: 0,
-    version: "18.3.0-www-modern-df7dbab9",
+    version: "18.3.0-www-modern-d368bffd",
     rendererPackageName: "react-art"
   };
 var internals$jscomp$inline_1284 = {
@@ -9803,7 +9803,7 @@ var internals$jscomp$inline_1284 = {
   scheduleRoot: null,
   setRefreshHandler: null,
   getCurrentFiber: null,
-  reconcilerVersion: "18.3.0-www-modern-df7dbab9"
+  reconcilerVersion: "18.3.0-www-modern-d368bffd"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
   var hook$jscomp$inline_1285 = __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-93baf717";
+var ReactVersion = "18.3.0-www-classic-bdb0331d";
 
 // This refers to a WWW module.
 var warningWWW = require("warning");
@@ -10566,11 +10566,7 @@ function replaySuspenseBoundary(
 
   try {
     // We use the safe form because we don't handle suspending here. Only error handling.
-    if (typeof childSlots === "number") {
-      resumeNode(request, task, childSlots, content, -1);
-    } else {
-      renderNode(request, task, content, -1);
-    }
+    renderNode(request, task, content, -1);
 
     if (task.replay.pendingTasks === 1 && task.replay.nodes.length > 0) {
       throw new Error(
@@ -10625,57 +10621,28 @@ function replaySuspenseBoundary(
     task.keyPath = prevKeyPath;
   }
 
-  var fallbackKeyPath = [keyPath[0], "Suspense Fallback", keyPath[2]];
-  var suspendedFallbackTask; // We create suspended task for the fallback because we don't want to actually work
+  var fallbackKeyPath = [keyPath[0], "Suspense Fallback", keyPath[2]]; // We create suspended task for the fallback because we don't want to actually work
   // on it yet in case we finish the main content, so we queue for later.
 
-  if (typeof fallbackSlots === "number") {
-    // Resuming directly in the fallback.
-    var resumedSegment = createPendingSegment(
-      request,
-      0,
-      null,
-      task.formatContext,
-      false,
-      false
-    );
-    resumedSegment.id = fallbackSlots;
-    resumedSegment.parentFlushed = true;
-    suspendedFallbackTask = createRenderTask(
-      request,
-      null,
-      fallback,
-      -1,
-      parentBoundary,
-      resumedSegment,
-      fallbackAbortSet,
-      fallbackKeyPath,
-      task.formatContext,
-      task.legacyContext,
-      task.context,
-      task.treeContext
-    );
-  } else {
-    var fallbackReplay = {
-      nodes: fallbackNodes,
-      slots: fallbackSlots,
-      pendingTasks: 0
-    };
-    suspendedFallbackTask = createReplayTask(
-      request,
-      null,
-      fallbackReplay,
-      fallback,
-      -1,
-      parentBoundary,
-      fallbackAbortSet,
-      fallbackKeyPath,
-      task.formatContext,
-      task.legacyContext,
-      task.context,
-      task.treeContext
-    );
-  }
+  var fallbackReplay = {
+    nodes: fallbackNodes,
+    slots: fallbackSlots,
+    pendingTasks: 0
+  };
+  var suspendedFallbackTask = createReplayTask(
+    request,
+    null,
+    fallbackReplay,
+    fallback,
+    -1,
+    parentBoundary,
+    fallbackAbortSet,
+    fallbackKeyPath,
+    task.formatContext,
+    task.legacyContext,
+    task.context,
+    task.treeContext
+  );
 
   {
     suspendedFallbackTask.componentStack = task.componentStack;
@@ -11433,53 +11400,6 @@ function resumeNode(request, task, segmentId, node, childIndex) {
   }
 }
 
-function resumeElement(
-  request,
-  task,
-  keyPath,
-  segmentId,
-  prevThenableState,
-  type,
-  props,
-  ref
-) {
-  var prevReplay = task.replay;
-  var blockedBoundary = task.blockedBoundary;
-  var resumedSegment = createPendingSegment(
-    request,
-    0,
-    null,
-    task.formatContext,
-    false,
-    false
-  );
-  resumedSegment.id = segmentId;
-  resumedSegment.parentFlushed = true;
-
-  try {
-    // Convert the current ReplayTask to a RenderTask.
-    var renderTask = task;
-    renderTask.replay = null;
-    renderTask.blockedSegment = resumedSegment;
-    renderElement(request, task, keyPath, prevThenableState, type, props, ref);
-    resumedSegment.status = COMPLETED;
-
-    if (blockedBoundary === null) {
-      request.completedRootSegment = resumedSegment;
-    } else {
-      queueCompletedSegment(blockedBoundary, resumedSegment);
-
-      if (blockedBoundary.parentFlushed) {
-        request.partialBoundaries.push(blockedBoundary);
-      }
-    }
-  } finally {
-    // Restore to a ReplayTask.
-    task.replay = prevReplay;
-    task.blockedSegment = null;
-  }
-}
-
 function replayElement(
   request,
   task,
@@ -11524,29 +11444,15 @@ function replayElement(
       };
 
       try {
-        if (typeof childSlots === "number") {
-          // Matched a resumable element.
-          resumeElement(
-            request,
-            task,
-            keyPath,
-            childSlots,
-            prevThenableState,
-            type,
-            props,
-            ref
-          );
-        } else {
-          renderElement(
-            request,
-            task,
-            keyPath,
-            prevThenableState,
-            type,
-            props,
-            ref
-          );
-        }
+        renderElement(
+          request,
+          task,
+          keyPath,
+          prevThenableState,
+          type,
+          props,
+          ref
+        );
 
         if (
           task.replay.pendingTasks === 1 &&
@@ -11682,8 +11588,14 @@ function renderNodeDestructiveImpl(
   node,
   childIndex
 ) {
-  // Stash the node we're working on. We'll pick up from this task in case
+  if (task.replay !== null && typeof task.replay.slots === "number") {
+    // TODO: Figure out a cheaper place than this hot path to do this check.
+    var resumeSegmentID = task.replay.slots;
+    resumeNode(request, task, resumeSegmentID, node, childIndex);
+    return;
+  } // Stash the node we're working on. We'll pick up from this task in case
   // something suspends.
+
   task.node = node;
   task.childIndex = childIndex; // Handle object types
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		c7ba8c098889b6dc47fa9c807bbba3975a658584
		a6ed60a8eb0626e5f84d0bdbb62c0b61219150d3