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

Support find chaining #1490

Closed
Stoom opened this issue Mar 27, 2020 · 9 comments
Closed

Support find chaining #1490

Stoom opened this issue Mar 27, 2020 · 9 comments

Comments

@Stoom
Copy link
Contributor

Stoom commented Mar 27, 2020

Version

1.0.0-beta.32

Steps to reproduce

Creating a follow-up issue from PR #1481. It seems that chaining wrapper.find that use component objects. It looks like using string selectors are fine.

What is expected?

wrapper.find to be chainable

What is actually happening?

According to PR #1481, it will throw exceptions

@Stoom
Copy link
Contributor Author

Stoom commented Mar 27, 2020

@unclejustin could you give some more detail on how find chaining was breaking? With the following test, it was working fine for me.

  it.only('allows chainging of component objects', () => {
    const TestChildComp = {
      template: '<div id="child" />',
      name: 'test-child-component'
    }
    const TestParentComp = {
      template: '<div id="parent"><slot /></div>',
      name: 'test-parent-component'
    }
    const TestComponent = {
      template: `
      <div>
        <TestParentComp ref="parent">
          <TestChildComp ref="child" />
        </TestParentComp>
      </div>`,
      components: {
        TestParentComp,
        TestChildComp
      }
    }
    const wrapper = mountingMethod(TestComponent)
    const parent = wrapper.find({ ref: 'parent'})
    expect(parent.find(TestChildComp).exists()).to.equal(true)
  })
 MOCHA  Testing...



  find with mount
    √ allows chainging of component objects

  find with shallowMount
    √ allows chainging of component objects


  2 passing (30ms)

 MOCHA  Tests completed successfully

@dobromir-hristov
Copy link
Contributor

That woks fine, because you are using find on another Vue instance. If you use ref seletor for a normal DOM node, it will fail.

@Stoom
Copy link
Contributor Author

Stoom commented Apr 4, 2020

Aaah so should we make this more clear in the docs, or is the end goal to support chaining on normal DOM nodes?

@lmiller1990
Copy link
Member

I don't think it even makes sense to find a component of a DOM node. If someone wants to find a component, why are they going via a DOM node in the first place? Just do find(Component).

I see a DOM node as a lower level construct than a Vue component, once you are on a DOM node, I don't think it really makes sense to attempt to find a Vue component, anyway.

@Stoom
Copy link
Contributor Author

Stoom commented Apr 6, 2020

@lmiller1990 do you have a recommendation on how to phrase the linked docs? To me, it seems to say that you can not do a find after a find.

@lmiller1990
Copy link
Member

It's tricky 🤔

Do we need document this? Do people look for obscure caveats like this in the docs? Maybe??

If you can come up with some simple and concise, happy to include it. It's difficult to explain. @dobromir-hristov suggested we support find and findByComponent as separate APIs in the future, and they cannot be used together. I like this solution - this problem goes away. Thoughts?

@unclejustin
Copy link
Contributor

I don't think it even makes sense to find a component of a DOM node. If someone wants to find a component, why are they going via a DOM node in the first place? Just do find(Component).

I see a DOM node as a lower level construct than a Vue component, once you are on a DOM node, I don't think it really makes sense to attempt to find a Vue component, anyway.

In my case I was trying to assert that an icon was displayed inside of an element without using css or some other layout option in the selector. Just a personal preference, refs feel cleaner to me.

It took me by surprise that this failed because the docs at them time said that find returns a wrapper and wrapper supports finding by component.

Personally I think find should be able to be chained either way, but I am admittedly naive in how this is implemented. Breaking out findByComponent would certainly address the issue and make it more clear.

@lmiller1990
Copy link
Member

Yep, I really agree with breaking them out. The main complexity is when you search for a DOM node, you are searching the actual DOM. When you search for a component, you are not searching the DOM - but Vue's vdom tree. This makes sense - you can't see a component in a DOM - it's just an abstraction created for developers - although it's probably not something you think about when testing.

The more I think about it, I see that VTU is actually two libraries... one for querying and interacting with DOM nodes, and one for interacting with Vue itself, and it's vdom and API. I wonder how we could improve the API to make this more clear. Changing beta (soon to be v1) probably doesn't make sense, but for Vue 3 integration, we have a chance to make some changes.

I think we should make an issue to discuss this!

@lmiller1990
Copy link
Member

Ok, let's do it: #1498

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants