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.

Sign up for GitHub

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

Jump to bottom

generate named slots content for stubbed components via shallowMount #1309

Closed
wants to merge 2 commits into from
Closed

generate named slots content for stubbed components via shallowMount #1309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7fdcbd
enforce consistent sort order of named slots in shallowMount output
Sep 13, 2019
97228ce
chore: rebasing create-component-stubs
JessicaSachs Jan 21, 2020
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
110 changes: 72 additions & 38 deletions packages/create-instance/create-component-stubs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import {
throwError,
camelize,
capitalize,
hyphenate,
keys
hyphenate
// keys
Copy link
Collaborator

Choose a reason for hiding this comment

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

Commented out for formatting until we can figure out how to merge everything in.

} from '../shared/util'
import {
componentNeedsCompiling,
Expand Down Expand Up @@ -80,25 +80,42 @@ function resolveOptions(component, _Vue) {
return options
}

function getScopedSlotRenderFunctions(ctx: any): Array<string> {
// In Vue 2.6+ a new v-slot syntax was introduced
// scopedSlots are now saved in parent._vnode.data.scopedSlots
// We filter out the _normalized and $stable key
if (
ctx &&
ctx.$options &&
ctx.$options.parent &&
ctx.$options.parent._vnode &&
ctx.$options.parent._vnode.data &&
ctx.$options.parent._vnode.data.scopedSlots
) {
const slotKeys: Array<string> = ctx.$options.parent._vnode.data.scopedSlots
return keys(slotKeys).filter(x => x !== '_normalized' && x !== '$stable')
}

return []
function getAttributes(
functional: boolean,
functionalCtx: any,
componentCtx
): Object {
return functional
? {
...functionalCtx.props,
...functionalCtx.data.attrs,
class: createClassString(
functionalCtx.data.staticClass,
functionalCtx.data.class
)
}
: componentCtx.$props
}

// function getScopedSlotRenderFunctions(ctx: any): Array<string> {
// // In Vue 2.6+ a new v-slot syntax was introduced
// // scopedSlots are now saved in parent._vnode.data.scopedSlots
// // We filter out the _normalized and $stable key
// if (
// ctx &&
// ctx.$options &&
// ctx.$options.parent &&
// ctx.$options.parent._vnode &&
// ctx.$options.parent._vnode.data &&
// ctx.$options.parent._vnode.data.scopedSlots
// ) {
// const slotKeys: Array<string> = ctx.$options.parent._vnode.data.scopedSlots
// return keys(slotKeys).filter(x => x !== '_normalized' && x !== '$stable')
// }
//
// return []
// }

Copy link
Collaborator

Choose a reason for hiding this comment

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

So @lmiller1990 I'm mostly concerned about why no tests break when I comment this out

export function createStubFromComponent(
originalComponent: Component,
name: string,
Expand All @@ -110,36 +127,53 @@ export function createStubFromComponent(
// ignoreElements does not exist in Vue 2.0.x
if (Vue.config.ignoredElements) {
Vue.config.ignoredElements.push(tagName)
if (Vue.config.ignoredElements.indexOf('template-stub') === -1) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

So the template-stub stays as <template-stub /> in the finished render, correct?

Vue.config.ignoredElements.push('template-stub')
}
}

return {
...getCoreProperties(componentOptions),
$_vueTestUtils_original: originalComponent,
$_doNotStubChildren: true,
render(h, context) {
const attrs = getAttributes(componentOptions.functional, context, this)
const slots = context ? context.slots() : this._renderProxy.$slots

// ensure consistent ordering of slots (default first, then alphabetical)
const sortedSlotEntries = Object.entries(slots)
Copy link
Member

Choose a reason for hiding this comment

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

Nice usage of Object.entries

sortedSlotEntries.sort(([slotNameA], [slotNameB]) =>
slotNameA === 'default'
? -1
: slotNameB === 'default'
? 1
: slotNameA.localeCompare(slotNameB)
Copy link
Member

Choose a reason for hiding this comment

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

I would have used a simple > and < for the string compare. Not sure it makes a big difference - it is common to use characters like á in code? (my language does not have those characters, so I have never seen this before).

If we are doing to support using localeCompare, maybe we should have a test containing a á character vs an a character. Although probably not strictly necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just a thought, this kind of ternary is super confusing. I would deff advise for some sort of reordering or other.

)

const sortedSlots = sortedSlotEntries.map(([slotName, slotChildren]) =>
slotName === 'default'
? slotChildren
: h('template-stub', { attrs: { slot: slotName } }, slotChildren)
)

// let children
// if (context) {
// children = sortedSlots
// } else if (this.$options._renderChildren) {
// children = this.$options._renderChildren
// } else {
// children = getScopedSlotRenderFunctions(this).map(x =>
// this.$options.parent._vnode.data.scopedSlots[x]()
// )
// }

Copy link
Collaborator

Choose a reason for hiding this comment

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

This was initially how I expanded out and merged dev and this PR, but was failing so I commented it out and reverted to mainly this PR's version of rendering slots

return h(
tagName,
{
ref: componentOptions.functional ? context.data.ref : undefined,
attrs: componentOptions.functional
? {
...context.props,
...context.data.attrs,
class: createClassString(
context.data.staticClass,
context.data.class
)
}
: {
...this.$props
}
attrs,
ref: componentOptions.functional ? context.data.ref : undefined
},
context
? context.children
: this.$options._renderChildren ||
getScopedSlotRenderFunctions(this).map(x =>
this.$options.parent._vnode.data.scopedSlots[x]()
)
sortedSlots
)
}
}
Expand Down
22 changes: 16 additions & 6 deletions test/specs/shallow-mount.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,9 @@ describeRunIf(process.env.TEST_ENV !== 'node', 'shallowMount', () => {
const TestComponent = {
template: `
<child>
<p slot="header">Hello</p>
<p slot="footer">World</p>
default slot content
<template slot="header">Hello</template>
<template slot="footer"><child /></template>
</child>
`
}
Expand All @@ -92,8 +93,11 @@ describeRunIf(process.env.TEST_ENV !== 'node', 'shallowMount', () => {
})
expect(wrapper.html()).to.equal(
'<child-stub>\n' +
' <p>Hello</p>\n' +
' <p>World</p>\n' +
' default slot content\n' +
' <template-stub slot="footer">\n' +
' <child-stub></child-stub>\n' +
' </template-stub>\n' +
' <template-stub slot="header">Hello</template-stub>\n' +
'</child-stub>'
)
})
Expand All @@ -105,7 +109,9 @@ describeRunIf(process.env.TEST_ENV !== 'node', 'shallowMount', () => {
expect(wrapper.find('.new-example').exists()).to.equal(true)
expect(wrapper.html()).to.equal(
'<componentwithvslot-stub>\n' +
' <p class="new-example">new slot syntax</p>\n' +
' <template-stub slot="newSyntax">\n' +
' <p class="new-example">new slot syntax</p>\n' +
' </template-stub>\n' +
'</componentwithvslot-stub>'
)
})
Expand All @@ -130,7 +136,11 @@ describeRunIf(process.env.TEST_ENV !== 'node', 'shallowMount', () => {
expect(wrapper.find({ name: 'Foo' }).exists()).to.equal(true)
expect(wrapper.find('.new-example').exists()).to.equal(true)
expect(wrapper.html()).to.equal(
'<foo-stub>\n' + ' <p class="new-example">text</p>\n' + '</foo-stub>'
'<foo-stub>\n' +
' <template-stub slot="newSyntax">\n' +
Copy link
Collaborator

Choose a reason for hiding this comment

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

The need to update all the assertions with template-stub worry me. This will be a breaking change :/

' <p class="new-example">text</p>\n' +
' </template-stub>\n' +
'</foo-stub>'
)
})

Expand Down