From 31518135c25aaa1b5c2799d2a18b6b9e9178409c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 21 Mar 2019 14:52:51 +0000 Subject: [PATCH] Strengthen nested update counter test coverage (#15166) * Isolate ReactUpdates-test cases This ensures their behavior is consistent when run in isolation, and that they actually test the cases they're describing. * Add coverage for cases where we reset nestedUpdateCounter These cases explicitly verify that we reset the counter in right places. * Add a mutually recursive test case * Add test coverage for useLayoutEffect loop --- .../src/__tests__/ReactUpdates-test.js | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js index a6b788902b1b..533ce0743a9a 100644 --- a/packages/react-dom/src/__tests__/ReactUpdates-test.js +++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js @@ -15,6 +15,7 @@ let ReactTestUtils; describe('ReactUpdates', () => { beforeEach(() => { + jest.resetModules(); React = require('react'); ReactDOM = require('react-dom'); ReactTestUtils = require('react-dom/test-utils'); @@ -1311,6 +1312,46 @@ describe('ReactUpdates', () => { ReactDOM.render(, container); }); + it('resets the update counter for unrelated updates', () => { + const container = document.createElement('div'); + const ref = React.createRef(); + + class EventuallyTerminating extends React.Component { + state = {step: 0}; + componentDidMount() { + this.setState({step: 1}); + } + componentDidUpdate() { + if (this.state.step < limit) { + this.setState({step: this.state.step + 1}); + } + } + render() { + return this.state.step; + } + } + + let limit = 55; + expect(() => { + ReactDOM.render(, container); + }).toThrow('Maximum'); + + // Verify that we don't go over the limit if these updates are unrelated. + limit -= 10; + ReactDOM.render(, container); + expect(container.textContent).toBe(limit.toString()); + ref.current.setState({step: 0}); + expect(container.textContent).toBe(limit.toString()); + ref.current.setState({step: 0}); + expect(container.textContent).toBe(limit.toString()); + + limit += 10; + expect(() => { + ref.current.setState({step: 0}); + }).toThrow('Maximum'); + expect(ref.current).toBe(null); + }); + it('does not fall into an infinite update loop', () => { class NonTerminating extends React.Component { state = {step: 0}; @@ -1336,6 +1377,88 @@ describe('ReactUpdates', () => { }).toThrow('Maximum'); }); + it('does not fall into an infinite update loop with useLayoutEffect', () => { + function NonTerminating() { + const [step, setStep] = React.useState(0); + React.useLayoutEffect(() => { + setStep(x => x + 1); + }); + return step; + } + + const container = document.createElement('div'); + expect(() => { + ReactDOM.render(, container); + }).toThrow('Maximum'); + }); + + it('can recover after falling into an infinite update loop', () => { + class NonTerminating extends React.Component { + state = {step: 0}; + componentDidMount() { + this.setState({step: 1}); + } + componentDidUpdate() { + this.setState({step: 2}); + } + render() { + return this.state.step; + } + } + + class Terminating extends React.Component { + state = {step: 0}; + componentDidMount() { + this.setState({step: 1}); + } + render() { + return this.state.step; + } + } + + const container = document.createElement('div'); + expect(() => { + ReactDOM.render(, container); + }).toThrow('Maximum'); + + ReactDOM.render(, container); + expect(container.textContent).toBe('1'); + + expect(() => { + ReactDOM.render(, container); + }).toThrow('Maximum'); + + ReactDOM.render(, container); + expect(container.textContent).toBe('1'); + }); + + it('does not fall into mutually recursive infinite update loop with same container', () => { + // Note: this test would fail if there were two or more different roots. + + class A extends React.Component { + componentDidMount() { + ReactDOM.render(, container); + } + render() { + return null; + } + } + + class B extends React.Component { + componentDidMount() { + ReactDOM.render(, container); + } + render() { + return null; + } + } + + const container = document.createElement('div'); + expect(() => { + ReactDOM.render(, container); + }).toThrow('Maximum'); + }); + it('does not fall into an infinite error loop', () => { function BadRender() { throw new Error('error');