diff --git a/README.md b/README.md index 066ad5ad..6ecd26f6 100644 --- a/README.md +++ b/README.md @@ -228,7 +228,8 @@ This somewhat helpful and descriptive message is supposed to help you identify potential problems implementing `observers` early on. If you miss the exception for some reason and ends up in production (prone to happen with dynamic children), this component will NOT unmount. Instead, it will gracefully catch -the error so that you can do custom logging and report it. For example: +the error and re-render the children so that you can do custom logging and +report it. For example: ```js import { Config } from '@researchgate/react-intersection-observer'; @@ -252,11 +253,16 @@ Config.errorReporter(function(error) { }); ``` -If this error happens during mount, it's easy to spot. However, a lot of these -errors usually happen during tree updates, because some child component that was -previously observed suddently ceaces to exist in the UI. This usually means that -either you shouldn't have rendered an `` around it anymore or, you -should have used the `disabled` property. +While sometimes this error happens during mount, and it's easy to spot, often +types of errors happen during tree updates, because some child component that +was previously observed suddently ceaces to exist in the UI. This usually means +that either you shouldn't have rendered an `` around it anymore or, +you should have used the `disabled` property. That's why we capture errors and +do re-rendering of the children as a fallback. + +If another kind of error happens, the `errorReporter` won't be invoked, and by +rendering the children the error will bubble up to the nearest error boundary +you defined. At [ResearchGate](www.researchgate.net), we have found that not unmounting the tree just because we failed to `observe()` a DOM node suits our use cases diff --git a/src/IntersectionObserver.js b/src/IntersectionObserver.js index a33008af..7da80470 100644 --- a/src/IntersectionObserver.js +++ b/src/IntersectionObserver.js @@ -8,6 +8,9 @@ import Config from './config'; const observerOptions = ['root', 'rootMargin', 'threshold']; const observableProps = ['root', 'rootMargin', 'threshold', 'disabled']; const { hasOwnProperty, toString } = Object.prototype; +const missingNodeError = new Error( + "ReactIntersectionObserver: Can't find DOM node in the provided children. Make sure to render at least one DOM node in the tree." +); const getOptions = (props) => { return observerOptions.reduce((options, key) => { @@ -110,9 +113,7 @@ class IntersectionObserver extends React.Component { return false; } if (!this.targetNode) { - throw new Error( - "ReactIntersectionObserver: Can't find DOM node in the provided children. Make sure to render at least one DOM node in the tree." - ); + throw missingNodeError; } this.observer = createObserver(getOptions(this.props)); this.target = this.targetNode; @@ -190,15 +191,27 @@ class ErrorBoundary extends React.Component { forwardedRef: PropTypes.oneOfType([PropTypes.func, PropTypes.object]), }; + static getDerivedStateFromError() { + return { hasError: true }; + } + + state = { + hasError: false, + }; + componentDidCatch(error, info) { - if (Config.errorReporter) { - Config.errorReporter(error, info); + if (error === missingNodeError) { + Config.errorReporter && Config.errorReporter(error, info); } } render() { const { forwardedRef, ...props } = this.props; + if (this.state.hasError) { + return props.children; + } + return ; } } diff --git a/src/__tests__/IntersectionObserver.spec.js b/src/__tests__/IntersectionObserver.spec.js index 87665f01..b4d4e5d8 100644 --- a/src/__tests__/IntersectionObserver.spec.js +++ b/src/__tests__/IntersectionObserver.spec.js @@ -141,6 +141,45 @@ test('reports errors by re-throwing trying observer children without a DOM node' Config.errorReporter = originalErrorReporter; }); +test('render a fallback when some unexpected error happens', () => { + global.spyOn(console, 'error'); // suppress error boundary warning + const originalErrorReporter = Config.errorReporter; + const spy = jest.fn(); + Config.errorReporter = spy; + class TestErrorBoundary extends React.Component { + state = { hasError: false }; + + componentDidCatch() { + this.setState({ hasError: true }); + } + + render() { + // eslint-disable-next-line react/prop-types + return this.state.hasError ? 'has-error' : this.props.children; + } + } + + const Boom = () => { + throw new Error('unexpected rendering error'); + }; + + const children = renderer + .create( + + + + + + ) + .toJSON(); + + // Tree changed because of the custom error boundary + expect(children).toBe('has-error'); + expect(spy).not.toBeCalled(); + + Config.errorReporter = originalErrorReporter; +}); + test('error boundary forwards ref', () => { let observer; renderer.create(