fix(runtime-core): rework vnode hooks handling

- peroperly support directive on components (e.g. <foo v-show="x">)
- consistently invoke raw vnode hooks on component vnodes (fix #684)

packages/runtime-core/__tests__/directives.spec.ts

@@ -98,6 +98,15 @@ describe('directives', () => {
       expect(prevVNode).toBe(null)
     }) as DirectiveHook)
 
+    const dir = {
+      beforeMount,
+      mounted,
+      beforeUpdate,
+      updated,
+      beforeUnmount,
+      unmounted
+    }
+
     let _instance: ComponentInternalInstance | null = null
     let _vnode: VNode | null = null
     let _prevVnode: VNode | null = null
@@ -109,14 +118,7 @@ describe('directives', () => {
         _prevVnode = _vnode
         _vnode = withDirectives(h('div', count.value), [
           [
-            {
+            dir,
-              beforeMount,
-              mounted,
-              beforeUpdate,
-              updated,
-              beforeUnmount,
-              unmounted
-            },
             // value
             count.value,
             // argument
@@ -132,17 +134,17 @@ describe('directives', () => {
     const root = nodeOps.createElement('div')
     render(h(Comp), root)
 
-    expect(beforeMount).toHaveBeenCalled()
+    expect(beforeMount).toHaveBeenCalledTimes(1)
-    expect(mounted).toHaveBeenCalled()
+    expect(mounted).toHaveBeenCalledTimes(1)
 
     count.value++
     await nextTick()
-    expect(beforeUpdate).toHaveBeenCalled()
+    expect(beforeUpdate).toHaveBeenCalledTimes(1)
-    expect(updated).toHaveBeenCalled()
+    expect(updated).toHaveBeenCalledTimes(1)
 
     render(null, root)
-    expect(beforeUnmount).toHaveBeenCalled()
+    expect(beforeUnmount).toHaveBeenCalledTimes(1)
-    expect(unmounted).toHaveBeenCalled()
+    expect(unmounted).toHaveBeenCalledTimes(1)
   })
 
   it('should work with a function directive', async () => {
@@ -198,4 +200,144 @@ describe('directives', () => {
     await nextTick()
     expect(fn).toHaveBeenCalledTimes(2)
   })
+
+  it('should work on component vnode', async () => {
+    const count = ref(0)
+
+    function assertBindings(binding: DirectiveBinding) {
+      expect(binding.value).toBe(count.value)
+      expect(binding.arg).toBe('foo')
+      expect(binding.instance).toBe(_instance && _instance.proxy)
+      expect(binding.modifiers && binding.modifiers.ok).toBe(true)
+    }
+
+    const beforeMount = jest.fn(((el, binding, vnode, prevVNode) => {
+      expect(el.tag).toBe('div')
+      // should not be inserted yet
+      expect(el.parentNode).toBe(null)
+      expect(root.children.length).toBe(0)
+
+      assertBindings(binding)
+
+      expect(vnode.type).toBe(_vnode!.type)
+      expect(prevVNode).toBe(null)
+    }) as DirectiveHook)
+
+    const mounted = jest.fn(((el, binding, vnode, prevVNode) => {
+      expect(el.tag).toBe('div')
+      // should be inserted now
+      expect(el.parentNode).toBe(root)
+      expect(root.children[0]).toBe(el)
+
+      assertBindings(binding)
+
+      expect(vnode.type).toBe(_vnode!.type)
+      expect(prevVNode).toBe(null)
+    }) as DirectiveHook)
+
+    const beforeUpdate = jest.fn(((el, binding, vnode, prevVNode) => {
+      expect(el.tag).toBe('div')
+      expect(el.parentNode).toBe(root)
+      expect(root.children[0]).toBe(el)
+
+      // node should not have been updated yet
+      // expect(el.children[0].text).toBe(`${count.value - 1}`)
+
+      assertBindings(binding)
+
+      expect(vnode.type).toBe(_vnode!.type)
+      expect(prevVNode!.type).toBe(_prevVnode!.type)
+    }) as DirectiveHook)
+
+    const updated = jest.fn(((el, binding, vnode, prevVNode) => {
+      expect(el.tag).toBe('div')
+      expect(el.parentNode).toBe(root)
+      expect(root.children[0]).toBe(el)
+
+      // node should have been updated
+      expect(el.children[0].text).toBe(`${count.value}`)
+
+      assertBindings(binding)
+
+      expect(vnode.type).toBe(_vnode!.type)
+      expect(prevVNode!.type).toBe(_prevVnode!.type)
+    }) as DirectiveHook)
+
+    const beforeUnmount = jest.fn(((el, binding, vnode, prevVNode) => {
+      expect(el.tag).toBe('div')
+      // should be removed now
+      expect(el.parentNode).toBe(root)
+      expect(root.children[0]).toBe(el)
+
+      assertBindings(binding)
+
+      expect(vnode.type).toBe(_vnode!.type)
+      expect(prevVNode).toBe(null)
+    }) as DirectiveHook)
+
+    const unmounted = jest.fn(((el, binding, vnode, prevVNode) => {
+      expect(el.tag).toBe('div')
+      // should have been removed
+      expect(el.parentNode).toBe(null)
+      expect(root.children.length).toBe(0)
+
+      assertBindings(binding)
+
+      expect(vnode.type).toBe(_vnode!.type)
+      expect(prevVNode).toBe(null)
+    }) as DirectiveHook)
+
+    const dir = {
+      beforeMount,
+      mounted,
+      beforeUpdate,
+      updated,
+      beforeUnmount,
+      unmounted
+    }
+
+    let _instance: ComponentInternalInstance | null = null
+    let _vnode: VNode | null = null
+    let _prevVnode: VNode | null = null
+
+    const Child = (props: { count: number }) => {
+      _prevVnode = _vnode
+      _vnode = h('div', props.count)
+      return _vnode
+    }
+
+    const Comp = {
+      setup() {
+        _instance = currentInstance
+      },
+      render() {
+        return withDirectives(h(Child, { count: count.value }), [
+          [
+            dir,
+            // value
+            count.value,
+            // argument
+            'foo',
+            // modifiers
+            { ok: true }
+          ]
+        ])
+      }
+    }
+
+    const root = nodeOps.createElement('div')
+    render(h(Comp), root)
+
+    expect(beforeMount).toHaveBeenCalledTimes(1)
+    expect(mounted).toHaveBeenCalledTimes(1)
+
+    count.value++
+    await nextTick()
+    expect(beforeUpdate).toHaveBeenCalledTimes(1)
+    expect(updated).toHaveBeenCalledTimes(1)
+
+    render(null, root)
+    expect(beforeUnmount).toHaveBeenCalledTimes(1)
+    expect(unmounted).toHaveBeenCalledTimes(1)
+  })
 })

packages/runtime-core/src/component.ts

@@ -113,6 +113,7 @@ export interface ComponentInternalInstance {
   data: Data
   props: Data
   attrs: Data
+  vnodeHooks: Data
   slots: Slots
   proxy: ComponentPublicInstance | null
   // alternative proxy used only for runtime-compiled render functions using
@@ -186,6 +187,7 @@ export function createComponentInstance(
     data: EMPTY_OBJ,
     props: EMPTY_OBJ,
     attrs: EMPTY_OBJ,
+    vnodeHooks: EMPTY_OBJ,
     slots: EMPTY_OBJ,
     refs: EMPTY_OBJ,
 

packages/runtime-core/src/componentProps.ts

@@ -11,7 +11,8 @@ import {
   hasOwn,
   toRawType,
   PatchFlags,
-  makeMap
+  makeMap,
+  isReservedProp
 } from '@vue/shared'
 import { warn } from './warning'
 import { Data, ComponentInternalInstance } from './component'
@@ -104,7 +105,8 @@ export function resolveProps(
 
   const { 0: options, 1: needCastKeys } = normalizePropsOptions(_options)!
   const props: Data = {}
-  let attrs: Data | undefined = void 0
+  let attrs: Data | undefined = undefined
+  let vnodeHooks: Data | undefined = undefined
 
   // update the instance propsProxy (passed to setup()) to trigger potential
   // changes
@@ -123,21 +125,28 @@ export function resolveProps(
 
   if (rawProps != null) {
     for (const key in rawProps) {
+      const value = rawProps[key]
       // key, ref are reserved and never passed down
-      if (key === 'key' || key === 'ref') continue
+      if (isReservedProp(key)) {
+        if (key !== 'key' && key !== 'ref') {
+          // vnode hooks.
+          ;(vnodeHooks || (vnodeHooks = {}))[key] = value
+        }
+        continue
+      }
       // prop option names are camelized during normalization, so to support
       // kebab -> camel conversion here we need to camelize the key.
       if (hasDeclaredProps) {
         const camelKey = camelize(key)
         if (hasOwn(options, camelKey)) {
-          setProp(camelKey, rawProps[key])
+          setProp(camelKey, value)
         } else {
           // Any non-declared props are put into a separate `attrs` object
           // for spreading. Make sure to preserve original key casing
-          ;(attrs || (attrs = {}))[key] = rawProps[key]
+          ;(attrs || (attrs = {}))[key] = value
         }
       } else {
-        setProp(key, rawProps[key])
+        setProp(key, value)
       }
     }
   }
@@ -206,6 +215,7 @@ export function resolveProps(
 
   instance.props = props
   instance.attrs = options ? attrs || EMPTY_OBJ : props
+  instance.vnodeHooks = vnodeHooks || EMPTY_OBJ
 }
 
 const normalizationMap = new WeakMap<

packages/runtime-core/src/componentRenderUtils.ts

@@ -46,6 +46,7 @@ export function renderComponentRoot(
     props,
     slots,
     attrs,
+    vnodeHooks,
     emit
   } = instance
 
@@ -92,14 +93,23 @@ export function renderComponentRoot(
       }
     }
 
+    // inherit vnode hooks
+    if (vnodeHooks !== EMPTY_OBJ) {
+      result = cloneVNode(result, vnodeHooks)
+    }
+    // inherit directives
+    if (vnode.dirs != null) {
+      if (__DEV__ && !isElementRoot(result)) {
+        warn(
+          `Runtime directive used on component with non-element root node. ` +
+            `The directives will not function as intended.`
+        )
+      }
+      result.dirs = vnode.dirs
+    }
     // inherit transition data
     if (vnode.transition != null) {
-      if (
+      if (__DEV__ && !isElementRoot(result)) {
-        __DEV__ &&
-        !(result.shapeFlag & ShapeFlags.COMPONENT) &&
-        !(result.shapeFlag & ShapeFlags.ELEMENT) &&
-        result.type !== Comment
-      ) {
         warn(
           `Component inside <Transition> renders non-element root node ` +
             `that cannot be animated.`
@@ -115,6 +125,14 @@ export function renderComponentRoot(
   return result
 }
 
+function isElementRoot(vnode: VNode) {
+  return (
+    vnode.shapeFlag & ShapeFlags.COMPONENT ||
+    vnode.shapeFlag & ShapeFlags.ELEMENT ||
+    vnode.type === Comment // potential v-if branch switch
+  )
+}
+
 export function shouldUpdateComponent(
   prevVNode: VNode,
   nextVNode: VNode,
@@ -137,6 +155,11 @@ export function shouldUpdateComponent(
     return true
   }
 
+  // force child update on runtime directive usage on component vnode.
+  if (nextVNode.dirs != null) {
+    return true
+  }
+
   if (patchFlag > 0) {
     if (patchFlag & PatchFlags.DYNAMIC_SLOTS) {
       // slot content that references values that might have changed,
@@ -174,6 +197,7 @@ export function shouldUpdateComponent(
     }
     return hasPropsChanged(prevProps, nextProps)
   }
+
   return false
 }