refactor: use more efficient on-demand clone to handle reused node edge cases

removes unnecessary slot/static node clones, fix #7292

Author: yyx990803

src/core/instance/lifecycle.js

@@ -239,7 +239,7 @@ export function updateChildComponent (
   // update $attrs and $listeners hash
   // these are also reactive so they may trigger child update if the child
   // used them during render
-  vm.$attrs = (parentVnode.data && parentVnode.data.attrs) || emptyObject
+  vm.$attrs = parentVnode.data.attrs || emptyObject
   vm.$listeners = listeners || emptyObject
 
   // update props
@@ -262,6 +262,7 @@ export function updateChildComponent (
     vm.$options._parentListeners = listeners
     updateComponentListeners(vm, listeners, oldListeners)
   }
+
   // resolve slots + force update if has children
   if (hasChildren) {
     vm.$slots = resolveSlots(renderChildren, parentVnode.context)

src/core/instance/render-helpers/render-static.js

@@ -1,7 +1,5 @@
 /* @flow */
 
-import { cloneVNode, cloneVNodes } from 'core/vdom/vnode'
-
 /**
  * Runtime helper for rendering static trees.
  */
@@ -12,11 +10,9 @@ export function renderStatic (
   const cached = this._staticTrees || (this._staticTrees = [])
   let tree = cached[index]
   // if has already-rendered static tree and not inside v-for,
-  // we can reuse the same tree by doing a shallow clone.
+  // we can reuse the same tree.
   if (tree && !isInFor) {
-    return Array.isArray(tree)
-      ? cloneVNodes(tree)
-      : cloneVNode(tree)
+    return tree
   }
   // otherwise, render a fresh tree.
   tree = cached[index] = this.$options.staticRenderFns[index].call(

src/core/instance/render.js

@@ -11,7 +11,7 @@ import {
 import { createElement } from '../vdom/create-element'
 import { installRenderHelpers } from './render-helpers/index'
 import { resolveSlots } from './render-helpers/resolve-slots'
-import VNode, { cloneVNodes, createEmptyVNode } from '../vdom/vnode'
+import VNode, { createEmptyVNode } from '../vdom/vnode'
 
 import { isUpdatingChildComponent } from './lifecycle'
 
@@ -62,20 +62,17 @@ export function renderMixin (Vue: Class<Component>) {
     const vm: Component = this
     const { render, _parentVnode } = vm.$options
 
-    if (vm._isMounted) {
-      // if the parent didn't update, the slot nodes will be the ones from
-      // last render. They need to be cloned to ensure "freshness" for this render.
+    // reset _rendered flag on slots for duplicate slot check
+    if (process.env.NODE_ENV !== 'production') {
       for (const key in vm.$slots) {
-        const slot = vm.$slots[key]
-        // _rendered is a flag added by renderSlot, but may not be present
-        // if the slot is passed from manually written render functions
-        if (slot._rendered || (slot[0] && slot[0].elm)) {
-          vm.$slots[key] = cloneVNodes(slot, true /* deep */)
-        }
+        // $flow-disable-line
+        vm.$slots[key]._rendered = false
       }
     }
 
-    vm.$scopedSlots = (_parentVnode && _parentVnode.data.scopedSlots) || emptyObject
+    if (_parentVnode) {
+      vm.$scopedSlots = _parentVnode.data.scopedSlots || emptyObject
+    }
 
     // set parent vnode. this allows render functions to have access
     // to the data on the placeholder node.

src/core/vdom/create-component.js

@@ -40,18 +40,22 @@ const componentVNodeHooks = {
     parentElm: ?Node,
     refElm: ?Node
   ): ?boolean {
-    if (!vnode.componentInstance || vnode.componentInstance._isDestroyed) {
+    if (
+      vnode.componentInstance &&
+      !vnode.componentInstance._isDestroyed &&
+      vnode.data.keepAlive
+    ) {
+      // kept-alive components, treat as a patch
+      const mountedNode: any = vnode // work around flow
+      componentVNodeHooks.prepatch(mountedNode, mountedNode)
+    } else {
       const child = vnode.componentInstance = createComponentInstanceForVnode(
         vnode,
         activeInstance,
         parentElm,
         refElm
       )
       child.$mount(hydrating ? vnode.elm : undefined, hydrating)
-    } else if (vnode.data.keepAlive) {
-      // kept-alive components, treat as a patch
-      const mountedNode: any = vnode // work around flow
-      componentVNodeHooks.prepatch(mountedNode, mountedNode)
     }
   },
 

src/core/vdom/create-element.js

@@ -3,12 +3,14 @@
 import config from '../config'
 import VNode, { createEmptyVNode } from './vnode'
 import { createComponent } from './create-component'
+import { traverse } from '../observer/traverse'
 
 import {
   warn,
   isDef,
   isUndef,
   isTrue,
+  isObject,
   isPrimitive,
   resolveAsset
 } from '../util/index'
@@ -116,10 +118,11 @@ export function _createElement (
     // direct component options / constructor
     vnode = createComponent(tag, data, context, children)
   }
-  if (isDef(vnode)) {
-    if (ns && !Array.isArray(vnode)) {
-      applyNS(vnode, ns)
-    }
+  if (Array.isArray(vnode)) {
+    return vnode
+  } else if (isDef(vnode)) {
+    if (isDef(ns)) applyNS(vnode, ns)
+    if (isDef(data)) registerDeepBindings(data)
     return vnode
   } else {
     return createEmptyVNode()
@@ -142,3 +145,15 @@ function applyNS (vnode, ns, force) {
     }
   }
 }
+
+// ref #5318
+// necessary to ensure parent re-render when deep bindings like :style and
+// :class are used on slot nodes
+function registerDeepBindings (data) {
+  if (isObject(data.style)) {
+    traverse(data.style)
+  }
+  if (isObject(data.class)) {
+    traverse(data.class)
+  }
+}

src/core/vdom/patch.js

@@ -10,7 +10,7 @@
  * of making flow understand it is not worth it.
  */
 
-import VNode from './vnode'
+import VNode, { cloneVNode } from './vnode'
 import config from '../config'
 import { SSR_ATTR } from 'shared/constants'
 import { registerRef } from './modules/ref'
@@ -121,7 +121,25 @@ export function createPatchFunction (backend) {
   }
 
   let creatingElmInVPre = 0
-  function createElm (vnode, insertedVnodeQueue, parentElm, refElm, nested) {
+
+  function createElm (
+    vnode,
+    insertedVnodeQueue,
+    parentElm,
+    refElm,
+    nested,
+    ownerArray,
+    index
+  ) {
+    if (isDef(vnode.elm) && isDef(ownerArray)) {
+      // This vnode was used in a previous render!
+      // now it's used as a new node, overwriting its elm would cause
+      // potential patch errors down the road when it's used as an insertion
+      // reference node. Instead, we clone the node on-demand before creating
+      // associated DOM element for it.
+      vnode = ownerArray[index] = cloneVNode(vnode)
+    }
+
     vnode.isRootInsert = !nested // for transition enter check
     if (createComponent(vnode, insertedVnodeQueue, parentElm, refElm)) {
       return
@@ -144,6 +162,7 @@ export function createPatchFunction (backend) {
           )
         }
       }
+
       vnode.elm = vnode.ns
         ? nodeOps.createElementNS(vnode.ns, tag)
         : nodeOps.createElement(tag, vnode)
@@ -267,7 +286,7 @@ export function createPatchFunction (backend) {
         checkDuplicateKeys(children)
       }
       for (let i = 0; i < children.length; ++i) {
-        createElm(children[i], insertedVnodeQueue, vnode.elm, null, true)
+        createElm(children[i], insertedVnodeQueue, vnode.elm, null, true, children, i)
       }
     } else if (isPrimitive(vnode.text)) {
       nodeOps.appendChild(vnode.elm, nodeOps.createTextNode(String(vnode.text)))
@@ -320,7 +339,7 @@ export function createPatchFunction (backend) {
 
   function addVnodes (parentElm, refElm, vnodes, startIdx, endIdx, insertedVnodeQueue) {
     for (; startIdx <= endIdx; ++startIdx) {
-      createElm(vnodes[startIdx], insertedVnodeQueue, parentElm, refElm)
+      createElm(vnodes[startIdx], insertedVnodeQueue, parentElm, refElm, false, vnodes, startIdx)
     }
   }
 
@@ -430,7 +449,7 @@ export function createPatchFunction (backend) {
           ? oldKeyToIdx[newStartVnode.key]
           : findIdxInOld(newStartVnode, oldCh, oldStartIdx, oldEndIdx)
         if (isUndef(idxInOld)) { // New element
-          createElm(newStartVnode, insertedVnodeQueue, parentElm, oldStartVnode.elm)
+          createElm(newStartVnode, insertedVnodeQueue, parentElm, oldStartVnode.elm, false, newCh, newStartIdx)
         } else {
           vnodeToMove = oldCh[idxInOld]
           if (sameVnode(vnodeToMove, newStartVnode)) {
@@ -439,7 +458,7 @@ export function createPatchFunction (backend) {
             canMove && nodeOps.insertBefore(parentElm, vnodeToMove.elm, oldStartVnode.elm)
           } else {
             // same key but different element. treat as new element
-            createElm(newStartVnode, insertedVnodeQueue, parentElm, oldStartVnode.elm)
+            createElm(newStartVnode, insertedVnodeQueue, parentElm, oldStartVnode.elm, false, newCh, newStartIdx)
           }
         }
         newStartVnode = newCh[++newStartIdx]

src/core/vdom/vnode.js

@@ -85,16 +85,15 @@ export function createTextVNode (val: string | number) {
 // used for static nodes and slot nodes because they may be reused across
 // multiple renders, cloning them avoids errors when DOM manipulations rely
 // on their elm reference.
-export function cloneVNode (vnode: VNode, deep?: boolean): VNode {
-  const componentOptions = vnode.componentOptions
+export function cloneVNode (vnode: VNode): VNode {
   const cloned = new VNode(
     vnode.tag,
     vnode.data,
     vnode.children,
     vnode.text,
     vnode.elm,
     vnode.context,
-    componentOptions,
+    vnode.componentOptions,
     vnode.asyncFactory
   )
   cloned.ns = vnode.ns
@@ -105,22 +104,5 @@ export function cloneVNode (vnode: VNode, deep?: boolean): VNode {
   cloned.fnOptions = vnode.fnOptions
   cloned.fnScopeId = vnode.fnScopeId
   cloned.isCloned = true
-  if (deep) {
-    if (vnode.children) {
-      cloned.children = cloneVNodes(vnode.children, true)
-    }
-    if (componentOptions && componentOptions.children) {
-      componentOptions.children = cloneVNodes(componentOptions.children, true)
-    }
-  }
   return cloned
 }
-
-export function cloneVNodes (vnodes: Array<VNode>, deep?: boolean): Array<VNode> {
-  const len = vnodes.length
-  const res = new Array(len)
-  for (let i = 0; i < len; i++) {
-    res[i] = cloneVNode(vnodes[i], deep)
-  }
-  return res
-}

test/unit/modules/vdom/patch/edge-cases.spec.js

@@ -26,7 +26,7 @@ describe('vdom patch: edge cases', () => {
   })
 
   // #3533
-  // a static node (<br>) is reused in createElm, which changes its elm reference
+  // a static node is reused in createElm, which changes its elm reference
   // and is inserted into a different parent.
   // later when patching the next element a DOM insertion uses it as the
   // reference node, causing a parent mismatch.
@@ -40,11 +40,12 @@ describe('vdom patch: edge cases', () => {
           <button @click="ok = !ok">toggle</button>
           <div class="b" v-if="ok">123</div>
           <div class="c">
-            <br><p>{{ 1 }}</p>
+            <div><span/></div><p>{{ 1 }}</p>
           </div>
           <div class="d">
             <label>{{ 2 }}</label>
           </div>
+          <div class="b" v-if="ok">123</div>
         </div>
       `
     }).$mount()
@@ -58,6 +59,43 @@ describe('vdom patch: edge cases', () => {
     }).then(done)
   })
 
+  it('should handle slot nodes being reused across render', done => {
+    const vm = new Vue({
+      template: `
+        <foo ref="foo">
+          <div>slot</div>
+        </foo>
+      `,
+      components: {
+        foo: {
+          data () {
+            return { ok: true }
+          },
+          render (h) {
+            const children = [
+              this.ok ? h('div', 'toggler ') : null,
+              h('div', [this.$slots.default, h('span', ' 1')]),
+              h('div', [h('label', ' 2')])
+            ]
+            return h('div', children)
+          }
+        }
+      }
+    }).$mount()
+    expect(vm.$el.textContent).toContain('toggler slot 1 2')
+    vm.$refs.foo.ok = false
+    waitForUpdate(() => {
+      expect(vm.$el.textContent).toContain('slot 1 2')
+      vm.$refs.foo.ok = true
+    }).then(() => {
+      expect(vm.$el.textContent).toContain('toggler slot 1 2')
+      vm.$refs.foo.ok = false
+    }).then(() => {
+      expect(vm.$el.textContent).toContain('slot 1 2')
+      vm.$refs.foo.ok = true
+    }).then(done)
+  })
+
   it('should synchronize vm\' vnode', done => {
     const comp = {
       data: () => ({ swap: true }),