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

fix: restore chaining and CSS selectors for findComponent #1910

Merged
merged 1 commit into from
Nov 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/api/wrapper/findAllComponents.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Returns a [`WrapperArray`](../wrapper-array/) of all matching Vue components.

- **Arguments:**

- `{Component|ref|name} selector`
- `selector` Use any valid [selector](../selectors.md)

- **Returns:** `{WrapperArray}`

Expand All @@ -21,3 +21,7 @@ expect(bar.exists()).toBeTruthy()
const bars = wrapper.findAllComponents(Bar)
expect(bars).toHaveLength(1)
```

::: warning Usage with CSS selectors
Using `findAllComponents` with CSS selector is subject to same limitations as [findComponent](api/wrapper/findComponent.md)
:::
30 changes: 29 additions & 1 deletion docs/api/wrapper/findComponent.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Returns `Wrapper` of first matching Vue component.

- **Arguments:**

- `{Component|ref|name} selector`
- `{Component|ref|string} selector`

- **Returns:** `{Wrapper}`

Expand All @@ -24,3 +24,31 @@ expect(barByName.exists()).toBe(true)
const barRef = wrapper.findComponent({ ref: 'bar' }) // => finds Bar by `ref`
expect(barRef.exists()).toBe(true)
```

::: warning Usage with CSS selectors
Using `findAllComponents` with CSS selector might have confusing behavior

Consider this example:

```js
const ChildComponent = {
name: 'Child',
template: '<div class="child"></div>'
}

const RootComponent = {
name: 'Root',
components: { ChildComponent },
template: '<child-component class="root" />'
}

const wrapper = mount(RootComponent)

const rootByCss = wrapper.findComponent('.root') // => finds Root
expect(rootByCss.vm.$options.name).toBe('Root')
const childByCss = wrapper.findComponent('.child')
expect(childByCss.vm.$options.name).toBe('Root') // => still Root
```

The reason for such behavior is that `RootComponent` and `ChildComponent` are sharing same DOM node and only first matching component is included for each unique DOM node
:::
4 changes: 4 additions & 0 deletions packages/shared/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,7 @@ export function warnDeprecated(method: string, fallback: string = '') {
warn(msg)
}
}

export function isVueWrapper(wrapper: Object) {
return wrapper.vm || wrapper.isFunctionalComponent
}
49 changes: 18 additions & 31 deletions packages/test-utils/src/wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
isPhantomJS,
nextTick,
warn,
warnDeprecated
warnDeprecated,
isVueWrapper
} from 'shared/util'
import { isElementVisible } from 'shared/is-visible'
import find from './find'
Expand Down Expand Up @@ -275,17 +276,6 @@ export default class Wrapper implements BaseWrapper {
this.__warnIfDestroyed()

const selector = getSelector(rawSelector, 'findComponent')
if (!this.vm && !this.isFunctionalComponent) {
throwError(
'You cannot chain findComponent off a DOM element. It can only be used on Vue Components.'
)
}

if (selector.type === DOM_SELECTOR) {
throwError(
'findComponent requires a Vue constructor or valid find object. If you are searching for DOM nodes, use `find` instead'
)
}

return this.__find(rawSelector, selector)
}
Expand Down Expand Up @@ -327,28 +317,25 @@ export default class Wrapper implements BaseWrapper {
this.__warnIfDestroyed()

const selector = getSelector(rawSelector, 'findAll')
if (!this.vm) {
throwError(
'You cannot chain findAllComponents off a DOM element. It can only be used on Vue Components.'
)
}
if (selector.type === DOM_SELECTOR) {
throwError(
'findAllComponents requires a Vue constructor or valid find object. If you are searching for DOM nodes, use `find` instead'
)
}
return this.__findAll(rawSelector, selector)

return this.__findAll(rawSelector, selector, isVueWrapper)
}

__findAll(rawSelector: Selector, selector: Object): WrapperArray {
__findAll(
rawSelector: Selector,
selector: Object,
filterFn: Function = () => true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any benefit to passing filterFn as an argument? It looks like __findAll is only called in one place, so filterFn will always be isVueWrapper. So should we just remove the argument entirely?

Then we would not need filter at all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xanf if we can resolve this one we can merge/release, can you take a look? Sorry on the delay, I've been mostly focused on V2.

Copy link
Contributor Author

@xanf xanf Nov 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lmiller1990

✔️ Done, sorry for delay, was focused on v1 -> v2 VTU migration :)

This one is actually used, I missed it :) It is used here: https://github.com/vuejs/vue-test-utils/pull/1910/files#diff-5c5637ddf50cba4f86e96ebb7854bb165fadf227f94d056fef19923be0a60e48R321

The reason for this is that we need to filter only component instances there :)

After releasing this I will do a big check-up if there are some low-hanging fruits in terms of v1/v2 behavior differences :)

): WrapperArray {
const nodes = find(this.rootNode, this.vm, selector)
const wrappers = nodes.map(node => {
// Using CSS Selector, returns a VueWrapper instance if the root element
// binds a Vue instance.
const wrapper = createWrapper(node, this.options)
wrapper.selector = rawSelector
return wrapper
})
const wrappers = nodes
.map(node => {
// Using CSS Selector, returns a VueWrapper instance if the root element
// binds a Vue instance.
const wrapper = createWrapper(node, this.options)
wrapper.selector = rawSelector
return wrapper
})
.filter(filterFn)

const wrapperArray = new WrapperArray(wrappers)
wrapperArray.selector = rawSelector
Expand Down
39 changes: 26 additions & 13 deletions test/specs/wrapper/find.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,20 +194,33 @@ describeWithShallowAndMount('find', mountingMethod => {
expect(wrapper.findComponent(Component).vnode).toBeTruthy()
})

it('throws an error if findComponent selector is a CSS selector', () => {
const wrapper = mountingMethod(Component)
const message =
'[vue-test-utils]: findComponent requires a Vue constructor or valid find object. If you are searching for DOM nodes, use `find` instead'
const fn = () => wrapper.findComponent('#foo')
expect(fn).toThrow(message)
})
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>'
}

it('throws an error if findComponent is chained off a DOM element', () => {
const wrapper = mountingMethod(ComponentWithChild)
const message =
'[vue-test-utils]: You cannot chain findComponent off a DOM element. It can only be used on Vue Components.'
const fn = () => wrapper.find('span').findComponent('#foo')
expect(fn).toThrow(message)
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'
)
})

it('allows using findComponent on functional component', () => {
Expand Down
57 changes: 44 additions & 13 deletions test/specs/wrapper/findAll.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,20 +149,51 @@ describeWithShallowAndMount('findAll', mountingMethod => {
expect(componentArr.length).toEqual(1)
})

it('throws an error if findAllComponents selector is a CSS selector', () => {
const wrapper = mountingMethod(Component)
const message =
'[vue-test-utils]: findAllComponents requires a Vue constructor or valid find object. If you are searching for DOM nodes, use `find` instead'
const fn = () => wrapper.findAllComponents('#foo')
expect(fn).toThrow(message)
})
it('findAllComponents ignores DOM nodes matching same CSS selector', () => {
const RootComponent = {
components: { Component },
template: '<div><Component class="foo" /><div class="foo"></div></div>'
}
const wrapper = mountingMethod(RootComponent)
expect(wrapper.findAllComponents('.foo')).toHaveLength(1)
expect(
wrapper
.findAllComponents('.foo')
.at(0)
.is(Component)
).toBe(true)
})

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>'
}

it('throws an error if chaining findAllComponents off a DOM element', () => {
const wrapper = mountingMethod(ComponentWithChild)
const message =
'[vue-test-utils]: You cannot chain findAllComponents off a DOM element. It can only be used on Vue Components.'
const fn = () => wrapper.find('span').findAllComponents('#foo')
expect(fn).toThrow(message)
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')
})

it('returns correct number of Vue Wrapper when component has a v-for', () => {
Expand Down
2 changes: 1 addition & 1 deletion test/specs/wrapper/setValue.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describeWithShallowAndMount('setValue', mountingMethod => {
})

if (process.env.TEST_ENV !== 'browser') {
it.only('sets element of multiselect value', async () => {
it('sets element of multiselect value', async () => {
const wrapper = mountingMethod(ComponentWithInput)
const select = wrapper.find('select.multiselect')
await select.setValue(['selectA', 'selectC'])
Expand Down