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

mount() wrapper is populated before componentDidMount() #1708

Closed
ericsoco opened this issue Jul 6, 2018 · 4 comments
Closed

mount() wrapper is populated before componentDidMount() #1708

ericsoco opened this issue Jul 6, 2018 · 4 comments

Comments

@ericsoco
Copy link

ericsoco commented Jul 6, 2018

Describe the bug
When mount()ing a component with a componentDidMount() method that appends DOMElements to a container element created in render(), the resulting wrapper does not contain those DOMElements.

To Reproduce

class MyComponent extends Component {
  componentDidMount() {
    const el = document.createElement('div');
    el.classList.add('new-inner');
    this.container.appendChild(el);
  }

  render() {
    return (
      <div
        ref={el => (this.container = el)}
        className={'outer'}
      >
        <div className={'inner'}></div>
      </div>
    );
  }
}

const wrapper = mount(<MyComponent>);

console.log(wrapper.debug());
// <MyComponent>
//   <div className="outer">
//     <div className="inner"></div>
//   </div>
// </MyComponent>

console.log(wrapper.html());
// <div class="outer"><div class="inner"></div><div class="new-inner"></div></div>

console.log(wrapper.find('.outer').length); // 1
console.log(wrapper.find('.inner').length); // 1
console.log(wrapper.find('.new-inner').length); // 0

I've confirmed that calling wrapper.update() does run render() a second time, but still does not update the wrapper. This makes me wonder if this is the same bug as #1153.

Expected behavior
Since mount() synchronously calls componentDidMount(), I expect the resulting wrapper to reflect the state of the component after componentDidMount(). Failing that, wrapper.update() should force a wrapper update.

Desktop (please complete the following information):

  • OS: OSX 10.13.5
  • Browser: Node 8.11.2
  • Versions:
    • enzyme@3.3.0
    • enzyme-adapter-react-15.4@1.0.5
@ljharb
Copy link
Member

ljharb commented Jul 7, 2018

I think that's the correct behavior - didMount runs after the mount, and although mount is calling that method, it should be returning the tree prior to DidMount - ie, when it's mounted.

If you add wrapper.forceUpdate() instead, does that work?

@ericsoco
Copy link
Author

TypeError: wrapper.forceUpdate is not a function :`(

I hear what you're saying, but because componentDidMount is called synchronously by mount() (i.e. before any subsequent assertions or .find() calls), my expectation was that the wrapper would contain the component state as of the end of the mount() call stack.

Whether or not it ends up being correct behavior, I can't use Enzyme selectors / inspection methods to assert the mounted DOM here. Not sure if that's a React 15 issue that will be solved by bumping to React 16 and corresponding Enzyme or not...

@ljharb
Copy link
Member

ljharb commented Sep 1, 2018

@ericsoco so the real issue here is that you're using DOM methods to directly manipulate the DOM, which is an antipattern in react. Instead, use state, and render as jsx the new div. This passes:

  it('works', () => {
    class MyComponent extends React.Component {
      constructor(...args) {
        super(...args);
        this.state = { newInner: false };
      }

      componentDidMount() {
        this.setState({ newInner: true });
      }

      render() {
        const { newInner } = this.state;
        return (
          <div
            ref={this.containerRef}
            className="outer"
          >
            <div className="inner" />
            {newInner && <div className="new-inner" />}
          </div>
        );
      }
    }

    const wrapper = mount(<MyComponent />);

    expect(wrapper.find('.outer')).to.have.lengthOf(1);
    expect(wrapper.find('.inner')).to.have.lengthOf(1);
    expect(wrapper.find('.new-inner')).to.have.lengthOf(1);
  });

In other words, enzyme is meant to test the React tree - and explicitly not to test the actual DOM. If you're doing any direct DOM manipulation, you're stepping outside of the box React wants you to stay inside - see https://github.com/airbnb/enzyme/blob/master/docs/common-issues.md#testing-third-party-libraries.

@ljharb ljharb closed this as completed Sep 1, 2018
@vamsi920
Copy link

I think that's the correct behavior - didMount runs after the mount, and although mount is calling that method, it should be returning the tree prior to DidMount - ie, when it's mounted.

If you add wrapper.forceUpdate() instead, does that work?

Throwing an error of :
wrapper.forceUpdate is not a function

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

3 participants