From f258d7db853dc42d1074d338ea4ae153b369e17c Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Thu, 5 Jul 2018 00:23:07 -0700 Subject: [PATCH 1/7] Add gSBU prevState test --- test/browser/lifecycle.js | 77 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/test/browser/lifecycle.js b/test/browser/lifecycle.js index b4d2a05aef..ffde88ead6 100644 --- a/test/browser/lifecycle.js +++ b/test/browser/lifecycle.js @@ -531,8 +531,76 @@ describe('Lifecycle methods', () => { 'componentDidUpdate' ]); }); + + it('should be passed the previous props and state', () => { + let previousProps = []; + let previousStates = []; + + let currentProps = []; + let currentStates = []; + + class Foo extends Component { + constructor(props) { + super(props); + this.state = { + value: 0 + }; + } + static getDerivedStateFromProps(props, state) { + return { + value: state.value + 1, + }; + } + getSnapshotBeforeUpdate(prevProps, prevState) { + previousProps.push(prevProps); + previousStates.push(prevState); + + currentProps.push(this.props); + currentStates.push(this.state); + } + componentDidMount() { + this.setState({ + value: this.state.value + 1 + }); + } + render() { + return
{this.state.value}
+ } + } + + let element = render(, scratch); + expect(element.textContent).to.be.equal('1'); + expect(previousStates).to.have.length(0); + expect(currentStates).to.have.length(0); + + element = render(, scratch, scratch.firstChild); + expect(element.textContent).to.be.equal('3'); + + expect(previousProps).to.deep.equal([{ + foo: "foo", + children: [] + }]); + + // prevState in getSnapshotBeforeUpdate should be + // the state before getDerivedStateFromProps is called... + expect(previousStates).to.deep.equal([{ + value: 1 + }]); + + // ...and this.state in getSnapshotBeforeUpdate should be + // the updated state after getDerivedStateFromProps is called... + expect(currentStates).to.deep.equal([{ + value: 3 + }]); + + expect(currentProps).to.deep.equal([{ + foo: "bar", + children: [] + }]); + }); }); + // TODO - look at describe('#componentWillUpdate', () => { it('should NOT be called on initial render', () => { class ReceivePropsComponent extends Component { @@ -636,6 +704,7 @@ describe('Lifecycle methods', () => { }); }); + // TODO - look at describe('#componentWillReceiveProps', () => { it('should NOT be called on initial render', () => { class ReceivePropsComponent extends Component { @@ -732,6 +801,13 @@ describe('Lifecycle methods', () => { }); }); + // TODO - Look at + describe('#componentDidUpdate', () => { + it('should be passed previous state', () => { + // + }); + }); + describe('top-level componentWillUnmount', () => { it('should invoke componentWillUnmount for top-level components', () => { @@ -922,6 +998,7 @@ describe('Lifecycle methods', () => { }); + // TODO - look at describe('shouldComponentUpdate', () => { let setState; From fc3e4af078808fa0dae67ac22ed9cee7eaf94a5d Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Fri, 6 Jul 2018 08:52:51 -0700 Subject: [PATCH 2/7] Add prevState, etc. test for componentDidUpdate --- test/browser/lifecycle.js | 68 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/test/browser/lifecycle.js b/test/browser/lifecycle.js index ffde88ead6..44ac6fa748 100644 --- a/test/browser/lifecycle.js +++ b/test/browser/lifecycle.js @@ -801,10 +801,72 @@ describe('Lifecycle methods', () => { }); }); - // TODO - Look at describe('#componentDidUpdate', () => { - it('should be passed previous state', () => { - // + it('should be passed previous props and state', () => { + let previousProps = []; + let previousStates = []; + + let currentProps = []; + let currentStates = []; + + class Foo extends Component { + constructor(props) { + super(props); + this.state = { + value: 0 + }; + } + static getDerivedStateFromProps(props, state) { + return { + value: state.value + 1, + }; + } + componentDidUpdate(prevProps, prevState) { + previousProps.push(prevProps); + previousStates.push(prevState); + + currentProps.push(this.props); + currentStates.push(this.state); + } + componentDidMount() { + this.setState({ + value: this.state.value + 1 + }); + } + render() { + return
{this.state.value}
+ } + } + + let element = render(, scratch); + expect(element.textContent).to.be.equal('1'); + expect(previousStates).to.have.length(0); + expect(currentStates).to.have.length(0); + + element = render(, scratch, scratch.firstChild); + expect(element.textContent).to.be.equal('3'); + + expect(previousProps).to.deep.equal([{ + foo: "foo", + children: [] + }]); + + // prevState in componentDidUpdate should be + // the state before getDerivedStateFromProps is called... + expect(previousStates).to.deep.equal([{ + value: 1 + }]); + + // ...and this.state in componentDidUpdate should be + // the updated state after getDerivedStateFromProps is called... + expect(currentStates).to.deep.equal([{ + value: 3 + }]); + + expect(currentProps).to.deep.equal([{ + foo: "bar", + children: [] + }]); }); }); From 4c5b361d8f41384afaf5e933455c79ee28854ced Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Fri, 6 Jul 2018 08:53:59 -0700 Subject: [PATCH 3/7] Add nextState, etc. test for shouldComponentUpdate --- test/browser/lifecycle.js | 70 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/test/browser/lifecycle.js b/test/browser/lifecycle.js index 44ac6fa748..980a97bf59 100644 --- a/test/browser/lifecycle.js +++ b/test/browser/lifecycle.js @@ -1060,7 +1060,6 @@ describe('Lifecycle methods', () => { }); - // TODO - look at describe('shouldComponentUpdate', () => { let setState; @@ -1102,6 +1101,75 @@ describe('Lifecycle methods', () => { expect(ShouldNot.prototype.shouldComponentUpdate).to.have.been.calledOnce; expect(ShouldNot.prototype.render).to.have.been.calledOnce; }); + + it('should be passed next props and state', () => { + let currentPropsLog = []; + let currentStatesLog = []; + + let nextPropsLog = []; + let nextStatesLog = []; + + class Foo extends Component { + constructor(props) { + super(props); + this.state = { + value: 0 + }; + } + static getDerivedStateFromProps(props, state) { + return { + value: state.value + 1, + }; + } + shouldComponentUpdate(nextProps, nextState) { + nextPropsLog.push(nextProps); + nextStatesLog.push(nextState); + + currentPropsLog.push(this.props); + currentStatesLog.push(this.state); + + return true; + } + componentDidMount() { + this.setState({ + value: this.state.value + 1 + }); + } + render() { + return
{this.state.value}
+ } + } + + let element = render(, scratch); + expect(element.textContent).to.be.equal('1'); + expect(nextStatesLog).to.have.length(0); + expect(currentStatesLog).to.have.length(0); + + element = render(, scratch, scratch.firstChild); + expect(element.textContent).to.be.equal('3'); + + expect(currentPropsLog).to.deep.equal([{ + foo: "foo", + children: [] + }]); + + // this.state in shouldComponentUpdate should be + // the state before getDerivedStateFromProps is called... + expect(currentStatesLog).to.deep.equal([{ + value: 1 + }]); + + // ...and nextState in shouldComponentUpdate should be + // the updated state after getDerivedStateFromProps is called... + expect(nextStatesLog).to.deep.equal([{ + value: 3 + }]); + + expect(nextPropsLog).to.deep.equal([{ + foo: "bar", + children: [] + }]); + }); }); From 3ab0f4d6c3e6d6bfc58d003d88a8fa9758bfd0b9 Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Fri, 6 Jul 2018 09:16:04 -0700 Subject: [PATCH 4/7] Add gDSFP params test --- test/browser/lifecycle.js | 56 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/test/browser/lifecycle.js b/test/browser/lifecycle.js index 980a97bf59..592f62bfd5 100644 --- a/test/browser/lifecycle.js +++ b/test/browser/lifecycle.js @@ -433,6 +433,59 @@ describe('Lifecycle methods', () => { // TODO: Investigate this test: // [should not override state with stale values if prevState is spread within getDerivedStateFromProps](https://github.com/facebook/react/blob/25dda90c1ecb0c662ab06e2c80c1ee31e0ae9d36/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js#L1035) + + it('should be passed next props and state', () => { + let nextPropsLog = []; + let nextStatesLog = []; + + class Foo extends Component { + constructor(props) { + super(props); + this.state = { + value: 0 + }; + } + static getDerivedStateFromProps(props, state) { + nextPropsLog.push({...props}); + nextStatesLog.push({...state}); + + return { + value: state.value + 1, + }; + } + componentDidMount() { + this.setState({ + value: this.state.value + 1 + }); + } + render() { + return
{this.state.value}
+ } + } + + let element = render(, scratch); + expect(element.textContent).to.be.equal('1'); + expect(nextStatesLog).to.have.length(1); + + element = render(, scratch, scratch.firstChild); + expect(element.textContent).to.be.equal('3'); + + // ...and nextState in shouldComponentUpdate should be + // the updated state after getDerivedStateFromProps is called... + expect(nextStatesLog).to.deep.equal([{ + value: 0 + },{ + value: 2 + }]); + + expect(nextPropsLog).to.deep.equal([{ + foo: "foo", + children: [] + },{ + foo: "bar", + children: [] + }]); + }); }); describe("#getSnapshotBeforeUpdate", () => { @@ -600,7 +653,7 @@ describe('Lifecycle methods', () => { }); }); - // TODO - look at + // TODO - add test for parameters describe('#componentWillUpdate', () => { it('should NOT be called on initial render', () => { class ReceivePropsComponent extends Component { @@ -704,7 +757,6 @@ describe('Lifecycle methods', () => { }); }); - // TODO - look at describe('#componentWillReceiveProps', () => { it('should NOT be called on initial render', () => { class ReceivePropsComponent extends Component { From 899aa6ef4324c70b0104bb74e8b443bf81177bb8 Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Fri, 6 Jul 2018 15:03:46 -0700 Subject: [PATCH 5/7] Separate testing new props and new state --- test/browser/lifecycle.js | 396 +++++++++++++++++++++++++------------- 1 file changed, 267 insertions(+), 129 deletions(-) diff --git a/test/browser/lifecycle.js b/test/browser/lifecycle.js index 592f62bfd5..6af82a4064 100644 --- a/test/browser/lifecycle.js +++ b/test/browser/lifecycle.js @@ -435,8 +435,11 @@ describe('Lifecycle methods', () => { // [should not override state with stale values if prevState is spread within getDerivedStateFromProps](https://github.com/facebook/react/blob/25dda90c1ecb0c662ab06e2c80c1ee31e0ae9d36/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js#L1035) it('should be passed next props and state', () => { - let nextPropsLog = []; - let nextStatesLog = []; + /** @type {() => void} */ + let updateState; + + let propLog = []; + let stateLog = []; class Foo extends Component { constructor(props) { @@ -444,44 +447,74 @@ describe('Lifecycle methods', () => { this.state = { value: 0 }; + updateState = () => this.setState({ + value: this.state.value + 1 + }); } static getDerivedStateFromProps(props, state) { - nextPropsLog.push({...props}); - nextStatesLog.push({...state}); + propLog.push({...props}); + stateLog.push({...state}); + // NOTE: Don't do this in real production code! + // https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html return { - value: state.value + 1, + value: state.value + 1 }; } - componentDidMount() { - this.setState({ - value: this.state.value + 1 - }); - } render() { - return
{this.state.value}
+ return
{this.state.value}
; } } + // Initial render + // state.value: initialized to 0 in constructor, 0 -> 1 in gDSFP let element = render(, scratch); expect(element.textContent).to.be.equal('1'); - expect(nextStatesLog).to.have.length(1); - - element = render(, scratch, scratch.firstChild); - expect(element.textContent).to.be.equal('3'); + expect(stateLog).to.have.length(1); + expect(stateLog).to.deep.equal([{ + value: 0 + }]); + expect(propLog).to.deep.equal([{ + foo: "foo", + children: [] + }]); - // ...and nextState in shouldComponentUpdate should be - // the updated state after getDerivedStateFromProps is called... - expect(nextStatesLog).to.deep.equal([{ + // New Props + // state.value: 1 -> 2 in gDSFP + render(, scratch, scratch.firstChild); + expect(element.textContent).to.be.equal('2'); + expect(stateLog).to.deep.equal([{ value: 0 - },{ - value: 2 + }, { + value: 1 + }]); + expect(propLog).to.deep.equal([{ + foo: "foo", + children: [] + }, { + foo: "bar", + children: [] }]); - expect(nextPropsLog).to.deep.equal([{ + // New state + // state.value: 2 -> 3 in updateState, 3 -> 4 in gDSFP + updateState(); + rerender(); + expect(element.textContent).to.be.equal('4'); + expect(stateLog).to.deep.equal([{ + value: 0 + }, { + value: 1 + }, { + value: 3 + }]); + expect(propLog).to.deep.equal([{ foo: "foo", children: [] - },{ + }, { + foo: "bar", + children: [] + }, { foo: "bar", children: [] }]); @@ -586,11 +619,14 @@ describe('Lifecycle methods', () => { }); it('should be passed the previous props and state', () => { - let previousProps = []; - let previousStates = []; + /** @type {() => void} */ + let updateState; + + let prevPropsLog = []; + let prevStateLog = []; - let currentProps = []; - let currentStates = []; + let curPropsLog = []; + let curStateLog = []; class Foo extends Component { constructor(props) { @@ -598,58 +634,90 @@ describe('Lifecycle methods', () => { this.state = { value: 0 }; + updateState = () => this.setState({ + value: this.state.value + 1 + }); } static getDerivedStateFromProps(props, state) { + // NOTE: Don't do this in real production code! + // https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html return { - value: state.value + 1, + value: state.value + 1 }; } getSnapshotBeforeUpdate(prevProps, prevState) { - previousProps.push(prevProps); - previousStates.push(prevState); + prevPropsLog.push({...prevProps}); + prevStateLog.push({...prevState}); - currentProps.push(this.props); - currentStates.push(this.state); - } - componentDidMount() { - this.setState({ - value: this.state.value + 1 - }); + curPropsLog.push({...this.props}); + curStateLog.push({...this.state}); } render() { - return
{this.state.value}
+ return
{this.state.value}
; } } - let element = render(, scratch); - expect(element.textContent).to.be.equal('1'); - expect(previousStates).to.have.length(0); - expect(currentStates).to.have.length(0); - - element = render(, scratch, scratch.firstChild); - expect(element.textContent).to.be.equal('3'); + // Expectation: + // `prevState` in getSnapshotBeforeUpdate should be + // the state before setState or getDerivedStateFromProps was called. + // `this.state` in getSnapshotBeforeUpdate should be + // the updated state after getDerivedStateFromProps was called. - expect(previousProps).to.deep.equal([{ + const finalPrevPropsLogs = [{ foo: "foo", children: [] - }]); + }, { + foo: "bar", + children: [] + }]; - // prevState in getSnapshotBeforeUpdate should be - // the state before getDerivedStateFromProps is called... - expect(previousStates).to.deep.equal([{ + const finalPrevStateLogs = [{ value: 1 - }]); - - // ...and this.state in getSnapshotBeforeUpdate should be - // the updated state after getDerivedStateFromProps is called... - expect(currentStates).to.deep.equal([{ - value: 3 - }]); + }, { + value: 2 + }]; - expect(currentProps).to.deep.equal([{ + const finalCurPropsLogs = [{ foo: "bar", children: [] - }]); + }, { + foo: "bar", + children: [] + }]; + + const finalCurStateLogs = [{ + value: 2 + }, { + value: 4 + }]; + + // Initial render + // state.value: initialized to 0 in constructor, 0 -> 1 in gDSFP + let element = render(, scratch); + expect(element.textContent).to.be.equal('1'); + expect(prevPropsLog).to.have.length(0); + expect(prevStateLog).to.have.length(0); + expect(curPropsLog).to.have.length(0); + expect(curStateLog).to.have.length(0); + + // New props + // state.value: 1 -> 2 in gDSFP + element = render(, scratch, scratch.firstChild); + expect(element.textContent).to.be.equal('2'); + expect(prevPropsLog).to.deep.equal(finalPrevPropsLogs.slice(0, 1)); + expect(prevStateLog).to.deep.equal(finalPrevStateLogs.slice(0, 1)); + expect(curPropsLog).to.deep.equal(finalCurPropsLogs.slice(0, 1)); + expect(curStateLog).to.deep.equal(finalCurStateLogs.slice(0, 1)); + + // New state + // state.value: 2 -> 3 in updateState, 3 -> 4 in gDSFP + updateState(); + rerender(); + expect(element.textContent).to.be.equal('4'); + expect(prevPropsLog).to.deep.equal(finalPrevPropsLogs); + expect(prevStateLog).to.deep.equal(finalPrevStateLogs); + expect(curPropsLog).to.deep.equal(finalCurPropsLogs); + expect(curStateLog).to.deep.equal(finalCurStateLogs); }); }); @@ -855,11 +923,14 @@ describe('Lifecycle methods', () => { describe('#componentDidUpdate', () => { it('should be passed previous props and state', () => { - let previousProps = []; - let previousStates = []; + /** @type {() => void} */ + let updateState; + + let prevPropsLog = []; + let prevStateLog = []; - let currentProps = []; - let currentStates = []; + let curPropsLog = []; + let curStateLog = []; class Foo extends Component { constructor(props) { @@ -867,58 +938,90 @@ describe('Lifecycle methods', () => { this.state = { value: 0 }; + updateState = () => this.setState({ + value: this.state.value + 1 + }); } static getDerivedStateFromProps(props, state) { + // NOTE: Don't do this in real production code! + // https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html return { - value: state.value + 1, + value: state.value + 1 }; } componentDidUpdate(prevProps, prevState) { - previousProps.push(prevProps); - previousStates.push(prevState); + prevPropsLog.push({...prevProps}); + prevStateLog.push(prevState); - currentProps.push(this.props); - currentStates.push(this.state); - } - componentDidMount() { - this.setState({ - value: this.state.value + 1 - }); + curPropsLog.push({...this.props}); + curStateLog.push({...this.state}); } render() { - return
{this.state.value}
+ return
{this.state.value}
; } } - let element = render(, scratch); - expect(element.textContent).to.be.equal('1'); - expect(previousStates).to.have.length(0); - expect(currentStates).to.have.length(0); - - element = render(, scratch, scratch.firstChild); - expect(element.textContent).to.be.equal('3'); + // Expectation: + // `prevState` in componentDidUpdate should be + // the state before setState and getDerivedStateFromProps was called. + // `this.state` in componentDidUpdate should be + // the updated state after getDerivedStateFromProps was called. - expect(previousProps).to.deep.equal([{ + const finalPrevPropsLogs = [{ foo: "foo", children: [] - }]); + }, { + foo: "bar", + children: [] + }]; - // prevState in componentDidUpdate should be - // the state before getDerivedStateFromProps is called... - expect(previousStates).to.deep.equal([{ + const finalPrevStateLogs = [{ value: 1 - }]); - - // ...and this.state in componentDidUpdate should be - // the updated state after getDerivedStateFromProps is called... - expect(currentStates).to.deep.equal([{ - value: 3 - }]); + }, { + value: 2 + }]; - expect(currentProps).to.deep.equal([{ + const finalCurPropsLogs = [{ foo: "bar", children: [] - }]); + }, { + foo: "bar", + children: [] + }]; + + const finalCurStateLogs = [{ + value: 2 + }, { + value: 4 + }]; + + // Initial render + // state.value: initialized to 0 in constructor, 0 -> 1 in gDSFP + let element = render(, scratch); + expect(element.textContent).to.be.equal('1'); + expect(prevPropsLog).to.have.length(0); + expect(prevStateLog).to.have.length(0); + expect(curPropsLog).to.have.length(0); + expect(curStateLog).to.have.length(0); + + // New props + // state.value: 1 -> 2 in gDSFP + element = render(, scratch, scratch.firstChild); + expect(element.textContent).to.be.equal('2'); + expect(prevPropsLog).to.deep.equal(finalPrevPropsLogs.slice(0, 1)); + expect(prevStateLog).to.deep.equal(finalPrevStateLogs.slice(0, 1)); + expect(curPropsLog).to.deep.equal(finalCurPropsLogs.slice(0, 1)); + expect(curStateLog).to.deep.equal(finalCurStateLogs.slice(0, 1)); + + // New state + // state.value: 2 -> 3 in updateState, 3 -> 4 in gDSFP + updateState(); + rerender(); + expect(element.textContent).to.be.equal('4'); + expect(prevPropsLog).to.deep.equal(finalPrevPropsLogs); + expect(prevStateLog).to.deep.equal(finalPrevStateLogs); + expect(curPropsLog).to.deep.equal(finalCurPropsLogs); + expect(curStateLog).to.deep.equal(finalCurStateLogs); }); }); @@ -1155,11 +1258,14 @@ describe('Lifecycle methods', () => { }); it('should be passed next props and state', () => { - let currentPropsLog = []; - let currentStatesLog = []; + /** @type {() => void} */ + let updateState; + + let curPropsLog = []; + let curStateLog = []; let nextPropsLog = []; - let nextStatesLog = []; + let nextStateLog = []; class Foo extends Component { constructor(props) { @@ -1167,60 +1273,92 @@ describe('Lifecycle methods', () => { this.state = { value: 0 }; + updateState = () => this.setState({ + value: this.state.value + 1 + }); } static getDerivedStateFromProps(props, state) { + // NOTE: Don't do this in real production code! + // https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html return { - value: state.value + 1, + value: state.value + 1 }; } shouldComponentUpdate(nextProps, nextState) { - nextPropsLog.push(nextProps); - nextStatesLog.push(nextState); + nextPropsLog.push({...nextProps}); + nextStateLog.push({...nextState}); - currentPropsLog.push(this.props); - currentStatesLog.push(this.state); + curPropsLog.push({...this.props}); + curStateLog.push({...this.state}); return true; } - componentDidMount() { - this.setState({ - value: this.state.value + 1 - }); - } render() { - return
{this.state.value}
+ return
{this.state.value}
; } } - let element = render(, scratch); - expect(element.textContent).to.be.equal('1'); - expect(nextStatesLog).to.have.length(0); - expect(currentStatesLog).to.have.length(0); + // Expectation: + // `this.state` in shouldComponentUpdate should be + // the state before setState or getDerivedStateFromProps was called + // `nextState` in shouldComponentUpdate should be + // the updated state after getDerivedStateFromProps was called - element = render(, scratch, scratch.firstChild); - expect(element.textContent).to.be.equal('3'); - - expect(currentPropsLog).to.deep.equal([{ + const finalCurPropsLogs = [{ foo: "foo", children: [] - }]); + }, { + foo: "bar", + children: [] + }]; - // this.state in shouldComponentUpdate should be - // the state before getDerivedStateFromProps is called... - expect(currentStatesLog).to.deep.equal([{ + const finalCurStateLogs = [{ value: 1 - }]); - - // ...and nextState in shouldComponentUpdate should be - // the updated state after getDerivedStateFromProps is called... - expect(nextStatesLog).to.deep.equal([{ - value: 3 - }]); + }, { + value: 2 + }]; - expect(nextPropsLog).to.deep.equal([{ + const finalNextPropsLogs = [{ foo: "bar", children: [] - }]); + }, { + foo: "bar", + children: [] + }]; + + const finalNextStateLogs = [{ + value: 2 + }, { + value: 4 + }]; + + // Initial render + // state.value: initialized to 0 in constructor, 0 -> 1 in gDSFP + let element = render(, scratch); + expect(element.textContent).to.be.equal('1'); + expect(curPropsLog).to.have.length(0); + expect(curStateLog).to.have.length(0); + expect(nextPropsLog).to.have.length(0); + expect(nextStateLog).to.have.length(0); + + // New props + // state.value: 1 -> 2 in gDSFP + element = render(, scratch, scratch.firstChild); + expect(element.textContent).to.be.equal('2'); + expect(curPropsLog).to.deep.equal(finalCurPropsLogs.slice(0, 1)); + expect(curStateLog).to.deep.equal(finalCurStateLogs.slice(0, 1)); + expect(nextPropsLog).to.deep.equal(finalNextPropsLogs.slice(0, 1)); + expect(nextStateLog).to.deep.equal(finalNextStateLogs.slice(0, 1)); + + // New state + // state.value: 2 -> 3 in updateState, 3 -> 4 in gDSFP + updateState(); + rerender(); + expect(element.textContent).to.be.equal('4'); + expect(curPropsLog).to.deep.equal(finalCurPropsLogs); + expect(curStateLog).to.deep.equal(finalCurStateLogs); + expect(nextPropsLog).to.deep.equal(finalNextPropsLogs); + expect(nextStateLog).to.deep.equal(finalNextStateLogs); }); }); From b2582effdb989837050ad58099aa09e2553d7e9c Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Fri, 6 Jul 2018 15:42:05 -0700 Subject: [PATCH 6/7] Simplify tests by removing log concept --- test/browser/lifecycle.js | 327 +++++++++++++++++--------------------- 1 file changed, 145 insertions(+), 182 deletions(-) diff --git a/test/browser/lifecycle.js b/test/browser/lifecycle.js index 6af82a4064..cccfeacbc4 100644 --- a/test/browser/lifecycle.js +++ b/test/browser/lifecycle.js @@ -438,8 +438,8 @@ describe('Lifecycle methods', () => { /** @type {() => void} */ let updateState; - let propLog = []; - let stateLog = []; + let propsArg; + let stateArg; class Foo extends Component { constructor(props) { @@ -452,8 +452,10 @@ describe('Lifecycle methods', () => { }); } static getDerivedStateFromProps(props, state) { - propLog.push({...props}); - stateLog.push({...state}); + // These object references might be updated later so copy + // object so we can assert their values at this snapshot in time + propsArg = {...props}; + stateArg = {...state}; // NOTE: Don't do this in real production code! // https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html @@ -470,54 +472,38 @@ describe('Lifecycle methods', () => { // state.value: initialized to 0 in constructor, 0 -> 1 in gDSFP let element = render(, scratch); expect(element.textContent).to.be.equal('1'); - expect(stateLog).to.have.length(1); - expect(stateLog).to.deep.equal([{ - value: 0 - }]); - expect(propLog).to.deep.equal([{ + expect(propsArg).to.deep.equal({ foo: "foo", children: [] - }]); + }); + expect(stateArg).to.deep.equal({ + value: 0 + }); // New Props // state.value: 1 -> 2 in gDSFP render(, scratch, scratch.firstChild); expect(element.textContent).to.be.equal('2'); - expect(stateLog).to.deep.equal([{ - value: 0 - }, { - value: 1 - }]); - expect(propLog).to.deep.equal([{ - foo: "foo", - children: [] - }, { + expect(propsArg).to.deep.equal({ foo: "bar", children: [] - }]); + }); + expect(stateArg).to.deep.equal({ + value: 1 + }); // New state // state.value: 2 -> 3 in updateState, 3 -> 4 in gDSFP updateState(); rerender(); expect(element.textContent).to.be.equal('4'); - expect(stateLog).to.deep.equal([{ - value: 0 - }, { - value: 1 - }, { - value: 3 - }]); - expect(propLog).to.deep.equal([{ - foo: "foo", - children: [] - }, { + expect(propsArg).to.deep.equal({ foo: "bar", children: [] - }, { - foo: "bar", - children: [] - }]); + }); + expect(stateArg).to.deep.equal({ + value: 3 + }); }); }); @@ -622,11 +608,10 @@ describe('Lifecycle methods', () => { /** @type {() => void} */ let updateState; - let prevPropsLog = []; - let prevStateLog = []; - - let curPropsLog = []; - let curStateLog = []; + let prevPropsArg; + let prevStateArg; + let curProps; + let curState; class Foo extends Component { constructor(props) { @@ -646,11 +631,13 @@ describe('Lifecycle methods', () => { }; } getSnapshotBeforeUpdate(prevProps, prevState) { - prevPropsLog.push({...prevProps}); - prevStateLog.push({...prevState}); + // These object references might be updated later so copy + // object so we can assert their values at this snapshot in time + prevPropsArg = {...prevProps}; + prevStateArg = {...prevState}; - curPropsLog.push({...this.props}); - curStateLog.push({...this.state}); + curProps = {...this.props}; + curState = {...this.state}; } render() { return
{this.state.value}
; @@ -663,61 +650,53 @@ describe('Lifecycle methods', () => { // `this.state` in getSnapshotBeforeUpdate should be // the updated state after getDerivedStateFromProps was called. - const finalPrevPropsLogs = [{ - foo: "foo", - children: [] - }, { - foo: "bar", - children: [] - }]; - - const finalPrevStateLogs = [{ - value: 1 - }, { - value: 2 - }]; - - const finalCurPropsLogs = [{ - foo: "bar", - children: [] - }, { - foo: "bar", - children: [] - }]; - - const finalCurStateLogs = [{ - value: 2 - }, { - value: 4 - }]; - // Initial render // state.value: initialized to 0 in constructor, 0 -> 1 in gDSFP let element = render(, scratch); expect(element.textContent).to.be.equal('1'); - expect(prevPropsLog).to.have.length(0); - expect(prevStateLog).to.have.length(0); - expect(curPropsLog).to.have.length(0); - expect(curStateLog).to.have.length(0); + expect(prevPropsArg).to.be.undefined; + expect(prevStateArg).to.be.undefined; + expect(curProps).to.be.undefined; + expect(curState).to.be.undefined; // New props // state.value: 1 -> 2 in gDSFP element = render(, scratch, scratch.firstChild); expect(element.textContent).to.be.equal('2'); - expect(prevPropsLog).to.deep.equal(finalPrevPropsLogs.slice(0, 1)); - expect(prevStateLog).to.deep.equal(finalPrevStateLogs.slice(0, 1)); - expect(curPropsLog).to.deep.equal(finalCurPropsLogs.slice(0, 1)); - expect(curStateLog).to.deep.equal(finalCurStateLogs.slice(0, 1)); + expect(prevPropsArg).to.deep.equal({ + foo: "foo", + children: [] + }); + expect(prevStateArg).to.deep.equal({ + value: 1 + }); + expect(curProps).to.deep.equal({ + foo: "bar", + children: [] + }); + expect(curState).to.deep.equal({ + value: 2 + }); // New state // state.value: 2 -> 3 in updateState, 3 -> 4 in gDSFP updateState(); rerender(); expect(element.textContent).to.be.equal('4'); - expect(prevPropsLog).to.deep.equal(finalPrevPropsLogs); - expect(prevStateLog).to.deep.equal(finalPrevStateLogs); - expect(curPropsLog).to.deep.equal(finalCurPropsLogs); - expect(curStateLog).to.deep.equal(finalCurStateLogs); + expect(prevPropsArg).to.deep.equal({ + foo: "bar", + children: [] + }); + expect(prevStateArg).to.deep.equal({ + value: 2 + }); + expect(curProps).to.deep.equal({ + foo: "bar", + children: [] + }); + expect(curState).to.deep.equal({ + value: 4 + }); }); }); @@ -926,11 +905,10 @@ describe('Lifecycle methods', () => { /** @type {() => void} */ let updateState; - let prevPropsLog = []; - let prevStateLog = []; - - let curPropsLog = []; - let curStateLog = []; + let prevPropsArg; + let prevStateArg; + let curProps; + let curState; class Foo extends Component { constructor(props) { @@ -950,11 +928,13 @@ describe('Lifecycle methods', () => { }; } componentDidUpdate(prevProps, prevState) { - prevPropsLog.push({...prevProps}); - prevStateLog.push(prevState); + // These object references might be updated later so copy + // object so we can assert their values at this snapshot in time + prevPropsArg = {...prevProps}; + prevStateArg = {...prevState}; - curPropsLog.push({...this.props}); - curStateLog.push({...this.state}); + curProps = {...this.props}; + curState = {...this.state}; } render() { return
{this.state.value}
; @@ -967,61 +947,53 @@ describe('Lifecycle methods', () => { // `this.state` in componentDidUpdate should be // the updated state after getDerivedStateFromProps was called. - const finalPrevPropsLogs = [{ - foo: "foo", - children: [] - }, { - foo: "bar", - children: [] - }]; - - const finalPrevStateLogs = [{ - value: 1 - }, { - value: 2 - }]; - - const finalCurPropsLogs = [{ - foo: "bar", - children: [] - }, { - foo: "bar", - children: [] - }]; - - const finalCurStateLogs = [{ - value: 2 - }, { - value: 4 - }]; - // Initial render // state.value: initialized to 0 in constructor, 0 -> 1 in gDSFP let element = render(, scratch); expect(element.textContent).to.be.equal('1'); - expect(prevPropsLog).to.have.length(0); - expect(prevStateLog).to.have.length(0); - expect(curPropsLog).to.have.length(0); - expect(curStateLog).to.have.length(0); + expect(prevPropsArg).to.be.undefined; + expect(prevStateArg).to.be.undefined; + expect(curProps).to.be.undefined; + expect(curState).to.be.undefined; // New props // state.value: 1 -> 2 in gDSFP element = render(, scratch, scratch.firstChild); expect(element.textContent).to.be.equal('2'); - expect(prevPropsLog).to.deep.equal(finalPrevPropsLogs.slice(0, 1)); - expect(prevStateLog).to.deep.equal(finalPrevStateLogs.slice(0, 1)); - expect(curPropsLog).to.deep.equal(finalCurPropsLogs.slice(0, 1)); - expect(curStateLog).to.deep.equal(finalCurStateLogs.slice(0, 1)); + expect(prevPropsArg).to.deep.equal({ + foo: "foo", + children: [] + }); + expect(prevStateArg).to.deep.equal({ + value: 1 + }); + expect(curProps).to.deep.equal({ + foo: "bar", + children: [] + }); + expect(curState).to.deep.equal({ + value: 2 + }); // New state // state.value: 2 -> 3 in updateState, 3 -> 4 in gDSFP updateState(); rerender(); expect(element.textContent).to.be.equal('4'); - expect(prevPropsLog).to.deep.equal(finalPrevPropsLogs); - expect(prevStateLog).to.deep.equal(finalPrevStateLogs); - expect(curPropsLog).to.deep.equal(finalCurPropsLogs); - expect(curStateLog).to.deep.equal(finalCurStateLogs); + expect(prevPropsArg).to.deep.equal({ + foo: "bar", + children: [] + }); + expect(prevStateArg).to.deep.equal({ + value: 2 + }); + expect(curProps).to.deep.equal({ + foo: "bar", + children: [] + }); + expect(curState).to.deep.equal({ + value: 4 + }); }); }); @@ -1261,11 +1233,10 @@ describe('Lifecycle methods', () => { /** @type {() => void} */ let updateState; - let curPropsLog = []; - let curStateLog = []; - - let nextPropsLog = []; - let nextStateLog = []; + let curProps; + let curState; + let nextPropsArg; + let nextStateArg; class Foo extends Component { constructor(props) { @@ -1285,11 +1256,11 @@ describe('Lifecycle methods', () => { }; } shouldComponentUpdate(nextProps, nextState) { - nextPropsLog.push({...nextProps}); - nextStateLog.push({...nextState}); + nextPropsArg = {...nextProps}; + nextStateArg = {...nextState}; - curPropsLog.push({...this.props}); - curStateLog.push({...this.state}); + curProps = {...this.props}; + curState = {...this.state}; return true; } @@ -1304,61 +1275,53 @@ describe('Lifecycle methods', () => { // `nextState` in shouldComponentUpdate should be // the updated state after getDerivedStateFromProps was called - const finalCurPropsLogs = [{ - foo: "foo", - children: [] - }, { - foo: "bar", - children: [] - }]; - - const finalCurStateLogs = [{ - value: 1 - }, { - value: 2 - }]; - - const finalNextPropsLogs = [{ - foo: "bar", - children: [] - }, { - foo: "bar", - children: [] - }]; - - const finalNextStateLogs = [{ - value: 2 - }, { - value: 4 - }]; - // Initial render // state.value: initialized to 0 in constructor, 0 -> 1 in gDSFP let element = render(, scratch); expect(element.textContent).to.be.equal('1'); - expect(curPropsLog).to.have.length(0); - expect(curStateLog).to.have.length(0); - expect(nextPropsLog).to.have.length(0); - expect(nextStateLog).to.have.length(0); + expect(curProps).to.be.undefined; + expect(curState).to.be.undefined; + expect(nextPropsArg).to.be.undefined; + expect(nextStateArg).to.be.undefined; // New props // state.value: 1 -> 2 in gDSFP element = render(, scratch, scratch.firstChild); expect(element.textContent).to.be.equal('2'); - expect(curPropsLog).to.deep.equal(finalCurPropsLogs.slice(0, 1)); - expect(curStateLog).to.deep.equal(finalCurStateLogs.slice(0, 1)); - expect(nextPropsLog).to.deep.equal(finalNextPropsLogs.slice(0, 1)); - expect(nextStateLog).to.deep.equal(finalNextStateLogs.slice(0, 1)); + expect(curProps).to.deep.equal({ + foo: "foo", + children: [] + }); + expect(curState).to.deep.equal({ + value: 1 + }); + expect(nextPropsArg).to.deep.equal({ + foo: "bar", + children: [] + }); + expect(nextStateArg).to.deep.equal({ + value: 2 + }); // New state // state.value: 2 -> 3 in updateState, 3 -> 4 in gDSFP updateState(); rerender(); expect(element.textContent).to.be.equal('4'); - expect(curPropsLog).to.deep.equal(finalCurPropsLogs); - expect(curStateLog).to.deep.equal(finalCurStateLogs); - expect(nextPropsLog).to.deep.equal(finalNextPropsLogs); - expect(nextStateLog).to.deep.equal(finalNextStateLogs); + expect(curProps).to.deep.equal({ + foo: "bar", + children: [] + }); + expect(curState).to.deep.equal({ + value: 2 + }); + expect(nextPropsArg).to.deep.equal({ + foo: "bar", + children: [] + }); + expect(nextStateArg).to.deep.equal({ + value: 4 + }); }); }); From c10d70581e7524e0174f7e876016da743cce5309 Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Fri, 6 Jul 2018 15:50:29 -0700 Subject: [PATCH 7/7] Remove TODO --- test/browser/lifecycle.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/browser/lifecycle.js b/test/browser/lifecycle.js index cccfeacbc4..d6b5699363 100644 --- a/test/browser/lifecycle.js +++ b/test/browser/lifecycle.js @@ -700,7 +700,6 @@ describe('Lifecycle methods', () => { }); }); - // TODO - add test for parameters describe('#componentWillUpdate', () => { it('should NOT be called on initial render', () => { class ReceivePropsComponent extends Component {