Fix to call cDU when an instance calls setState #1742
Conversation
stub instance.setState when the component is mounting Fixes enzymejs#1452.
koba04
changed the title
|
hmm, I should pass the test case |
|
Fixed #1742 (comment) |
| @@ -1468,7 +1468,7 @@ describe('shallow', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it('should call componentWillReceiveProps for new renders', () => { | |||
| it.skip('should call componentWillReceiveProps for new renders', () => { | |||
| expect(spy).to.have.property('callCount', 2); | ||
| }); | ||
|
|
||
| it('should call `componentDidUpdate` when component’s `setState` is called through a bound method', () => { |
There was a problem hiding this comment.
This second test is essentially the same as the first test above, but this one adds a button. Do we need to test that use case since it's not very different than the first?
There was a problem hiding this comment.
This test is to verify to work well with a bound function.
this.onChange = this.onChange.bind(this);
My first implementation didn't pass this.
| const wrapper = shallow(<Foo />); | ||
| wrapper.setState({ foo: 'wrapper setState update' }); | ||
| expect(wrapper.state('foo')).to.equal('wrapper setState update'); | ||
| expect(spy).to.have.property('callCount', 1); |
There was a problem hiding this comment.
I believe the lines above this comment should be one test, expecting that a wrapper.setState call works with componentDidUpdate, while the lines below this should be a separate test to ensure that the instance methods can set their own state and cause a componentDidUpdate as well.
Test 1: 'should call componentDidUpdate when a wrapper’s setState is called'
Test 2: 'should call componentDidUpdate when component’s setState is called'
ljharb
force-pushed
the
fix-to-call-cDU-from-instance
branch
from
f12be61 to
02c6868
Compare
|
LGTM! |
ljharb
force-pushed
the
fix-to-call-cDU-from-instance
branch
from
5b2d442 to
a50570d
Compare
|
Thanks! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| // Ensure to call componentDidUpdate when instance.setState is called | ||
| if (instance && lifecycles.componentDidUpdate.onSetState && !instance[SET_STATE]) { | ||
| privateSet(instance, SET_STATE, instance.setState); | ||
| instance.setState = (...args) => this.setState(...args); |
There was a problem hiding this comment.
With this change i am now seeing alot of failing tests which have a setState in componentDidMount. Method "setState" is only meant to be run on a single node. undefined found instead.
Fix #1452
This is not an ideal fix, but works 😅
(This PR is based on #1741 to run all tests)