Put the fix behind a flag

packages/react-reconciler/src/ReactFiberHooks.js

@@ -20,6 +20,7 @@ import type {SuspenseConfig} from './ReactFiberSuspenseConfig';
 import type {ReactPriorityLevel} from './SchedulerWithReactIntegration';
 
 import ReactSharedInternals from 'shared/ReactSharedInternals';
+import {rebasedUpdatesNeverUncommit} from 'shared/ReactFeatureFlags';
 
 import {NoWork} from './ReactFiberExpirationTime';
 import {readContext} from './ReactFiberNewContext';
@@ -776,16 +777,17 @@ function updateReducer<S, I, A>(
     let isRebasing = rebaseTime !== NoWork;
 
     do {
+      const updateExpirationTime = update.expirationTime;
       if (prevUpdate === rebaseEnd) {
         isRebasing = false;
       }
-      const updateExpirationTime = update.expirationTime;
       if (
         // Check if this update should be skipped
         updateExpirationTime < renderExpirationTime &&
         // If we're currently rebasing, don't skip this update if we already
         // committed it.
-        (!isRebasing || updateExpirationTime < rebaseTime)
+        (!rebasedUpdatesNeverUncommit ||
+          (!isRebasing || updateExpirationTime < rebaseTime))
       ) {
         // Priority is insufficient. Skip this update. If this is the first
         // skipped update, the previous update/state is the new base

packages/react-reconciler/src/ReactUpdateQueue.js

@@ -97,7 +97,10 @@ import {
 import {Callback, ShouldCapture, DidCapture} from 'shared/ReactSideEffectTags';
 import {ClassComponent} from 'shared/ReactWorkTags';
 
-import {debugRenderPhaseSideEffectsForStrictMode} from 'shared/ReactFeatureFlags';
+import {
+  debugRenderPhaseSideEffectsForStrictMode,
+  rebasedUpdatesNeverUncommit,
+} from 'shared/ReactFeatureFlags';
 
 import {StrictMode} from './ReactTypeOfMode';
 import {
@@ -479,7 +482,8 @@ export function processUpdateQueue<State>(
       updateExpirationTime < renderExpirationTime &&
       // If we're currently rebasing, don't skip this update if we already
       // committed it.
-      (!isRebasing || updateExpirationTime < rebaseTime)
+      (!rebasedUpdatesNeverUncommit ||
+        (!isRebasing || updateExpirationTime < rebaseTime))
     ) {
       // This update does not have sufficient priority. Skip it.
       if (newRebaseTime === NoWork) {

packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js

@@ -654,127 +654,133 @@ describe('ReactIncrementalUpdates', () => {
     });
   });
 
-  it('when rebasing, does not exclude updates that were already committed, regardless of priority', async () => {
-    const {useState, useLayoutEffect} = React;
-
-    let pushToLog;
-    function App() {
-      const [log, setLog] = useState('');
-      pushToLog = msg => {
-        setLog(prevLog => prevLog + msg);
-      };
+  it.experimental(
+    'when rebasing, does not exclude updates that were already committed, regardless of priority',
+    async () => {
+      const {useState, useLayoutEffect} = React;
+
+      let pushToLog;
+      function App() {
+        const [log, setLog] = useState('');
+        pushToLog = msg => {
+          setLog(prevLog => prevLog + msg);
+        };
 
-      useLayoutEffect(
-        () => {
-          Scheduler.unstable_yieldValue('Committed: ' + log);
-          if (log === 'B') {
+        useLayoutEffect(
+          () => {
+            Scheduler.unstable_yieldValue('Committed: ' + log);
+            if (log === 'B') {
+              // Right after B commits, schedule additional updates.
+              Scheduler.unstable_runWithPriority(
+                Scheduler.unstable_UserBlockingPriority,
+                () => {
+                  pushToLog('C');
+                },
+              );
+              setLog(prevLog => prevLog + 'D');
+            }
+          },
+          [log],
+        );
+
+        return log;
+      }
+
+      const root = ReactNoop.createRoot();
+      await ReactNoop.act(async () => {
+        root.render(<App />);
+      });
+      expect(Scheduler).toHaveYielded(['Committed: ']);
+      expect(root).toMatchRenderedOutput('');
+
+      await ReactNoop.act(async () => {
+        pushToLog('A');
+        Scheduler.unstable_runWithPriority(
+          Scheduler.unstable_UserBlockingPriority,
+          () => {
+            pushToLog('B');
+          },
+        );
+      });
+      expect(Scheduler).toHaveYielded([
+        // A and B are pending. B is higher priority, so we'll render that first.
+        'Committed: B',
+        // Because A comes first in the queue, we're now in rebase mode. B must
+        // be rebased on top of A. Also, in a layout effect, we received two new
+        // updates: C and D. C is user-blocking and D is synchronous.
+        //
+        // First render the synchronous update. What we're testing here is that
+        // B *is not dropped* even though it has lower than sync priority. That's
+        // because we already committed it. However, this render should not
+        // include C, because that update wasn't already committed.
+        'Committed: BD',
+        'Committed: BCD',
+        'Committed: ABCD',
+      ]);
+      expect(root).toMatchRenderedOutput('ABCD');
+    },
+  );
+
+  it.experimental(
+    'when rebasing, does not exclude updates that were already committed, regardless of priority (classes)',
+    async () => {
+      let pushToLog;
+      class App extends React.Component {
+        state = {log: ''};
+        pushToLog = msg => {
+          this.setState(prevState => ({log: prevState.log + msg}));
+        };
+        componentDidUpdate() {
+          Scheduler.unstable_yieldValue('Committed: ' + this.state.log);
+          if (this.state.log === 'B') {
             // Right after B commits, schedule additional updates.
             Scheduler.unstable_runWithPriority(
               Scheduler.unstable_UserBlockingPriority,
               () => {
-                pushToLog('C');
+                this.pushToLog('C');
               },
             );
-            setLog(prevLog => prevLog + 'D');
+            this.pushToLog('D');
           }
-        },
-        [log],
-      );
-
-      return log;
-    }
-
-    const root = ReactNoop.createRoot();
-    await ReactNoop.act(async () => {
-      root.render(<App />);
-    });
-    expect(Scheduler).toHaveYielded(['Committed: ']);
-    expect(root).toMatchRenderedOutput('');
-
-    await ReactNoop.act(async () => {
-      pushToLog('A');
-      Scheduler.unstable_runWithPriority(
-        Scheduler.unstable_UserBlockingPriority,
-        () => {
-          pushToLog('B');
-        },
-      );
-    });
-    expect(Scheduler).toHaveYielded([
-      // A and B are pending. B is higher priority, so we'll render that first.
-      'Committed: B',
-      // Because A comes first in the queue, we're now in rebase mode. B must
-      // be rebased on top of A. Also, in a layout effect, we received two new
-      // updates: C and D. C is user-blocking and D is synchronous.
-      //
-      // First render the synchronous update. What we're testing here is that
-      // B *is not dropped* even though it has lower than sync priority. That's
-      // because we already committed it. However, this render should not
-      // include C, because that update wasn't already committed.
-      'Committed: BD',
-      'Committed: BCD',
-      'Committed: ABCD',
-    ]);
-    expect(root).toMatchRenderedOutput('ABCD');
-  });
-
-  it('when rebasing, does not exclude updates that were already committed, regardless of priority (classes)', async () => {
-    let pushToLog;
-    class App extends React.Component {
-      state = {log: ''};
-      pushToLog = msg => {
-        this.setState(prevState => ({log: prevState.log + msg}));
-      };
-      componentDidUpdate() {
-        Scheduler.unstable_yieldValue('Committed: ' + this.state.log);
-        if (this.state.log === 'B') {
-          // Right after B commits, schedule additional updates.
-          Scheduler.unstable_runWithPriority(
-            Scheduler.unstable_UserBlockingPriority,
-            () => {
-              this.pushToLog('C');
-            },
-          );
-          this.pushToLog('D');
+        }
+        render() {
+          pushToLog = this.pushToLog;
+          return this.state.log;
         }
       }
-      render() {
-        pushToLog = this.pushToLog;
-        return this.state.log;
-      }
-    }
 
-    const root = ReactNoop.createRoot();
-    await ReactNoop.act(async () => {
-      root.render(<App />);
-    });
-    expect(Scheduler).toHaveYielded([]);
-    expect(root).toMatchRenderedOutput('');
-
-    await ReactNoop.act(async () => {
-      pushToLog('A');
-      Scheduler.unstable_runWithPriority(
-        Scheduler.unstable_UserBlockingPriority,
-        () => {
-          pushToLog('B');
-        },
-      );
-    });
-    expect(Scheduler).toHaveYielded([
-      // A and B are pending. B is higher priority, so we'll render that first.
-      'Committed: B',
-      // Because A comes first in the queue, we're now in rebase mode. B must
-      // be rebased on top of A. Also, in a layout effect, we received two new
-      // updates: C and D. C is user-blocking and D is synchronous.
-      //
-      // First render the synchronous update. What we're testing here is that
-      // B *is not dropped* even though it has lower than sync priority. That's
-      // because we already committed it. However, this render should not
-      // include C, because that update wasn't already committed.
-      'Committed: BD',
-      'Committed: BCD',
-      'Committed: ABCD',
-    ]);
-    expect(root).toMatchRenderedOutput('ABCD');
-  });
+      const root = ReactNoop.createRoot();
+      await ReactNoop.act(async () => {
+        root.render(<App />);
+      });
+      expect(Scheduler).toHaveYielded([]);
+      expect(root).toMatchRenderedOutput('');
+
+      await ReactNoop.act(async () => {
+        pushToLog('A');
+        Scheduler.unstable_runWithPriority(
+          Scheduler.unstable_UserBlockingPriority,
+          () => {
+            pushToLog('B');
+          },
+        );
+      });
+      expect(Scheduler).toHaveYielded([
+        // A and B are pending. B is higher priority, so we'll render that first.
+        'Committed: B',
+        // Because A comes first in the queue, we're now in rebase mode. B must
+        // be rebased on top of A. Also, in a layout effect, we received two new
+        // updates: C and D. C is user-blocking and D is synchronous.
+        //
+        // First render the synchronous update. What we're testing here is that
+        // B *is not dropped* even though it has lower than sync priority. That's
+        // because we already committed it. However, this render should not
+        // include C, because that update wasn't already committed.
+        'Committed: BD',
+        'Committed: BCD',
+        'Committed: ABCD',
+      ]);
+      expect(root).toMatchRenderedOutput('ABCD');
+    },
+  );
 });

packages/shared/ReactFeatureFlags.js

@@ -92,3 +92,5 @@ export const enableTrustedTypesIntegration = false;
 
 // Flag to turn event.target and event.currentTarget in ReactNative from a reactTag to a component instance
 export const enableNativeTargetAsInstance = false;
+
+export const rebasedUpdatesNeverUncommit = __EXPERIMENTAL__;

packages/shared/forks/ReactFeatureFlags.native-fb.js

@@ -43,6 +43,7 @@ export const warnAboutStringRefs = false;
 export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
 export const enableTrustedTypesIntegration = false;
+export const rebasedUpdatesNeverUncommit = __EXPERIMENTAL__;
 
 // Only used in www builds.
 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.native-oss.js

@@ -38,6 +38,7 @@ export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
 export const enableTrustedTypesIntegration = false;
 export const enableNativeTargetAsInstance = false;
+export const rebasedUpdatesNeverUncommit = __EXPERIMENTAL__;
 
 // Only used in www builds.
 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.persistent.js

@@ -38,6 +38,7 @@ export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
 export const enableTrustedTypesIntegration = false;
 export const enableNativeTargetAsInstance = false;
+export const rebasedUpdatesNeverUncommit = __EXPERIMENTAL__;
 
 // Only used in www builds.
 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.test-renderer.js

@@ -38,6 +38,7 @@ export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
 export const enableTrustedTypesIntegration = false;
 export const enableNativeTargetAsInstance = false;
+export const rebasedUpdatesNeverUncommit = __EXPERIMENTAL__;
 
 // Only used in www builds.
 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

@@ -36,6 +36,7 @@ export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
 export const enableTrustedTypesIntegration = false;
 export const enableNativeTargetAsInstance = false;
+export const rebasedUpdatesNeverUncommit = __EXPERIMENTAL__;
 
 // Only used in www builds.
 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.www.js

@@ -87,6 +87,8 @@ export const flushSuspenseFallbacksInTests = true;
 
 export const enableNativeTargetAsInstance = false;
 
+export const rebasedUpdatesNeverUncommit = __EXPERIMENTAL__;
+
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
 type Check<_X, Y: _X, X: Y = _X> = null;