[act] reset scope depth on synchronous errors (#15937)

* reset scope depth on synchronous errors we weren't resetting the acting scope depth on sync errors thrown in the callback. this fixes that. * typos * add a test to make sure sync error propagate
facebook · Jun 20, 2019 · ff91bfa · ff91bfa
1 parent e61c9e0
commit ff91bfa
Show file tree

Hide file tree

Showing 4 changed files with 133 additions and 17 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
@@ -404,20 +404,6 @@ function runActTests(label, render, unmount) {
         expect(container.innerHTML).toBe('1');
       });
 
-      it('propagates errors', async () => {
-        let err;
-        try {
-          await act(async () => {
-            await sleep(100);
-            throw new Error('some error');
-          });
-        } catch (_err) {
-          err = _err;
-        } finally {
-          expect(err instanceof Error).toBe(true);
-          expect(err.message).toBe('some error');
-        }
-      });
       it('can handle cascading promises', async () => {
         // this component triggers an effect, that waits a tick,
         // then sets state. repeats this 5 times.
@@ -524,5 +510,111 @@ function runActTests(label, render, unmount) {
         });
       }
     });
+    describe('error propagation', () => {
+      it('propagates errors - sync', () => {
+        let err;
+        try {
+          act(() => {
+            throw new Error('some error');
+          });
+        } catch (_err) {
+          err = _err;
+        } finally {
+          expect(err instanceof Error).toBe(true);
+          expect(err.message).toBe('some error');
+        }
+      });
+
+      it('should propagate errors from effects - sync', () => {
+        function App() {
+          React.useEffect(() => {
+            throw new Error('oh no');
+          });
+          return null;
+        }
+        let error;
+
+        try {
+          act(() => {
+            render(<App />, container);
+          });
+        } catch (_error) {
+          error = _error;
+        } finally {
+          expect(error instanceof Error).toBe(true);
+          expect(error.message).toBe('oh no');
+        }
+      });
+
+      it('propagates errors - async', async () => {
+        let err;
+        try {
+          await act(async () => {
+            await sleep(100);
+            throw new Error('some error');
+          });
+        } catch (_err) {
+          err = _err;
+        } finally {
+          expect(err instanceof Error).toBe(true);
+          expect(err.message).toBe('some error');
+        }
+      });
+
+      it('should cleanup after errors - sync', () => {
+        function App() {
+          React.useEffect(() => {
+            Scheduler.yieldValue('oh yes');
+          });
+          return null;
+        }
+        let error;
+        try {
+          act(() => {
+            throw new Error('oh no');
+          });
+        } catch (_error) {
+          error = _error;
+        } finally {
+          expect(error instanceof Error).toBe(true);
+          expect(error.message).toBe('oh no');
+          // should be able to render components after this tho
+          act(() => {
+            render(<App />, container);
+          });
+          expect(Scheduler).toHaveYielded(['oh yes']);
+        }
+      });
+
+      it('should cleanup after errors - async', async () => {
+        function App() {
+          async function somethingAsync() {
+            await null;
+            Scheduler.yieldValue('oh yes');
+          }
+          React.useEffect(() => {
+            somethingAsync();
+          });
+          return null;
+        }
+        let error;
+        try {
+          await act(async () => {
+            await sleep(100);
+            throw new Error('oh no');
+          });
+        } catch (_error) {
+          error = _error;
+        } finally {
+          expect(error instanceof Error).toBe(true);
+          expect(error.message).toBe('oh no');
+          // should be able to render components after this tho
+          await act(async () => {
+            render(<App />, container);
+          });
+          expect(Scheduler).toHaveYielded(['oh yes']);
+        }
+      });
+    });
   });
 }
diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js
@@ -108,7 +108,15 @@ function act(callback: () => Thenable) {
     }
   }
 
-  const result = batchedUpdates(callback);
+  let result;
+  try {
+    result = batchedUpdates(callback);
+  } catch (error) {
+    // on sync errors, we still want to 'cleanup' and decrement actingUpdatesScopeDepth
+    onDone();
+    throw error;
+  }
+
   if (
     result !== null &&
     typeof result === 'object' &&

diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js
@@ -724,7 +724,15 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       }
     }
 
-    const result = batchedUpdates(callback);
+    let result;
+    try {
+      result = batchedUpdates(callback);
+    } catch (error) {
+      // on sync errors, we still want to 'cleanup' and decrement actingUpdatesScopeDepth
+      onDone();
+      throw error;
+    }
+
     if (
       result !== null &&
       typeof result === 'object' &&

diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js
@@ -89,7 +89,15 @@ function act(callback: () => Thenable) {
     }
   }
 
-  const result = batchedUpdates(callback);
+  let result;
+  try {
+    result = batchedUpdates(callback);
+  } catch (error) {
+    // on sync errors, we still want to 'cleanup' and decrement actingUpdatesScopeDepth
+    onDone();
+    throw error;
+  }
+
   if (
     result !== null &&
     typeof result === 'object' &&