enzymejs / enzyme-matchers

Jasmine/Jest assertions for enzyme
MIT License
892 stars 116 forks source link

not.toHaveProp() doesn't work with propName={undefined} #285

Closed tleunen closed 5 years ago

tleunen commented 5 years ago

After testing, toHaveProp doesn't work well with undefined values.

Let's say you have mount(<Comp action={undefined} />), these two don't behave the same:

The following will fail:

expect(wrapper).not.toHaveProp('action');
Expected <Comp> not to receive the prop "action"
--
  | Expected props: action: undefined
  | Actual props: action: undefined

But the following works fine:

expect(wrapper).toHaveProp('action', undefined);

They should probably be similar, right?

blainekasten commented 5 years ago

Hmm. It should work. This is the line that runs that:

https://github.com/FormidableLabs/enzyme-matchers/blob/master/packages/enzyme-matchers/src/assertions/toHaveProp.js#L32

You are correct in that assertion should pass. Are you on latest?

tleunen commented 5 years ago

7.0.1, yes. I'll double check the code while debugging the assertion later. I'll report back

tleunen commented 5 years ago

You can easily reproduce that by creating a test in jest-enzyme with this:

const wrapper = shallow(<div undef={undefined}>Hi</div>);
expect(wrapper).not.toHaveProp('undef');

@blainekasten As you said, not.toHaveProp('x') goes into the condition you mentioned, but it passes because the prop exists. Here's the log for the props from the test at the beginning of this post:

{ props: { undef: undefined, children: 'Hi' } }

Could it be that we just need to check for the undefined value as well?

pass: props.hasOwnProperty(propKey) && props[propKey] !== undefined
blainekasten commented 5 years ago

So i've been diving into this. The more I work on it, the more I like the existing workflow. Technically speaking, undef={undefined} still means that undef is a prop on the component. So to say that the component is not.toHaveProp('undef') is technically untrue.

If a prop is getting passed in as undefined, it seems both simple, and honestly more helpful to see the explicit undefined expectance.

So with that, I'm going to close this. Feel free to continue the discussion if you have further arguments. Thanks for raising the issue! Definitely made me question the goal. 👍

tleunen commented 5 years ago

Yep. I actually agree with you too. The more I thought about it, the more I understood why it was done like that ;)