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 mounting option #821

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion packages/create-instance/create-functional-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

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,
Expand Down
2 changes: 1 addition & 1 deletion packages/create-instance/create-instance.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// @flow

import { createSlotVNodes } from './add-slots'
import addMocks from './add-mocks'
import { addEventLogger } from './log-events'
import { createComponentStubs } from 'shared/stub-components'
import { createSlotVNodes } from './create-slot-vnodes'
import { throwError, warn, vueVersion } from 'shared/util'
import { compileTemplate } from 'shared/compile-template'
import extractInstanceOptions from './extract-instance-options'
Expand Down
62 changes: 62 additions & 0 deletions packages/create-instance/create-render-slot.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// @flow

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

const _renderSlot = Vue.prototype._t

function createVNodes (
vm: Component,
slotValue: Component | string
): ?Array<VNode> {
if (typeof slotValue === 'string') {
const compiledResult = compileToFunctions(`<div>${slotValue}{{ }}</div>`)
Copy link
Member

Choose a reason for hiding this comment

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

Why the empty interpolation {{ }}?

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 do not understand.
It is not necessary.
I thought the compileToFunctions raises an error if it dose not exist at #274.

const _staticRenderFns = vm._renderProxy.$options.staticRenderFns
vm._renderProxy.$options.staticRenderFns = compiledResult.staticRenderFns
Copy link
Member

Choose a reason for hiding this comment

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

Why do you save the renderFunctions and then reassign?

const vnodes = compiledResult.render.call(
vm._renderProxy, vm.$createElement
).children
vm._renderProxy.$options.staticRenderFns = _staticRenderFns
return vnodes
}
return [vm.$createElement(slotValue)]
}

export default function createRenderSlot (
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of replacing the renderSlot alias? Does this solve the issue of parent being undefined?

options: Object
): (
name: string,
fallback: ?Array<VNode>,
props: ?Object,
bindObject: ?Object
) => ?Array<VNode> {
return function renderSlot (
name: string,
fallback: ?Array<VNode>,
props: ?Object,
bindObject: ?Object
): ?Array<VNode> {
if (options.slots && options.slots[name]) {
this.$slots[name] = []
const slotsValue = options.slots[name]
if (Array.isArray(slotsValue)) {
slotsValue.forEach((value) => {
if (typeof value === 'string') {
const vnodes = createVNodes(this, value)
if (Array.isArray(vnodes)) {
this.$slots[name].push(...vnodes)
}
} else {
this.$slots[name].push(this.$createElement(value))
}
})
} else {
const vnodes = createVNodes(this, slotsValue)
if (Array.isArray(vnodes)) {
this.$slots[name] = vnodes
}
}
}
return _renderSlot.call(this, name, fallback, props, bindObject)
}
}
6 changes: 6 additions & 0 deletions packages/create-instance/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// @flow

import createInstance from './create-instance'
import createRenderSlot from './create-render-slot'

export { createInstance, createRenderSlot }
2 changes: 1 addition & 1 deletion packages/create-instance/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "create-instance",
"version": "1.0.0-beta.20",
"main": "create-instance.js",
"main": "index.js",
"private": true
}
4 changes: 3 additions & 1 deletion packages/server-test-utils/src/renderToString.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @flow

import Vue from 'vue'
import createInstance from 'create-instance'
import { createInstance, createRenderSlot } from 'create-instance'
import { throwError } from 'shared/util'
import { createRenderer } from 'vue-server-renderer'
import testUtils from '@vue/test-utils'
Expand Down Expand Up @@ -29,6 +29,8 @@ export default function renderToString (
throwError(`you cannot use attachToDocument with ` + `renderToString`)
}
const vueConstructor = testUtils.createLocalVue(options.localVue)
vueConstructor.prototype._t = createRenderSlot(options)

const vm = createInstance(
component,
mergeOptions(options, config),
Expand Down
3 changes: 2 additions & 1 deletion packages/test-utils/src/mount.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import './matches-polyfill'
import './object-assign-polyfill'
import Vue from 'vue'
import VueWrapper from './vue-wrapper'
import createInstance from 'create-instance'
import createElement from './create-element'
import createLocalVue from './create-local-vue'
import errorHandler from './error-handler'
import { findAllVueComponentsFromVm } from './find-vue-components'
import { mergeOptions } from 'shared/merge-options'
import config from './config'
import warnIfNoWindow from './warn-if-no-window'
import { createInstance, createRenderSlot } from 'create-instance'

Vue.config.productionTip = false
Vue.config.devtools = false
Expand All @@ -28,6 +28,7 @@ export default function mount (
// Remove cached constructor
delete component._Ctor
const vueConstructor = createLocalVue(options.localVue)
vueConstructor.prototype._t = createRenderSlot(options)

const elm = options.attachToDocument ? createElement() : undefined

Expand Down
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">{{ time }},{{ fromLocalVue }},{{ bar }}</span></div>
</template>

<script>
export default{
name: 'component-with-parent-name',
props: ['fromLocalVue', 'time'],
data () {
return {
bar: 'quux'
}
},
mounted () {
this.$parent.childComponentName = this.$options.name
}
}
</script>
82 changes: 78 additions & 4 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 @@ -224,14 +226,18 @@ describeWithMountingMethods('options.slots', mountingMethod => {
it('mounts component with text slot', () => {
const wrapper = mountingMethod(ComponentWithSlots, {
slots: {
default: 'hello,',
header: 'world'
header: 'hello,',
default: 'world'
}
})
if (mountingMethod.name === 'renderToString') {
expect(wrapper).contains('hello,world')
expect(wrapper).contains(
'<div data-server-rendered="true" class="container"><header>hello,</header> <main>world</main> <footer></footer></div>'
)
} else {
expect(wrapper.text()).to.contain('hello,world')
expect(wrapper.html()).to.equal(
'<div class="container"><header>hello,</header> <main>world</main> <footer></footer></div>'
)
}
})

Expand Down Expand Up @@ -568,4 +574,72 @@ describeWithMountingMethods('options.slots', mountingMethod => {
expect(wrapper.contains(ComponentAsAClass)).to.equal(true)
}
})

itDoNotRunIf(
mountingMethod.name === 'renderToString',
'mounts component with default slot if passed string in slot object',
() => {
const wrapper1 = mount(ComponentWithSlots, { slots: { default: 'foo<span>123</span>{{ foo }}' }})
expect(wrapper1.find('main').html()).to.equal('<main>foo<span>123</span>bar</main>')
const wrapper2 = mount(ComponentWithSlots, { slots: { default: '<p>1</p>{{ foo }}2' }})
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is how slots should behave. foo should not be resolved using the child vm that it's rendered in. If it uses any instance to resolve foo on, it should be the root instance. But I don't think we should support any variables in the slots option as it adds too much complexity

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.
Your pointing out is correct.
I misunderstood about it.

expect(wrapper2.find('main').html()).to.equal('<main><p>1</p>bar2</main>')
const wrapper3 = mount(ComponentWithSlots, { slots: { default: '<p>1</p>{{ foo }}<p>2</p>' }})
expect(wrapper3.find('main').html()).to.equal('<main><p>1</p>bar<p>2</p></main>')
const wrapper4 = mount(ComponentWithSlots, { slots: { default: '123' }})
expect(wrapper4.find('main').html()).to.equal('<main>123</main>')
const wrapper5 = mount(ComponentWithSlots, { slots: { default: '1{{ foo }}2' }})
expect(wrapper5.find('main').html()).to.equal('<main>1bar2</main>')
wrapper5.trigger('keydown')
expect(wrapper5.find('main').html()).to.equal('<main>1BAR2</main>')
const wrapper6 = mount(ComponentWithSlots, { slots: { default: '<p>1</p><p>2</p>' }})
expect(wrapper6.find('main').html()).to.equal('<main><p>1</p><p>2</p></main>')
const wrapper7 = mount(ComponentWithSlots, { slots: { default: '1<p>2</p>3' }})
expect(wrapper7.find('main').html()).to.equal('<main>1<p>2</p>3</main>')
const wrapper8 = mountingMethod(ComponentWithSlots, { slots: { default: ' space ' }})
expect(wrapper8.find('main').html()).to.equal('<main> space </main>')
}
)

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 {
time: 1,
childComponentName: ''
}
}
},
{
components: {
ComponentWithParentName
},
slots: {
default: [
'<component-with-parent-name :fromLocalVue="bar" :time="time" />',
'<component-with-parent-name :fromLocalVue="bar" :time="time" />'
]
},
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">1,FOO,quux</span></div><div><span baz="qux">1,FOO,quux</span></div></div>')
ParentComponent.vm.time = 2
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">2,FOO,quux</span></div><div><span baz="qux">2,FOO,quux</span></div></div>')
}
)
})