New API: Alias exists to isEmpty

[christemple]

Question: Should isEmpty() be replaced by an exist() function?

My personal experience
So I have been trying to test a React component that can return false in the render function [1].

After reading the docs I seen isEmptyand thought this would be a perfect fit, until I read the example of how to use isEmpty [2]

It almost seems like isEmpty() in this scenario is closer to exists() for example, the docs demonstrate searching for a child node that does not exist, and that is the only time isEmpty() will return true.

The scenario I was hoping for is given the following component:

class Example extends React.Component {
  render() {
    return false;
  }
}

and the following test:

it('is empty', () => {
  const wrapper = mount(<Example />);
  expect(wrapper.isEmpty()).to.equal(true);
});

This results in a failure expecting false to equal true, because the isEmpty() call doesn't check what I assumed it to, it checks the length of the node, of which every wrapper never has 0, unless you do a .find for a node within it that does not exist.

Even doing the following still does not result in behaviour that I'd expect, it still returns false for isEmpty():

it('is empty', () => {
  const wrapper = mount(<Example />);
  const example = wrapper.find(Example);
  expect(example.isEmpty()).to.equal(true);
});

After a while of reading through the docs I found a way of accomplishing what I was looking to do:

it('is empty', () => {
  const wrapper = mount(<Example />);
  expect(wrapper.html()).to.equal(null);
});

So, to wrap up...

• Does anyone else find isEmpty() misleading?
• Would exist() be more intuitive for ehat isEmpty() is currently doing?
• Would the isEmpty() implementation suit better to checking .html() === null?

Thanks

Disclaimer: I have only started using enzyme and think it's a fantastic library

[1] https://facebook.github.io/react/docs/react-component.html#render
[2] http://airbnb.io/enzyme/docs/api/ReactWrapper/isEmpty.html

[blainekasten]

@chriscartlidge Thanks for a fantastic write up! You raised good points. I'll try and consolidate my thoughts.

1. exists is a much better name than isEmpty for checking if a query found nodes.
2. isEmpty still doesn't feel right for checking .html() === null, and I'm not sure that use-case warrants an explicit API.

The isEmpty API makes sense from enzyme's semantics, as it is checking for node(s). But from a user standpoint, it is an awkward API that in some ways forces you to understand enzyme's decisions.

However, the main issue with changing the API to support exists over isEmpty is that it is a breaking change, and testing utility libraries are the hardest to introduce breaking changes in. Typical users who upgrade a testing library to a breaking change, just delete any tests that break as it's difficult to find all uses.

In order to do this successfully, I would say we could just add the exists API and put in deprecation warning logs into calls of isEmpty for a while. Eventually removing them and bumping major.

Before jumping into it though, let's make sure this API addition/change is in line with the other maintainers thoughts, @ljharb @aweary

[ljharb]

I'm fine with adding the "exists" API, and I agree with the hesitation around making a breaking change.

[blainekasten]

@ljharb thanks! @christemple did you want to write the code and submit a PR? If you do, ping me and i'll work on getting it implemented.

[themre]

I also noticed that in the docs there is expect(wrapper.contains(<div className="foo bar" />)).to.equal(true); which is not correct.
to.equal should be toEqual(true). CRA project has luckily this correctly documented.

[ljharb]

@themre in fact it's correct, using chai's expect. You're thinking of the expect library, which is not the most common assertion lib used with mocha.

[blackpost38]

I had a same problem. I tested with isEmpty but i got fail.

But I found interesting something that is isEmptyRender in ReactWrapper.jsx

I think we have to use isEmptyRender when testing react component that returns null or false

(but isEmptyRender is not shown in docs. maybe we need to update docs.)

[christemple]

@blainekasten @ljharb thanks for the feedback! I'll get a PR sorted this weekend with your suggestions.

@blackpost38 I'll also have a look at isEmptyRender, thanks for the heads up.