From cfadb98011e188114bb822ee6f678cd09ddac7e3 Mon Sep 17 00:00:00 2001 From: Evan You Date: Mon, 10 Feb 2020 13:15:36 -0500 Subject: [PATCH] fix(runtime-core): rework vnode hooks handling - peroperly support directive on components (e.g. ) - consistently invoke raw vnode hooks on component vnodes (fix #684) --- .../runtime-core/__tests__/directives.spec.ts | 170 ++++++++++++++++-- packages/runtime-core/src/component.ts | 2 + packages/runtime-core/src/componentProps.ts | 22 ++- .../runtime-core/src/componentRenderUtils.ts | 36 +++- 4 files changed, 204 insertions(+), 26 deletions(-) diff --git a/packages/runtime-core/__tests__/directives.spec.ts b/packages/runtime-core/__tests__/directives.spec.ts index e8e64bb78ec..bfd6e439a46 100644 --- a/packages/runtime-core/__tests__/directives.spec.ts +++ b/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), [ [ - { - beforeMount, - mounted, - beforeUpdate, - updated, - beforeUnmount, - unmounted - }, + dir, // value count.value, // argument @@ -132,17 +134,17 @@ describe('directives', () => { const root = nodeOps.createElement('div') render(h(Comp), root) - expect(beforeMount).toHaveBeenCalled() - expect(mounted).toHaveBeenCalled() + expect(beforeMount).toHaveBeenCalledTimes(1) + expect(mounted).toHaveBeenCalledTimes(1) count.value++ await nextTick() - expect(beforeUpdate).toHaveBeenCalled() - expect(updated).toHaveBeenCalled() + expect(beforeUpdate).toHaveBeenCalledTimes(1) + expect(updated).toHaveBeenCalledTimes(1) render(null, root) - expect(beforeUnmount).toHaveBeenCalled() - expect(unmounted).toHaveBeenCalled() + expect(beforeUnmount).toHaveBeenCalledTimes(1) + 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) + }) }) diff --git a/packages/runtime-core/src/component.ts b/packages/runtime-core/src/component.ts index 3081ecad264..f7b12400fc1 100644 --- a/packages/runtime-core/src/component.ts +++ b/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, diff --git a/packages/runtime-core/src/componentProps.ts b/packages/runtime-core/src/componentProps.ts index baad88fd30c..a0d1b8046b1 100644 --- a/packages/runtime-core/src/componentProps.ts +++ b/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< diff --git a/packages/runtime-core/src/componentRenderUtils.ts b/packages/runtime-core/src/componentRenderUtils.ts index eb26c1d9359..33a3f57cd22 100644 --- a/packages/runtime-core/src/componentRenderUtils.ts +++ b/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 ( - __DEV__ && - !(result.shapeFlag & ShapeFlags.COMPONENT) && - !(result.shapeFlag & ShapeFlags.ELEMENT) && - result.type !== Comment - ) { + if (__DEV__ && !isElementRoot(result)) { warn( `Component inside 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 }