Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression on 'componentDidUpdate' not being called when calling 'setProps' on shallow wrapped component #2020

Closed
2 of 13 tasks
BertrandFritsch opened this issue Feb 22, 2019 · 4 comments

Comments

@BertrandFritsch
Copy link

BertrandFritsch commented Feb 22, 2019

Calling setProps on a shallow wrapper may result in not calling componentDidUpdate of the wrapped component. It's the case when the state depends on a prop and when that specific setProps call does change the state. If that call doesn't change the state, componentDidUpdate is called. I suspect this is a regression of #2007. It worked fine in version 3.8.0

To reproduce

import { shallow } from 'enzyme';
import * as React from 'react';

class DummyComp extends React.PureComponent {
  constructor(props) {
    super(props);
    this.state = { stateCounter: 0 };
  }

  static getDerivedStateFromProps(nextProps) {
    // set stateCounter if asked for
    if (nextProps.propCounter > -1) {
      return {
        stateCounter: nextProps.propCounter
      };
    }

    return null;
  }

  componentDidUpdate() {
    console.log('Counter:', this.state.stateCounter);
  }

  render() {
    return <p>Counter: { this.state.stateCounter }</p>;
  }
}

describe('Counter', () => {
  it('should call cDU on setProps', () => {

    const wrapper = shallow(<DummyComp propCounter={ 0 } />);

    const cDUSpy = jest.spyOn(wrapper.instance(), 'componentDidUpdate');

    // does not change the state, so cDU will be called
    wrapper.setProps({ propCounter: -1 });

    expect(cDUSpy).toBeCalledTimes(1);

    // does change the state, cDU should be called anyway
    wrapper.setProps({ propCounter: 1 });

    expect(cDUSpy).toBeCalledTimes(2); // this test will fail
  });
});

Your environment

API

  • shallow
  • mount
  • render

Version

library version
enzyme 3.9.0
react 16.8.2
react-dom 16.8.2
react-test-renderer 16.8.2
adapter (below)

Adapter

  • enzyme-adapter-react-16
  • enzyme-adapter-react-16.3
  • enzyme-adapter-react-16.2
  • enzyme-adapter-react-16.1
  • enzyme-adapter-react-15
  • enzyme-adapter-react-15.4
  • enzyme-adapter-react-14
  • enzyme-adapter-react-13
  • enzyme-adapter-react-helper
  • others ( )
@ljharb
Copy link
Member

ljharb commented Feb 22, 2019

cc @peanutenthusiast; see also #2007 (comment)

@ghost
Copy link

ghost commented Feb 23, 2019

@ljharb thanks for the cc will look into this.

@ghost
Copy link

ghost commented Feb 26, 2019

Replicated the above test case, which results in the aforementioned regression. With regards to the fix and the previous discussions from 1862, is it possible to apply a fix without implementing batched updates? @ljharb any pointers would be appreciated

@ljharb
Copy link
Member

ljharb commented Feb 26, 2019

@peanutenthusiast i'm honestly not sure; the first thing ideally would be a PR that contains the failing test case, and we can iterate from there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants