componentDidUpdate is not called for shallow rendered component

[delijah]

npm package versions

enzyme: 3.2.0
enzyme-adapter-react-16: 1.1.1
react: 16.2.0
react-dom: 16.2.0

Problem

When changing the state inside a component, componentDidUpdate is not called.

Expected behaviour

When changing the state inside a component, componentDidUpdate should get called.

Code example

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

class Dropdown extends React.Component {
    constructor(props) {
        super(props);

        console.log('constructor');

        this.state = {
            highlightedIndex: 0,
        };

        this.handleMouseOver = this.handleMouseOver.bind(this);
    }

    componentDidUpdate() {
        console.log('componentDidUpdate');
    }

    handleMouseOver() {
        console.log('handleMouseOver', 0);

        this.setState({
            highlightedIndex: 1,
        }, () => {
            console.log('handleMouseOver', 1);
        });

        console.log('handleMouseOver', 2);
    }

    render() {
        console.log('render');

        return <div onMouseOver={this.handleMouseOver}>Please hover me</div>;
    }
}

it('should work', () => {
    const wrapper = shallow(<Dropdown />);

    wrapper.simulate('mouseover');
    wrapper.update();
});

Console output

  console.log .../__tests__/Dropdown.js:8
    constructor

  console.log .../__tests__/Dropdown.js:34
    render

  console.log .../__tests__/Dropdown.js:22
    handleMouseOver 0

  console.log .../__tests__/Dropdown.js:34
    render

  console.log .../__tests__/Dropdown.js:27
    handleMouseOver 1

  console.log .../__tests__/Dropdown.js:30
    handleMouseOver 2

[koba04]

Currently, enzyme supports all lifecycle methods only when the update occurs through ShallowWrapper.
In this case, handleMouseOver is calling setState of the ReactComponent instance so we can't do anything without overriding setState of ReactComponent instance.

• Support all Component Lifecycle methods in ShallowRenderer #318 (comment)

I used to create a patch to support this case but we should investigate that the patch doesn't break anything.
(Also I should support forceUpdate (and replaceState) as well.

• koba04@88fdddc

@ljharb What do you think?

[ljharb]

In general, simulate doesn't actually simulate anything, so I think it should never be used.

If instead, this did wrapper.prop('onMouseOver')(), and then on the next tick, did wrapper.update(), what would happen?

[koba04]

If instead, this did wrapper.prop('onMouseOver')(), and then on the next tick, did wrapper.update(), what would happen?

The result is same. componentDidUpdate is never called.
In this case, I guess we have to override instance.setState to support componentDidUpdate because we can't know whether to call setState or not.

[craigkovatch]

Edit: recanting, looks like I misunderstood.

[koba04]

@craigkovatch The issue you've mentioned is about not to call componentDidUpdate when setState is called with mount? If so, it's a strange behavior because mount is just calling setState React provides.
wrapper.update() is just to update the rendered element that the wrapper is storing, not to rerender the element.

[ljharb]

@koba04 i'm not sure why we'd need to override setState; wouldn't that cause the component to rerender normally, and enzyme could be aware of that?

[koba04]

@ljharb
Yes, that causes to rerender normally as ReactShallowRenderer, which means componentDidUpdate is never called because ReactShallowRenderer doesn't support the lifecycle method.
When setState is called from ReactComponent instance directly, it is processed in the React ShallowRenderer so enzyme can't handle the update to call the lifecycle method.

Current lifecycle methods support is hooking update methods ShallowWrapper provides, which are shallowWrapper.setState or shallowWrapper.setProps or shallow() etc...
So enzyme should be aware of that to do that.

[craigkovatch]

@koba04 you are right, the issue I saw was inside of render() after a wrapper.update(). I will investigate the details and file a separate issue if necessary.

[delijah]

Hey. Do you have any updates on this?

[smoholkar]

@koba04 @ljharb In some cases when I set the value method on component instance, I have to first call component.instance().forceUpdate() & then component.update(). Why do I need to forceUpdate the instance explicitly ?

[koba04]

@smoholkar
I'm not sure why calling instance().forceUpdate() is required in your case.
Could you share the example?

[smoholkar]

@koba04 Here's one example

const wrapper = shallow(<TestComponent {...tableProps} {...props} />);
wrapper.instance().scrollAutoVisible = true;
wrapper.instance().forceUpdate();
wrapper.update();
const baseThings = wrapper.find('.base-things');
 expect(baseThings).toHaveLength(2);

[koba04]

@smoholkar You might be misunderstanding what update() does.
update() never rerender, only syncs a rendered elements between ShallowWrapper and ReactShallowRenderer. so if you want to rerender a component you have to use setState or forceUpdate or something.

When you use wrapper.setState() or wrapper.setProps() or something, you don't need to call update() manually because enzyme calls update() for you.
But if you need to update through instance.setState() or instance.forceUpdate() (your example), you have to call update() manually because enzyme can't handle updates happen in React Component instances.

[redonkulus]

@koba04 I'm hitting this issue too. I have an onClick handler in my component that calls setState. In enzyme 3 / react 16 I do not see componentDidUpdate being called. This is with disableLifecycleMethods: false set in the shallow constructor.

Here is a simple gist: https://gist.github.com/redonkulus/f60372c4b1091f5a8440364e69688cc6

[koba04]

@redonkulus Yes, it's the same issue as this. See also #1452 (comment)

[itssumitrai]

I am having the same issue as well.

[ferreiratiago]

Any update on this?

[ljharb]

Can someone provide a simple test case (that does not use simulate) that illustrates the problem?

[koba04]

I've created a simple test case for this.

koba04@193f40a

[ljharb]

Thanks; https://travis-ci.org/airbnb/enzyme/jobs/400283980 that shows that it's failing on React 16 specifically.

[koba04]

Yes, ReactShallowRenderer called componentDidUpdate on setState until React v15.
So in order to fix this, I assume we have to override setState (and forceUpdate) on React Component instance like the following.

#1452 (comment)

[ljharb]

ok, i've added a branch for someone to build off of - i'm not sure how to handle this in a way that is a) not React-specific outside adapters, b) not a breaking change to all the adapters (altho this might be necessary).

[ajbogh]

Currently, enzyme supports all lifecycle methods only when the update occurs through ShallowWrapper.

@koba04, this makes so much sense now. I was banging my head trying to figure out why a component that called setState itself wasn't calling the componentDidUpdate function, but if I called it myself it was working, however the prevState and this.state values were the same.

I believe this is a breaking change from enzyme 2 because it used to allow component functions to call setState and the componentDidUpdate lifecycle method would get called. We didn't need an external wrapper.update() or wrapper.setState() to be called. Can someone verify this and explain the reasoning for the change?

Component {
  componentDidUpdate(prevProps, prevState){
      // not called internally, but prevState and this.state will always match 
      // when called externally.
      console.log(prevState, this.state);
      if(prevState.thing !== this.state.thing){
        this.props.myCallback();
      }
  }

  updateThing(newThing){
    // doesn't call componentDidUpdate in enzyme 3.4.0
    setState({ thing: newThing });
  }
}

// enzyme test
wrapper.instance().updateThing('test');
expect(mockProps.myCallback).toHaveBeenCalled(); // false
wrapper.update();
expect(mockProps.myCallback).toHaveBeenCalled(); // false
wrapper.setState(); // or setProps();
expect(mockProps.myCallback).toHaveBeenCalled(); // false, state and prevState are the same

The workaround that I've discovered is to unit test the componentDidUpdate function by itself using setState from within an enzyme test, and test the function that called it separately by spying on wrapper.instance.setState().

[koba04]

@ajbogh

Can someone verify this and explain the reasoning for the change?

This change was caused by facebook/react#10372, which means this change isn't intentional for Enzyme.

Replacing setState of the instance returned from this.instance() might be a reasonable way to solve this.

[koba04]

I've created a PR to fix this. #1742

I should pass the test case.
3b3ab38

Fixed.

[ajbogh]

@koba04, thanks for the PR and comment regarding replacing setState, sadly that didn't work because it created a new error:

ShallowWrapper::setState() can only be called on the root

The fix that I used was to split the tests into two, one that spied on the instance's setState to make sure the function call used setState, and another that executed wrapper.setState() with the same information that the function set, to make sure the componentDidUpdate worked. Using 2 different tests works from a unit test standpoint, but it's still not ideal for integration-style tests (which is what this project is using).

I commented on your PR and approved it, however I am not an enzyme contributor so my approval might not mean much. Thanks!

[koba04]

@ajbogh

ShallowWrapper::setState() can only be called on the root

Could you create a test to reproduce it?

[ajbogh]

@koba04, using the same example as above, the way that I implemented it is this:

MyComponent extends React.Component {
  componentDidUpdate(prevProps, prevState){
      // not called internally, but prevState and this.state will always match 
      // when called externally.
      console.log(prevState, this.state);
      if(prevState.thing !== this.state.thing){
        this.props.myCallback();
      }
  }

  updateThing(newThing){
    // doesn't call componentDidUpdate in enzyme 3.4.0
    this.setState({ thing: newThing });
    // ^^^ error caused by `this` context
  }
}

// enzyme test
spyOn(wrapper.instance(), 'setState').and.callFake(wrapper.setState); //mock the setState
wrapper.instance().updateThing('test');
// Error: ShallowWrapper::setState() can only be called on the root

Please note that your new code should make this problem irrelevant.

The fix that I used was to split the test:

spyOn(wrapper.instance, 'setState');
wrapper.instance().updateThing('test');
expect(wrapper.instance().setState).toHaveBeenCalledWith({thing: 'test'});

// test the componentDidUpdate separately
wrapper.setState({thing: 'test'});
expect(mockProps.myCallback).toHaveBeenCalled();