From 530ca1b2db315fbd0e360807b2031d26665c5d3d Mon Sep 17 00:00:00 2001 From: Evan You Date: Sat, 1 Dec 2018 21:07:18 -0500 Subject: [PATCH] fix(core): properly handle reused vnodes This also removes the restrictions on rendering the same slot multiple times. close #7913 --- .../instance/render-helpers/render-slot.js | 14 +--- src/core/instance/render.js | 8 --- src/core/vdom/patch.js | 26 +++++-- .../features/component/component-slot.spec.js | 70 ++++++++++--------- 4 files changed, 56 insertions(+), 62 deletions(-) diff --git a/src/core/instance/render-helpers/render-slot.js b/src/core/instance/render-helpers/render-slot.js index a58daa74e6..52221f4bbc 100644 --- a/src/core/instance/render-helpers/render-slot.js +++ b/src/core/instance/render-helpers/render-slot.js @@ -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 diff --git a/src/core/instance/render.js b/src/core/instance/render.js index be12d72603..def4be6db6 100644 --- a/src/core/instance/render.js +++ b/src/core/instance/render.js @@ -62,14 +62,6 @@ export function renderMixin (Vue: Class) { 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 } diff --git a/src/core/vdom/patch.js b/src/core/vdom/patch.js index 8857294b8f..7e58bced6a 100644 --- a/src/core/vdom/patch.js +++ b/src/core/vdom/patch.js @@ -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] @@ -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 { @@ -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)) { @@ -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 diff --git a/test/unit/features/component/component-slot.spec.js b/test/unit/features/component/component-slot.spec.js index 75b5254601..28dafad437 100644 --- a/test/unit/features/component/component-slot.spec.js +++ b/test/unit/features/component/component-slot.spec.js @@ -482,44 +482,46 @@ describe('Component slot', () => { }).then(done) }) - it('warn duplicate slots', () => { - new Vue({ - template: `
- -
foo
-
bar
- -
`, - components: { - test: { - template: `
- -
-
` - } - } - }).$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: `
- -
foo
- -
`, + it('should support duplicate slots', done => { + const vm = new Vue({ + template: ` + +
{{ n }}
+ + `, + data: { + n: 1 + }, components: { - test: { - template: `
- - -
` + foo: { + data() { + return { ok: true } + }, + template: ` +
+ + +
+
+ ` } } }).$mount() - expect('Duplicate presence of slot "default"').not.toHaveBeenWarned() + expect(vm.$el.innerHTML).toBe(`
1
1
1
`) + vm.n++ + waitForUpdate(() => { + expect(vm.$el.innerHTML).toBe(`
2
2
2
`) + vm.n++ + }).then(() => { + expect(vm.$el.innerHTML).toBe(`
3
3
3
`) + vm.$refs.foo.ok = false + }).then(() => { + expect(vm.$el.innerHTML).toBe(`
3
3
`) + vm.n++ + vm.$refs.foo.ok = true + }).then(() => { + expect(vm.$el.innerHTML).toBe(`
4
4
4
`) + }).then(done) }) // #3518