[Fix] shallow: .setState(): stub out setState on non-root code paths as well.
#1763
Conversation
…paths as well. Fixes #1756
| } | ||
| } | ||
|
|
||
| expect(shallow(<Comp />).debug()).to.equal(''); |
There was a problem hiding this comment.
Does setState in componentDidMount invoke componentDidUpdate?
There was a problem hiding this comment.
it should; cDU is invoked whenever the component updates. no?
There was a problem hiding this comment.
In order to be called cDU, we should stub setState so I think we should move calling cDM after the stubbing.
There was a problem hiding this comment.
I'll write the tests and the patch.
https://github.com/airbnb/enzyme/compare/fix_shallow_setstate...koba04:fix_shallow_setstate?expand=1
| ({ instance } = this[RENDERER].getNode()); | ||
| } | ||
| // Ensure to call componentDidUpdate when instance.setState is called | ||
| if (instance && lifecycles.componentDidUpdate.onSetState && !instance[SET_STATE]) { |
There was a problem hiding this comment.
Nits: I should have overridden setState only in options.disableLifecycleMethods is false.
There was a problem hiding this comment.
i'll add a test for that.
There was a problem hiding this comment.
actually, can you write one? :-D i'm not sure where to put it.
or is this just an optimization?
There was a problem hiding this comment.
a05d4cb is the right place. It is unnecessary when options. disableLifecycleMethods is true.
ljharb
force-pushed
the
fix_shallow_setstate
branch
from
a05d4cb to
8b97f1f
Compare
|
Thanks, this looks great. Will merge once tests pass. |
[Fix] `shallow`: `.setState()`: stub out `setState` on non-root code paths as well.
|
Not sure why, but the 3.4.3 release broke my tests. I receive The relevant test is attached. I was able to figure out that it('<MyComponent />', async () => {
fetch.mockResponse(JSON.stringify(jsonData));
const wrapper = shallow(<MyComponent />, { disableLifecycleMethods: true });
await wrapper.instance().componentDidMount();
wrapper.update();
expect(fetch.mock.calls.length).toEqual(1);
expect(wrapper.find(Table).length).toEqual(1);
}); |
|
@monken please file a new issue; commenting on a merged PR isn't going to get anything fixed. |
| @@ -2142,7 +2142,7 @@ describe('shallow', () => { | |||
| expect(wrapper.find('.bar')).to.have.lengthOf(1); | |||
| }); | |||
|
|
|||
| it.skip('allows setState inside of componentDidMount', () => { | |||
| it('allows setState inside of componentDidMount', () => { | |||
There was a problem hiding this comment.
should not allow setState inside of componentDidMount
Fixes #1756
cc @koba04