Request to reinstate ability to use CSS selectors in findComponent

[dospunk]

It said in the release notes that if we felt strongly about this feature we could open an issue, so that's what I'm doing. I don't think it makes sense to remove the ability to use CSS selectors in findComponent just to make VTU2 more in line with VTU1, and I think there are some notable benefits to using CSS selectors to find components.

The most significant benefit (in my opinion) is the ability to more specifically select components. Say I have a component like so, that imports a Spinner component and then shows it on two buttons depending on which is clicked

<template>
  <button @click="buttonASpinning = true">A <Spinner id="btnASpinner" v-if="buttonASpinning" /></button>
  <button @click="buttonBSpinning = true">B <Spinner id="btnBSpinner" v-if="buttonBSpinning" /></button>
</template>

<script>
import { defineComponent, ref } from 'vue'
import Spinner from '@/components/Spinner.vue'

export default defineComponent({
  components: { Spinner },
  setup() {
    const buttonASpinning = ref(false)
    const buttonBSpinning = ref(false)
    return { buttonASpinning, buttonBSpinning }
  },
})
</script>

Without CSS selectors I have to use findAllComponents(Spinner)[1] to access Spinner B, which will break if the buttons switch positions, if there are any other spinners in any other components before this component on the page, etc. With large components or views this can become a nightmare of remembering state for completely unrelated parts of the page.

It was also just nice to have another option for selecting components that didn't come with the downsides of the other three. Selecting by constructor requires you to import the constructor into your test, selecting by name requires you to know the name of the component which isn't always readily available or even present for 3rd party components, and selecting by ref requires you to add refs to components that don't already have them just for testing. Personally I like to use data attributes for selectors in tests since they can be easily removed in production.

[xanf]

With #897 being merged you can do this one:

wrapper.find('#btnBSpinner').findComponent(Spinner)

I would like to oppose this proposal - I really like simplicity of mental model - selecting by query selectors always return DOM nodes and I consider workaround mentioned on top sufficient for any "extreme" use cases. The main downside of selecting component by CSS selector is that actually multiple components can match single selector (see #689 for more context)

[dospunk]

Ah I didn't know that chaining find and findComponent was possible now! That definitely takes care of the main issue I had (despite being a little more verbose), and while it doesn't fix the issues with the other methods of selection, I can see your point about keeping the mental model simple.

[xanf]

@dospunk totally understand you. We also used a lot of chaining DOM selectors to find componnents in pre-1.0 versions of vtu and now it's time to pay

[freakzlike]

I am using this alot, so I can use the attribute and selector for unit and E2E tests. E.g:

<template>
  <TextButton label="Delete" data-test="delete-button"/>
  <TextButton label="Edit" data-test="edit-button"/>
</template>

const deleteButton = wrapper.findComponent('[data-test=delete-button')
const editButton = wrapper.findComponent('[data-test=edit-button')

Now I need to use the DOM selector for data-test and the Component selector:

const deleteButton = wrapper.find('[data-test=delete-button').findComponent({ name: 'TextButton' })
const editButton = wrapper.find('[data-test=edit-button').findComponent({ name: 'TextButton' })

Unfortunately this is not possible yet, because #897 is not in 2.0.0-rc.13

[xanf]

Oops, sorry for misleading information about #897 - I'm using own fork which is far ahead of master, so totally forgot to check if this was released

[lmiller1990]

Slightly related: chaining find and findComponent seems pretty popular, so I made a quick release just for this feature. https://github.com/vuejs/vue-test-utils-next/releases/tag/v2.0.0-rc.14

As for the feature request, I don't like the idea of mixing find (DOM selector) and findComponent (VDOM selector, entirely separate abstractions. That said, I don't really have much motivation to oppose this change - I effectively already conceded by not opposing find(...).findComponent(...) as of #897, which is basically the same as findComponent('#selector') to me.

I am interested in why @xanf opposes this, but does not oppose find(...).findComponent(...). What is the difference?

[picasocro1]

In my opinion the biggest pros of finding components using selectors was the ability to strictly separate test and dev contexts.

Using data-test or similar attributes we have testing markers which are totaly independent from the dev process.

Using component names or refs we make unnecessery dependency in our tests and make it more bugprone in case of any refactors.

I understand the argument of not mixing DOM selectors with VDOM selectors but still consider it as weaker against the idea of mixing dev and test context using same names / refs.

What's more as @lmiller1990 noticed using "trick" with find(...).findComponent(...) we achive nothing in my opinion mixing DOM/VDOM as well but we lose the possibility to separate dev/test context strongly and use test dedicated data attributes only.

So I also postulate to reinstate ability to use CSS selectors in getComponent/findComponent :)

[lmiller1990]

Inclined to agree - opinions on mixing DOM/VDOM aside, if we have find(...).findComponent(...), I don't see how that is different to findComponent('.foo').

I'd still like to get some opposite opinions, mainly @xanf, but seems like this is something we'd want to do pre 2.0.0 (I don't see much else blocking a full 2.0.0, it'd be great to get out of the rc stage).

[picasocro1]

I also would like to recive some opposite arguments :) So if anyone have any please share them.

Just for now I downgrade our project to ver rc.12 to use query selectors but still hope that in pre 2.0.0 it will be reinstated :)

[marina-mosti]

I have to agree with @freakzlike , the ability to be able to target a component using a data-test attribute is highly valuable. I've found in most cases where I use this, that I don't want to forcefully bind my test to a particular instance of a component because it's not valuable.

A component may change, for example a "RaisedButton" may change for a "FlatButton", which would break the test. Instead I can target the underlying component with data-test and let my test be agnostic to what component it is, as long as it can listen to a click for example.

This is also very valuable like he said, when building everything to also work in the same way with e2es.

RC 13 has a lot of very valuable fixes and improvements that have a lot of my tests locked with a .skip, and this particular breaking change explodes our whole setup. Is this something we could potentially put behind a toggle like config.renderStubDefaultSlot = true?

[kensodemann]

I agree with what @marina-mosti said. For now, I am changing my tests as such:

const button = wrapper.find('[data-testid="submit-button"]').findComponent({ name: 'ion-button' });

It's a bit of a pain, but it works in my case. That said, this is just for an app developed as part of a training we give our customers, not a real app. I could just as easily instruct the customers to stay on rc.12 for now and adapt when this is all sorted out... 🤷‍♂️

Personally, I would just add a css or some such field to the selector in findComponent to make the above more like:

const button = wrapper.findComponent({ css: '[data-testid="submit-button"]' });

[lmiller1990]

Let's just support CSS with findComponent. Ideals and best practices aside, deprecating this obviously is bothering a lot of users.

Would someone like to make a PR?

[xanf]

@lmiller1990 I'll handle that for both versions of VTU (actually it's kind of confusing that this discussion was never raised in VTU v1 repo)

[marina-mosti]

Thanks @lmiller1990 @xanf much appreciated.

Perhaps there should be a section for best practices in the docs that explains the problems with using CSS selector?
The docs are currently a bit confusing https://next.vue-test-utils.vuejs.org/api/#findcomponent where even one of the examples still has the data-test selector.

LMK if you want some help with that 👍🏻

[lmiller1990]

Hard to say if there is really "problems" with using CSS selectors to find components - if it works, there's not real problem. To me, it feels more like philosophical differences.

Always good to improve the docs, though, so if you think they can be better, please make them better 🚀

@xanf if you want to pick this up, that would be great!

[xanf]

Short status update for everyone involved: ETA for pull requests: tomorrow :)

[lmiller1990]

After this and a few <script setup> bugs I think we can move to a stable 2.0.0.

[xanf]

@lmiller1990
WDYT of this corner case:

<div class="foo"></div>
<SomeComponent class="foo" />
<div class="foo"></div>

Am I correct that we're expecting to silently ignore divs and return array with 1 element for .findAllComponents('.foo')?

[xanf]

@marina-mosti @lmiller1990 @afontcu @cexbrayat I would like to have your input here.
I'm actively working on this both for VTU v1 and VTU v2 and found a corner-case which seems to be close-to-impossible to solve in efficient way

Let the code speak for me:

 it('findComponent returns top-level component when multiple components are matching', () => {
    const DeepNestedChild = {
      name: 'DeepNestedChild',
      template: '<div>I am deeply nested</div>'
    }
    const NestedChild = {
      name: 'NestedChild',
      components: { DeepNestedChild },
      template: '<deep-nested-child class="in-child" />'
    }
    const RootComponent = {
      name: 'RootComponent',
      components: { NestedChild },
      template: '<div><nested-child class="in-root"></nested-child></div>'
    }

    const wrapper = mountingMethod(RootComponent, { stubs: { NestedChild } })

    expect(wrapper.findComponent('.in-root').vm.$options.name).toEqual('NestedChild')
    
    // someone might expect DeepNestedChild here, but
    // we always return TOP component matching DOM element
    expect(wrapper.findComponent('.in-child').vm.$options.name).toEqual('NestedChild')
  })

(this code is taken from my tests for VTU v1, so mountingMethod === mount / shallowMount)

One might obviously expect that .in-child selector will return DeepNestedComponent but this is quite impossible to implement - while having separate vnodes NestedComponent and DeepNestedComponent are sharing same DOM node, so our match of CSS selector will trigger on both... And it is very hard to figure out that we should return DeepNestedComponent in this case.

Also in the light of #689 which was original issue we have this case:

 it('findAllComponents returns top-level components when components are nested', () => {
    const DeepNestedChild = {
      name: 'DeepNestedChild',
      template: '<div>I am deeply nested</div>'
    }
    const NestedChild = {
      name: 'NestedChild',
      components: { DeepNestedChild },
      template: '<deep-nested-child class="in-child" />'
    }
    const RootComponent = {
      name: 'RootComponent',
      components: { NestedChild },
      template: '<div><nested-child class="in-root"></nested-child></div>'
    }

    const wrapper = mountingMethod(RootComponent, { stubs: { NestedChild } })

    expect(wrapper.findAllComponents('.in-root')).toHaveLength(1)
    expect(
      wrapper.findAllComponents('.in-root').at(0).vm.$options.name
    ).toEqual('NestedChild')

    expect(wrapper.findAllComponents('.in-child')).toHaveLength(1)

    // someone might expect DeepNestedChild here, but
    // we always return TOP component matching DOM element
    expect(
      wrapper.findAllComponents('.in-child').at(0).vm.$options.name
    ).toEqual('NestedChild')
  })

Theoretically both NestedChild and DeepNestedChild are matching CSS selector (no matter .in-root or in-child), but returning 2 of them might be superconfusing (DeepNestedChild might be an implementation detail of NestedChild, which could come from library), so my suggestion is that we should voice this in docs

Something like (including an example above):

When multiple components are immediately-nested in each other sharing same DOM node only top-level component will be returned as a result of .findComponent or .findAllComponents when CSS selector is used. If you need specific component use chaining to achieve that: wrapper.find('.in-child').findComponent(DeepNestedChild)

WDYT?

[xanf]

After certain research & discussions - I feel it's ok to proceed just with this warning in docs - this should be quite rare edge case:

• if you're using mount - it should be quite rare edge-case to use findComponent at all - we are expecting people to build their assertions against real DOM nodes
• if you're using shallowMount - most of the nodes are stubbed, so the case when we have multiple nodes matching same DOM node should be quite rare (usually top-level one will be stubbed)

So in terms of general usability it should not be affecting us to much, still worth mentioning in comments, though

[marina-mosti]

Hey @xanf I agree. I am not a fan of mount getting pushed as the "default" way to test components, it breaks in my opinion the unit encapsulation and it becomes a pseudo-integration test.

Class in particular is a complicated way to target stuff even for units, and I'm sure there's people doing it but with the way that attrs.class is working now I would definitely advise or warn about it as you said.

Thanks for pushing this issue to the finish line 🙌🏻

[xanf]

@marina-mosti the drama is far more bigger than it feels :)

it('findComponent returns top-level components when components are nested', () => {
    const DeepNestedChild = {
      name: 'DeepNestedChild',
      template: '<div>I am deeply nested</div>'
    }
    const NestedChild = {
      name: 'NestedChild',
      components: { DeepNestedChild },
      template: '<deep-nested-child data-testid="child" />'
    }
    const RootComponent = {
      name: 'RootComponent',
      components: { NestedChild },
      template: '<nested-child/>'
    }

    const wrapper = mount(RootComponent);
    expect(
      wrapper.findComponent('[data-testid=child]').vm.$options.name
    ).toEqual('RootComponent') // yes, Root, not DeepNestedChild :bomb:

[marina-mosti]

@xanf yikes. Honestly, I never ran into these type of issues before at work because we very very very rarely use mount, if at all. Spitballing, but maybe this should only work with shallow? I still argue that this is an integration test at this point.

The unit test for RootComponent should not be aware of internal implementation of NestedChild, and we're drifting into the land of bad practices IMO ~

[xanf]

@marina-mosti same here, I'm shallowMount adept, but I definitely will not want to introduce API which will be specific to shallowMount. I hope note in the docs will be sufficient

[lmiller1990]

Am I correct that we're expecting to silently ignore divs and return array with 1 element for .findAllComponents('.foo')?

Yes, I think this makes sense... findComponent should only find components, right?

@marina-mosti

I think that all the APIs should work with both shallowMount and mount where possible. This was mostly find in V1, and looks like it'll be mostly fine in V2, except some niche edge cases. Regarding unit vs integration tests, I don't see why you shouldn't be able to write both using Vue Test Utils (as opposed to Vue Unit Test Utils).

Regarding the edge cases, I think we have to accept these will exist and just document them. Most people won't run into them, and if they do, hopefully the docs can provide some useful guidance.