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: improve slots option #813

Merged
merged 9 commits into from
Jul 22, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions packages/create-instance/create-functional-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

import { throwError } from 'shared/util'
import { validateSlots } from './validate-slots'
import { createSlotVNodes } from './add-slots'
import { createSlotVNodes } from './create-slot-vnodes'

export default function createFunctionalComponent (
component: Component,
mountingOptions: Options
mountingOptions: Options,
_Vue: Component
): Component {
if (mountingOptions.context && typeof mountingOptions.context !== 'object') {
throwError('mount.context must be an object')
Expand All @@ -25,7 +26,7 @@ export default function createFunctionalComponent (
mountingOptions.context.children.map(
x => (typeof x === 'function' ? x(h) : x)
)) ||
createSlotVNodes(h, mountingOptions.slots || {})
createSlotVNodes(h, mountingOptions.slots || {}, _Vue)
)
},
name: component.name,
Expand Down
6 changes: 3 additions & 3 deletions packages/create-instance/create-instance.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow

import { createSlotVNodes } from './add-slots'
import { createSlotVNodes } from './create-slot-vnodes'
import addMocks from './add-mocks'
import { addEventLogger } from './log-events'
import { createComponentStubs } from 'shared/stub-components'
Expand Down Expand Up @@ -40,7 +40,7 @@ export default function createInstance (
(component.options && component.options.functional) ||
component.functional
) {
component = createFunctionalComponent(component, options)
component = createFunctionalComponent(component, options, _Vue)
} else if (options.context) {
throwError(
`mount.context can only be used when mounting a ` + `functional component`
Expand Down Expand Up @@ -129,7 +129,7 @@ export default function createInstance (
provide: options.provide,
render (h) {
const slots = options.slots
? createSlotVNodes(h, options.slots)
? createSlotVNodes(h, options.slots, _Vue)
: undefined
return h(
Constructor,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// @flow

import Vue from 'vue'
import { compileToFunctions } from 'vue-template-compiler'

function startsWithTag (str: SlotValue): boolean {
Expand All @@ -9,7 +10,8 @@ function startsWithTag (str: SlotValue): boolean {
function createVNodesForSlot (
h: Function,
slotValue: SlotValue,
name: string
name: string,
_Vue: any
): VNode | string {
if (typeof slotValue === 'string' && !startsWithTag(slotValue)) {
return slotValue
Expand All @@ -18,22 +20,41 @@ function createVNodesForSlot (
const el =
typeof slotValue === 'string' ? compileToFunctions(slotValue) : slotValue

const vnode = h(el)
let vnode = h(el)
if (typeof slotValue === 'string') {
const vue = new Vue()
Copy link
Member

Choose a reason for hiding this comment

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

I think this would be a problem if you mount a component that uses localVue and the slot component accesses a property that is only added to the localVue prototype, can you write a test to check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is.
Changing localVue.prototype, this does not work.
Using vm._renderProxy is correct if possible.
I will add test for changing localVue.prototype.

const _vue = new _Vue()
Copy link
Member

Choose a reason for hiding this comment

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

why not just use the _vue.renderProxy rather than mixing both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it raises an error.

for (const key in _vue._renderProxy) {
if (!(vue._renderProxy[key])) {
vue._renderProxy[key] = _vue._renderProxy[key]
Copy link
Member

Choose a reason for hiding this comment

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

Why not always use _vue.renderProxy?

Copy link
Contributor Author

@38elements 38elements Jul 22, 2018

Choose a reason for hiding this comment

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

When vue.renderProxy is used, it raises an error.
When _vue.renderProxy is used, it raises an error.

TypeError: Cannot read property 'Ctor' of undefined

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean when _vue.renderProxy is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am sorry.
Yes, I do.

}
}
try {
// $FlowIgnore
vnode = el.render.call(vue._renderProxy, h)
Copy link
Member

Choose a reason for hiding this comment

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

Do components without a render function work here?

For example:

mount(TestComponent,{
  slots: { default: {
    template: '<div />'
  }}
})

Can you add a test for this kind of slot

vnode = h(vnode.tag, vnode.data || {}, vnode.children)
} catch (e) {
}
}

vnode.data.slot = name
return vnode
}

export function createSlotVNodes (
h: Function,
slots: SlotsObject
slots: SlotsObject,
_Vue: any
): Array<VNode | string> {
return Object.keys(slots).reduce((acc, key) => {
const content = slots[key]
if (Array.isArray(content)) {
const nodes = content.map(slotDef => createVNodesForSlot(h, slotDef, key))
const nodes = content.map(
slotDef => createVNodesForSlot(h, slotDef, key, _Vue)
)
return acc.concat(nodes)
}

return acc.concat(createVNodesForSlot(h, content, key))
return acc.concat(createVNodesForSlot(h, content, key, _Vue))
}, [])
}
18 changes: 18 additions & 0 deletions test/resources/components/component-with-parent-name.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<template>
<div><span baz="qux">{{ fromLocalVue }},{{ bar }}</span></div>
</template>

<script>
export default{
name: 'component-with-parent-name',
props: ['fromLocalVue'],
data () {
return {
bar: 'quux'
}
},
mounted () {
this.$parent.childComponentName = this.$options.name
}
}
</script>
39 changes: 39 additions & 0 deletions test/specs/mounting-options/slots.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import { compileToFunctions } from 'vue-template-compiler'
import Component from '~resources/components/component.vue'
import ComponentWithSlots from '~resources/components/component-with-slots.vue'
import ComponentAsAClass from '~resources/components/component-as-a-class.vue'
import ComponentWithParentName from '~resources/components/component-with-parent-name.vue'
import { describeWithMountingMethods, vueVersion } from '~resources/utils'
import { itSkipIf, itDoNotRunIf } from 'conditional-specs'
import { mount, createLocalVue } from '~vue/test-utils'

describeWithMountingMethods('options.slots', mountingMethod => {
it('mounts component with default slot if passed component in slot object', () => {
Expand Down Expand Up @@ -568,4 +570,41 @@ describeWithMountingMethods('options.slots', mountingMethod => {
expect(wrapper.contains(ComponentAsAClass)).to.equal(true)
}
})

itDoNotRunIf(
mountingMethod.name === 'renderToString',
'sets a component which can access the parent component and the child component',
() => {
const localVue = createLocalVue()
localVue.prototype.bar = 'FOO'
const ParentComponent = mount(
{
name: 'parentComponent',
template: '<div><slot /></div>',
data () {
return {
childComponentName: ''
}
}
},
{
components: {
ComponentWithParentName
},
slots: {
default: [
'<component-with-parent-name :fromLocalVue="bar" />',
'<component-with-parent-name :fromLocalVue="bar" />'
]
},
localVue
}
)
const childComponentName = 'component-with-parent-name'
expect(ParentComponent.vm.childComponentName).to.equal(childComponentName)
expect(ParentComponent.vm.$children.length).to.equal(2)
expect(ParentComponent.vm.$children.every(c => c.$options.name === childComponentName)).to.equal(true)
expect(ParentComponent.html()).to.equal('<div><div><span baz="qux">FOO,quux</span></div><div><span baz="qux">FOO,quux</span></div></div>')
}
)
})