Always skip unmounted/unmounting error boundaries (facebook#19627)

The behavior of error boundaries for passive effects that throw during cleanup was recently changed so that React ignores boundaries which are also unmounting in favor of still-mounted boundaries. This commit implements that same behavior for layout effects (useLayoutEffect, componentWillUnmount, and ref-detachment). The new, skip-unmounting-boundaries behavior is behind a feature flag (`skipUnmountedBoundaries`).
bvaughn · Aug 17, 2020 · bcca5a6 · bcca5a6
1 parent 1a41a19
commit bcca5a6
Show file tree

Hide file tree

Showing 17 changed files with 480 additions and 73 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js
@@ -2473,4 +2473,175 @@ describe('ReactErrorBoundaries', () => {
       'Caught an error: gotta catch em all.',
     );
   });
+
+  // @gate skipUnmountedBoundaries
+  it('catches errors thrown in componentWillUnmount', () => {
+    class LocalErrorBoundary extends React.Component {
+      state = {error: null};
+      static getDerivedStateFromError(error) {
+        Scheduler.unstable_yieldValue(
+          `ErrorBoundary static getDerivedStateFromError`,
+        );
+        return {error};
+      }
+      render() {
+        const {children, id, fallbackID} = this.props;
+        const {error} = this.state;
+        if (error) {
+          Scheduler.unstable_yieldValue(`${id} render error`);
+          return <Component id={fallbackID} />;
+        }
+        Scheduler.unstable_yieldValue(`${id} render success`);
+        return children || null;
+      }
+    }
+
+    class Component extends React.Component {
+      render() {
+        const {id} = this.props;
+        Scheduler.unstable_yieldValue('Component render ' + id);
+        return id;
+      }
+    }
+
+    class LocalBrokenComponentWillUnmount extends React.Component {
+      componentWillUnmount() {
+        Scheduler.unstable_yieldValue(
+          'BrokenComponentWillUnmount componentWillUnmount',
+        );
+        throw Error('Expected');
+      }
+
+      render() {
+        Scheduler.unstable_yieldValue('BrokenComponentWillUnmount render');
+        return 'broken';
+      }
+    }
+
+    const container = document.createElement('div');
+
+    ReactDOM.render(
+      <LocalErrorBoundary id="OuterBoundary" fallbackID="OuterFallback">
+        <Component id="sibling" />
+        <LocalErrorBoundary id="InnerBoundary" fallbackID="InnerFallback">
+          <LocalBrokenComponentWillUnmount />
+        </LocalErrorBoundary>
+      </LocalErrorBoundary>,
+      container,
+    );
+
+    expect(container.firstChild.textContent).toBe('sibling');
+    expect(container.lastChild.textContent).toBe('broken');
+    expect(Scheduler).toHaveYielded([
+      'OuterBoundary render success',
+      'Component render sibling',
+      'InnerBoundary render success',
+      'BrokenComponentWillUnmount render',
+    ]);
+
+    ReactDOM.render(
+      <LocalErrorBoundary id="OuterBoundary" fallbackID="OuterFallback">
+        <Component id="sibling" />
+      </LocalErrorBoundary>,
+      container,
+    );
+
+    // React should skip over the unmounting boundary and find the nearest still-mounted boundary.
+    expect(container.firstChild.textContent).toBe('OuterFallback');
+    expect(container.lastChild.textContent).toBe('OuterFallback');
+    expect(Scheduler).toHaveYielded([
+      'OuterBoundary render success',
+      'Component render sibling',
+      'BrokenComponentWillUnmount componentWillUnmount',
+      'ErrorBoundary static getDerivedStateFromError',
+      'OuterBoundary render error',
+      'Component render OuterFallback',
+    ]);
+  });
+
+  // @gate skipUnmountedBoundaries
+  it('catches errors thrown while detaching refs', () => {
+    class LocalErrorBoundary extends React.Component {
+      state = {error: null};
+      static getDerivedStateFromError(error) {
+        Scheduler.unstable_yieldValue(
+          `ErrorBoundary static getDerivedStateFromError`,
+        );
+        return {error};
+      }
+      render() {
+        const {children, id, fallbackID} = this.props;
+        const {error} = this.state;
+        if (error) {
+          Scheduler.unstable_yieldValue(`${id} render error`);
+          return <Component id={fallbackID} />;
+        }
+        Scheduler.unstable_yieldValue(`${id} render success`);
+        return children || null;
+      }
+    }
+
+    class Component extends React.Component {
+      render() {
+        const {id} = this.props;
+        Scheduler.unstable_yieldValue('Component render ' + id);
+        return id;
+      }
+    }
+
+    class LocalBrokenCallbackRef extends React.Component {
+      _ref = ref => {
+        Scheduler.unstable_yieldValue('LocalBrokenCallbackRef ref ' + !!ref);
+        if (ref === null) {
+          throw Error('Expected');
+        }
+      };
+
+      render() {
+        Scheduler.unstable_yieldValue('LocalBrokenCallbackRef render');
+        return <div ref={this._ref}>ref</div>;
+      }
+    }
+
+    const container = document.createElement('div');
+
+    ReactDOM.render(
+      <LocalErrorBoundary id="OuterBoundary" fallbackID="OuterFallback">
+        <Component id="sibling" />
+        <LocalErrorBoundary id="InnerBoundary" fallbackID="InnerFallback">
+          <LocalBrokenCallbackRef />
+        </LocalErrorBoundary>
+      </LocalErrorBoundary>,
+      container,
+    );
+
+    expect(container.firstChild.textContent).toBe('sibling');
+    expect(container.lastChild.textContent).toBe('ref');
+    expect(Scheduler).toHaveYielded([
+      'OuterBoundary render success',
+      'Component render sibling',
+      'InnerBoundary render success',
+      'LocalBrokenCallbackRef render',
+      'LocalBrokenCallbackRef ref true',
+    ]);
+
+    ReactDOM.render(
+      <LocalErrorBoundary id="OuterBoundary" fallbackID="OuterFallback">
+        <Component id="sibling" />
+      </LocalErrorBoundary>,
+      container,
+    );
+
+    // React should skip over the unmounting boundary and find the nearest still-mounted boundary.
+    expect(container.firstChild.textContent).toBe('OuterFallback');
+    expect(container.lastChild.textContent).toBe('OuterFallback');
+    expect(Scheduler).toHaveYielded([
+      'OuterBoundary render success',
+      'Component render sibling',
+      'LocalBrokenCallbackRef ref false',
+      'ErrorBoundary static getDerivedStateFromError',
+      'OuterBoundary render error',
+      'Component render OuterFallback',
+    ]);
+  });
 });