DevTools: Fix "unknown" updater in profiler when a component unsuspends

facebook · Jun 2, 2024 · dcc5ca6 · dcc5ca6
1 parent a9394fc
commit dcc5ca6
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 5 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js
@@ -625,6 +625,27 @@ export function markRootUpdated(root: FiberRoot, updateLane: Lane) {
   // idle updates until after all the regular updates have finished; there's no
   // way it could unblock a transition.
   if (updateLane !== IdleLane) {
+    if (enableUpdaterTracking) {
+      if (isDevToolsPresent) {
+        // transfer pending updaters from pingedLanes to updateLane
+        const pendingUpdatersLaneMap = root.pendingUpdatersLaneMap;
+        const updaters = pendingUpdatersLaneMap[laneToIndex(updateLane)];
+        let lanes = root.pingedLanes;
+        while (lanes > 0) {
+          const index = laneToIndex(lanes);
+          const lane = 1 << index;
+
+          const pingedUpdaters = pendingUpdatersLaneMap[index];
+          pingedUpdaters.forEach(pingedUpdater => {
+            updaters.add(pingedUpdater);
+          });
+          pingedUpdaters.clear();
+
+          lanes &= ~lane;
+        }
+      }
+    }
+
     root.suspendedLanes = NoLanes;
     root.pingedLanes = NoLanes;
   }
@@ -656,7 +677,8 @@ export function markRootSuspended(
 }
 
 export function markRootPinged(root: FiberRoot, pingedLanes: Lanes) {
-  root.pingedLanes |= root.suspendedLanes & pingedLanes;
+  // TODO: When would we ever ping lanes that we aren't suspended on?
+  root.pingedLanes |= pingedLanes;
 }
 
 export function markRootFinished(

diff --git a/packages/react-reconciler/src/__tests__/ReactUpdaters-test.internal.js b/packages/react-reconciler/src/__tests__/ReactUpdaters-test.internal.js
@@ -49,6 +49,14 @@ describe('updaters', () => {
           schedulerTags.push(fiber.tag);
           schedulerTypes.push(fiber.elementType);
         });
+        fiberRoot.pendingUpdatersLaneMap.forEach((pendingUpdaters, index) => {
+          if (pendingUpdaters.size > 0) {
+            const lane = 1 << index;
+            throw new Error(
+              `Lane ${lane} has pending updaters. Either you didn't assert on all updates in your test or React is leaking updaters.`,
+            );
+          }
+        });
         allSchedulerTags.push(schedulerTags);
         allSchedulerTypes.push(schedulerTypes);
       }),
@@ -266,9 +274,6 @@ describe('updaters', () => {
     await waitForAll([]);
   });
 
-  // This test should be convertable to createRoot but the allScheduledTypes assertions are no longer the same
-  // So I'm leaving it in legacy mode for now and just disabling if legacy mode is turned off
-  // @gate !disableLegacyMode
   it('should cover suspense pings', async () => {
     let data = null;
     let resolver = null;
@@ -323,7 +328,7 @@ describe('updaters', () => {
       return promise;
     });
     assertLog(['onCommitRoot']);
-    expect(allSchedulerTypes).toEqual([[null], [Suspender], []]);
+    expect(allSchedulerTypes).toEqual([[null], [Suspender], [Suspender]]);
 
     // Verify no outstanding flushes
     await waitForAll([]);