Add protection against multiple roots

facebook · Apr 7, 2022 · c77a203 · c77a203
1 parent 44bd862
commit c77a203
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 16 deletions.
diff --git a/fixtures/packaging/babel-standalone/dev.html b/fixtures/packaging/babel-standalone/dev.html
@@ -5,10 +5,41 @@
     <script src="https://unpkg.com/babel-standalone@6/babel.js"></script>
     <div id="container"></div>
     <script type="text/babel">
-      ReactDOM.render(
-        <h1>Hello World!</h1>,
-        document.getElementById('container')
-      );
+      function App({ id }) {
+        const [x, setX] = React.useState(0)
+        const [y, setY] = React.useState(0)
+        React.useEffect(() => {
+          if (x < 10)
+            setX(x + 1)
+        }, [x])
+        return <h1>Root {id} [{x}]</h1>
+      }
+
+      let roots = []
+      for (let i = 0; i < 100; i++) {
+        let child = document.createElement('div')
+        document.body.appendChild(child)
+        //roots[i] = child
+ //       ReactDOM.render(
+   //       <App id={i} />,
+     //     child
+       // );        
+        roots[i] = ReactDOM.createRoot(child)
+        roots[i].render(<App id={i} />)
+      }
+
+      let iter = 1
+
+      document.body.addEventListener('click', () => {
+        iter++
+        for (let i = 0; i < 100; i++) {
+     //     ReactDOM.render(
+       //     <App id={iter + '_' + i} />,
+         //   roots[i]
+          //);   
+            roots[i].render(<App id={iter + '_' + i} />)
+        }
+      })
     </script>
   </body>
 </html>
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -401,6 +401,7 @@ let didScheduleUpdateDuringPassiveEffects = false;
 
 const NESTED_PASSIVE_UPDATE_LIMIT = 50;
 let nestedPassiveUpdateCount: number = 0;
+let rootWithPassiveNestedUpdates: FiberRoot | null = null;
 
 // If two updates are scheduled within the same event, we should treat their
 // event times as simultaneous, even if the actual clock time has advanced
@@ -2214,6 +2215,7 @@ function commitRootImpl(
     releaseRootPooledCache(root, remainingLanes);
     if (__DEV__) {
       nestedPassiveUpdateCount = 0;
+      rootWithPassiveNestedUpdates = null;
     }
   }
 
@@ -2481,7 +2483,12 @@ function flushPassiveEffectsImpl() {
     // If additional passive effects were scheduled, increment a counter. If this
     // exceeds the limit, we'll fire a warning.
     if (didScheduleUpdateDuringPassiveEffects) {
-      nestedPassiveUpdateCount++;
+      if (root === rootWithPassiveNestedUpdates) {
+        nestedPassiveUpdateCount++;
+      } else {
+        nestedPassiveUpdateCount = 0;
+        rootWithPassiveNestedUpdates = root;
+      }
     } else {
       nestedPassiveUpdateCount = 0;
     }
@@ -2760,6 +2767,8 @@ function checkForNestedUpdates() {
   if (__DEV__) {
     if (nestedPassiveUpdateCount > NESTED_PASSIVE_UPDATE_LIMIT) {
       nestedPassiveUpdateCount = 0;
+      rootWithPassiveNestedUpdates = null;
+
       console.error(
         'Maximum update depth exceeded. This can happen when a component ' +
           "calls setState inside useEffect, but useEffect either doesn't " +

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -401,6 +401,7 @@ let didScheduleUpdateDuringPassiveEffects = false;
 
 const NESTED_PASSIVE_UPDATE_LIMIT = 50;
 let nestedPassiveUpdateCount: number = 0;
+let rootWithPassiveNestedUpdates: FiberRoot | null = null;
 
 // If two updates are scheduled within the same event, we should treat their
 // event times as simultaneous, even if the actual clock time has advanced
@@ -2214,6 +2215,7 @@ function commitRootImpl(
     releaseRootPooledCache(root, remainingLanes);
     if (__DEV__) {
       nestedPassiveUpdateCount = 0;
+      rootWithPassiveNestedUpdates = null;
     }
   }
 
@@ -2481,7 +2483,12 @@ function flushPassiveEffectsImpl() {
     // If additional passive effects were scheduled, increment a counter. If this
     // exceeds the limit, we'll fire a warning.
     if (didScheduleUpdateDuringPassiveEffects) {
-      nestedPassiveUpdateCount++;
+      if (root === rootWithPassiveNestedUpdates) {
+        nestedPassiveUpdateCount++;
+      } else {
+        nestedPassiveUpdateCount = 0;
+        rootWithPassiveNestedUpdates = root;
+      }
     } else {
       nestedPassiveUpdateCount = 0;
     }
@@ -2760,6 +2767,8 @@ function checkForNestedUpdates() {
   if (__DEV__) {
     if (nestedPassiveUpdateCount > NESTED_PASSIVE_UPDATE_LIMIT) {
       nestedPassiveUpdateCount = 0;
+      rootWithPassiveNestedUpdates = null;
+
       console.error(
         'Maximum update depth exceeded. This can happen when a component ' +
           "calls setState inside useEffect, but useEffect either doesn't " +

diff --git a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js
@@ -584,16 +584,9 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
           'calls setState inside componentWillUpdate or componentDidUpdate. React limits ' +
           'the number of nested updates to prevent infinite loops.',
       );
-    }).toErrorDev(
-      gate(flags => flags.enableUseSyncExternalStoreShim)
-        ? [
-            'The result of getSnapshot should be cached to avoid an infinite loop',
-          ]
-        : [
-            'Maximum update depth exceeded.',
-            'The result of getSnapshot should be cached to avoid an infinite loop',
-          ],
-    );
+    }).toErrorDev([
+      'The result of getSnapshot should be cached to avoid an infinite loop',
+    ]);
   });
 
   test('getSnapshot can return NaN without infinite loop warning', async () => {