Discussion: find(), is() and contains() are inconsistent

[codebryo]

I spent some more time in thinking about the consistency of the functions used to interact with the DOM.

So here's the example. The markup looks like this.

<div class="my-wrapper">
  <button>Action</button>
</div>

and assume it's inside the mounted wrapper.

wrapper.find('div') returns the wrapping element
wrapper.findAll('div') returns the wrapping element
wrapper.contains('div') returns false
wrapper.is('div') returns true

That's how it behaves currently. For me this though sparks the questions why find would include the whole markup and contains will start inside the root element. The explanation I found, is that the is function kind of messes up the logic. We treat the wrapper as the wrapping DOM tag, though it's actually a wrapper for the whole Vue component.

I would expect that contains would also include the root element of the markup, as pretty much the Vue wrapper actually contains a div element.
So maybe is should be used in a different way like wrapper.find('.my-wrapper').is('div').
That would make sense to me in verifying the returned element is actually the one I'd expect.
So to get the root element, as it is useful for some tests I'd recommend a new method like node that will return the root DOM node of the markup.

Example:
wrapper.node().is('div') should return true
If we then decide, calling is on the wrapper directly should do the same as a shortcut, I would totally agree. But Then I'd also expect that the wrapper.contains('div') should also return true in our current case.

wdyt?

[posva]

isn't wrapper.contains('div') like wrapper.find('div').exists()?

[eddyerburgh]

I think the behavior of contains, exists and is are fine. contains searches the entire tree, is checks the wrapper node, exists returns false if find cannot find a matching node.

In my mind, wrapper should be thought of as a node with some methods.

We discussed the behavior of find and findAll here - #5

I originally thought the behavior should be that the root node is not included in find or findAll. I then changed my mind because everyone in the thread thought the current behavior is more intuitive.

I still think that not including the root node is more intuitive for me. Currently I could run:

wrapper.find('div').wrapper.find('div').wrapper.find('div').wrapper.find('div').wrapper.find('div').exists()

and it would return true 😕

document.querySelector and document.querySelectorAll don't include the root node, so I'm starting to think that that behavior is more intuituve.

It would be good to get more input on this before we release, as it will be difficult to change once it's published. I'll reopen the issue on find's behavior - #5

[jackmellis]

I'm thinking about it this way:

You:

Does codebryo's component contain a div element?

Me:

Yes, it's the root element

You:

Does that div element contain a div element?

Me:

Nope

So wrapper.find('div').exists() would be true but wrapper.find('div').wrapper.find('div').exists() would be false.

I assume the issue there is presumably vue-test-utils will realise the div is a component and wouldn't then know whether to include itself in the contains method.

[codebryo]

@eddyerburgh as there is not much going on here, and a consensus was met already in the past I don't want to hold up the release with this discussion and would lean towards closing it.

[eddyerburgh]

Ok, I'm also happy to close 👍

[eddyerburgh]

Reopening, because the behavior of contains could be confusing for users.

My vote is that we change the behavior of contains to include the Wrapper.

[phanan]

I think that's a good idea. Though I understand the difference, the current behavior is a bit weird and inconsistent, at least to me, as I've always seen wrapper to be the <template> that wraps everything in it.

I'd argue that more often than not, to test if the component has been rendered, a user would just check if the root node exists and naturally think of contains('.root.selector') which, to his surprise, returns false.

[eddyerburgh]

After looking into it again, I think changing the behavior of contains to include the wrapper root element is a good idea. contains has been used inconsistently between JQuery and the DOM:

const div = document.querySelector('div')
jQuery.contains(div, div) // returns false

document.contains includes the element:

div.contains(div) // returns true