Skip to content

Commit

Permalink
fix(core): properly handle reused vnodes
Browse files Browse the repository at this point in the history
This also removes the restrictions on rendering the same slot multiple
times.

close #7913
  • Loading branch information
yyx990803 committed Dec 2, 2018
1 parent 097f622 commit 530ca1b
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 62 deletions.
14 changes: 1 addition & 13 deletions src/core/instance/render-helpers/render-slot.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,7 @@ export function renderSlot (
}
nodes = scopedSlotFn(props) || fallback
} else {
const slotNodes = this.$slots[name]
// warn duplicate slot usage
if (slotNodes) {
if (process.env.NODE_ENV !== 'production' && slotNodes._rendered) {
warn(
`Duplicate presence of slot "${name}" found in the same render tree ` +
`- this will likely cause render errors.`,
this
)
}
slotNodes._rendered = true
}
nodes = slotNodes || fallback
nodes = this.$slots[name] || fallback
}

const target = props && props.slot
Expand Down
8 changes: 0 additions & 8 deletions src/core/instance/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,6 @@ export function renderMixin (Vue: Class<Component>) {
const vm: Component = this
const { render, _parentVnode } = vm.$options

// reset _rendered flag on slots for duplicate slot check
if (process.env.NODE_ENV !== 'production') {
for (const key in vm.$slots) {
// $flow-disable-line
vm.$slots[key]._rendered = false
}
}

if (_parentVnode) {
vm.$scopedSlots = _parentVnode.data.scopedSlots || emptyObject
}
Expand Down
26 changes: 19 additions & 7 deletions src/core/vdom/patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,20 +434,20 @@ export function createPatchFunction (backend) {
} else if (isUndef(oldEndVnode)) {
oldEndVnode = oldCh[--oldEndIdx]
} else if (sameVnode(oldStartVnode, newStartVnode)) {
patchVnode(oldStartVnode, newStartVnode, insertedVnodeQueue)
patchVnode(oldStartVnode, newStartVnode, insertedVnodeQueue, newCh, newStartIdx)
oldStartVnode = oldCh[++oldStartIdx]
newStartVnode = newCh[++newStartIdx]
} else if (sameVnode(oldEndVnode, newEndVnode)) {
patchVnode(oldEndVnode, newEndVnode, insertedVnodeQueue)
patchVnode(oldEndVnode, newEndVnode, insertedVnodeQueue, newCh, newEndIdx)
oldEndVnode = oldCh[--oldEndIdx]
newEndVnode = newCh[--newEndIdx]
} else if (sameVnode(oldStartVnode, newEndVnode)) { // Vnode moved right
patchVnode(oldStartVnode, newEndVnode, insertedVnodeQueue)
patchVnode(oldStartVnode, newEndVnode, insertedVnodeQueue, newCh, newEndIdx)
canMove && nodeOps.insertBefore(parentElm, oldStartVnode.elm, nodeOps.nextSibling(oldEndVnode.elm))
oldStartVnode = oldCh[++oldStartIdx]
newEndVnode = newCh[--newEndIdx]
} else if (sameVnode(oldEndVnode, newStartVnode)) { // Vnode moved left
patchVnode(oldEndVnode, newStartVnode, insertedVnodeQueue)
patchVnode(oldEndVnode, newStartVnode, insertedVnodeQueue, newCh, newStartIdx)
canMove && nodeOps.insertBefore(parentElm, oldEndVnode.elm, oldStartVnode.elm)
oldEndVnode = oldCh[--oldEndIdx]
newStartVnode = newCh[++newStartIdx]
Expand All @@ -461,7 +461,7 @@ export function createPatchFunction (backend) {
} else {
vnodeToMove = oldCh[idxInOld]
if (sameVnode(vnodeToMove, newStartVnode)) {
patchVnode(vnodeToMove, newStartVnode, insertedVnodeQueue)
patchVnode(vnodeToMove, newStartVnode, insertedVnodeQueue, newCh, newStartIdx)
oldCh[idxInOld] = undefined
canMove && nodeOps.insertBefore(parentElm, vnodeToMove.elm, oldStartVnode.elm)
} else {
Expand Down Expand Up @@ -505,11 +505,23 @@ export function createPatchFunction (backend) {
}
}

function patchVnode (oldVnode, vnode, insertedVnodeQueue, removeOnly) {
function patchVnode (
oldVnode,
vnode,
insertedVnodeQueue,
ownerArray,
index,
removeOnly
) {
if (oldVnode === vnode) {
return
}

if (isDef(vnode.elm) && isDef(ownerArray)) {
// clone reused vnode
vnode = ownerArray[index] = cloneVNode(vnode)
}

const elm = vnode.elm = oldVnode.elm

if (isTrue(oldVnode.isAsyncPlaceholder)) {
Expand Down Expand Up @@ -709,7 +721,7 @@ export function createPatchFunction (backend) {
const isRealElement = isDef(oldVnode.nodeType)
if (!isRealElement && sameVnode(oldVnode, vnode)) {
// patch existing root node
patchVnode(oldVnode, vnode, insertedVnodeQueue, removeOnly)
patchVnode(oldVnode, vnode, insertedVnodeQueue, null, null, removeOnly)
} else {
if (isRealElement) {
// mounting to a real element
Expand Down
70 changes: 36 additions & 34 deletions test/unit/features/component/component-slot.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -482,44 +482,46 @@ describe('Component slot', () => {
}).then(done)
})

it('warn duplicate slots', () => {
new Vue({
template: `<div>
<test>
<div>foo</div>
<div slot="a">bar</div>
</test>
</div>`,
components: {
test: {
template: `<div>
<slot></slot><slot></slot>
<div v-for="i in 3"><slot name="a"></slot></div>
</div>`
}
}
}).$mount()
expect('Duplicate presence of slot "default"').toHaveBeenWarned()
expect('Duplicate presence of slot "a"').toHaveBeenWarned()
})

it('should not warn valid conditional slots', () => {
new Vue({
template: `<div>
<test>
<div>foo</div>
</test>
</div>`,
it('should support duplicate slots', done => {
const vm = new Vue({
template: `
<foo ref="foo">
<div slot="a">{{ n }}</div>
</foo>
`,
data: {
n: 1
},
components: {
test: {
template: `<div>
<slot v-if="true"></slot>
<slot v-else></slot>
</div>`
foo: {
data() {
return { ok: true }
},
template: `
<div>
<slot name="a" />
<slot v-if="ok" name="a" />
<pre><slot name="a" /></pre>
</div>
`
}
}
}).$mount()
expect('Duplicate presence of slot "default"').not.toHaveBeenWarned()
expect(vm.$el.innerHTML).toBe(`<div>1</div> <div>1</div> <pre><div>1</div></pre>`)
vm.n++
waitForUpdate(() => {
expect(vm.$el.innerHTML).toBe(`<div>2</div> <div>2</div> <pre><div>2</div></pre>`)
vm.n++
}).then(() => {
expect(vm.$el.innerHTML).toBe(`<div>3</div> <div>3</div> <pre><div>3</div></pre>`)
vm.$refs.foo.ok = false
}).then(() => {
expect(vm.$el.innerHTML).toBe(`<div>3</div> <!----> <pre><div>3</div></pre>`)
vm.n++
vm.$refs.foo.ok = true
}).then(() => {
expect(vm.$el.innerHTML).toBe(`<div>4</div> <div>4</div> <pre><div>4</div></pre>`)
}).then(done)
})

// #3518
Expand Down

0 comments on commit 530ca1b

Please sign in to comment.