From 4f7c7d8dea309b9dc33d818f6c6df2cefa24271d Mon Sep 17 00:00:00 2001
From: Tianyu Yao <skyyao@fb.com>
Date: Fri, 21 Oct 2022 19:26:47 -0700
Subject: [PATCH] Fix remaining tests

---
 .../react-reconciler/src/ReactFiberWorkLoop.new.js     |  7 +++++--
 .../react-reconciler/src/ReactFiberWorkLoop.old.js     |  7 +++++--
 .../src/__tests__/ReactUpdaters-test.internal.js       |  9 +++++++--
 .../src/__tests__/useSubscription-test.js              | 10 ++++++----
 4 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index 946fa0f16d..8725682268 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -2497,6 +2497,7 @@ function commitRootImpl(
     }
   }
 
+  const rootUpdatePriorityBeforeLayoutEffect = root.updatePriority;
   // Check if there are any effects in the whole tree.
   // TODO: This is left over from the effect list implementation, where we had
   // to check for the existence of `firstEffect` to satisfy Flow. I think the
@@ -2697,9 +2698,11 @@ function commitRootImpl(
   // TODO: We can optimize this by not scheduling the callback earlier. Since we
   // currently schedule the callback in multiple places, will wait until those
   // are consolidated.
+  //// TODO: Should we flushPassiveEffects When `pendingPassiveEffectsLanes` is a retry lane?
   if (
     (enableUnifiedSyncLane
-      ? root.updatePriority === DiscreteEventPriority
+      ? includesSomeLane(pendingPassiveEffectsLanes, SyncLane) &&
+        rootUpdatePriorityBeforeLayoutEffect === DiscreteEventPriority
       : includesSomeLane(pendingPassiveEffectsLanes, SyncLane)) &&
     root.tag !== LegacyRoot
   ) {
@@ -2708,7 +2711,7 @@ function commitRootImpl(
 
   // Read this again, since a passive effect might have updated it
   remainingLanes = root.pendingLanes;
-  if (includesSomeLane(remainingLanes, (SyncLane: Lane))) {
+  if (includesSomeLane(remainingLanes, SyncLane)) {
     if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
       markNestedUpdateScheduled();
     }
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
index 7190ea57ec..7a5e7b9313 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -2497,6 +2497,7 @@ function commitRootImpl(
     }
   }
 
+  const rootUpdatePriorityBeforeLayoutEffect = root.updatePriority;
   // Check if there are any effects in the whole tree.
   // TODO: This is left over from the effect list implementation, where we had
   // to check for the existence of `firstEffect` to satisfy Flow. I think the
@@ -2697,9 +2698,11 @@ function commitRootImpl(
   // TODO: We can optimize this by not scheduling the callback earlier. Since we
   // currently schedule the callback in multiple places, will wait until those
   // are consolidated.
+  //// TODO: Should we flushPassiveEffects When `pendingPassiveEffectsLanes` is a retry lane?
   if (
     (enableUnifiedSyncLane
-      ? root.updatePriority === DiscreteEventPriority
+      ? includesSomeLane(pendingPassiveEffectsLanes, SyncLane) &&
+        rootUpdatePriorityBeforeLayoutEffect === DiscreteEventPriority
       : includesSomeLane(pendingPassiveEffectsLanes, SyncLane)) &&
     root.tag !== LegacyRoot
   ) {
@@ -2708,7 +2711,7 @@ function commitRootImpl(
 
   // Read this again, since a passive effect might have updated it
   remainingLanes = root.pendingLanes;
-  if (includesSomeLane(remainingLanes, (SyncLane: Lane))) {
+  if (includesSomeLane(remainingLanes, SyncLane)) {
     if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
       markNestedUpdateScheduled();
     }
diff --git a/packages/react-reconciler/src/__tests__/ReactUpdaters-test.internal.js b/packages/react-reconciler/src/__tests__/ReactUpdaters-test.internal.js
index 36c6f02bc4..4b7ea16c2e 100644
--- a/packages/react-reconciler/src/__tests__/ReactUpdaters-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactUpdaters-test.internal.js
@@ -371,10 +371,15 @@ describe('updaters', () => {
     onCommitRootShouldYield = true;
 
     await act(async () => {
-      triggerError();
+      if (gate(flags => flags.enableUnifiedSyncLane)) {
+        React.startTransition(() => {
+          triggerError();
+        });
+      } else {
+        triggerError();
+      }
     });
     expect(Scheduler).toHaveYielded(['onCommitRoot', 'error', 'onCommitRoot']);
-    //// TODO: ??
     expect(allSchedulerTypes).toEqual([[Parent], [ErrorBoundary]]);
 
     // Verify no outstanding flushes
diff --git a/packages/use-subscription/src/__tests__/useSubscription-test.js b/packages/use-subscription/src/__tests__/useSubscription-test.js
index fa6314573e..ec7efeea55 100644
--- a/packages/use-subscription/src/__tests__/useSubscription-test.js
+++ b/packages/use-subscription/src/__tests__/useSubscription-test.js
@@ -454,11 +454,13 @@ describe('useSubscription', () => {
       observableA.next('a-2');
 
       // Update again
-      //// TODO: ???
-      // renderer.update(<Parent observed={observableA} />);
-      React.startTransition(() => {
+      if (gate(flags => flags.enableUnifiedSyncLane)) {
+        React.startTransition(() => {
+          renderer.update(<Parent observed={observableA} />);
+        });
+      } else {
         renderer.update(<Parent observed={observableA} />);
-      });
+      }
 
       // Flush everything and ensure that the correct subscribable is used
       expect(Scheduler).toFlushAndYield([