fix(compiler): force block for custom dirs and inline beforeUpdate hooks

to ensure they are called before children updates

Author: yyx990803

packages/compiler-core/__tests__/transforms/__snapshots__/transformText.spec.ts.snap

@@ -67,13 +67,13 @@ exports[`compiler: transform text element with custom directives and only one te
 
 return function render(_ctx, _cache) {
   with (_ctx) {
-    const { toDisplayString: _toDisplayString, createTextVNode: _createTextVNode, resolveDirective: _resolveDirective, withDirectives: _withDirectives, openBlock: _openBlock, createElementBlock: _createElementBlock } = _Vue
+    const { toDisplayString: _toDisplayString, createTextVNode: _createTextVNode, resolveDirective: _resolveDirective, openBlock: _openBlock, createElementBlock: _createElementBlock, withDirectives: _withDirectives } = _Vue
 
     const _directive_foo = _resolveDirective(\\"foo\\")
 
     return _withDirectives((_openBlock(), _createElementBlock(\\"p\\", null, [
       _createTextVNode(_toDisplayString(foo), 1 /* TEXT */)
-    ], 512 /* NEED_PATCH */)), [
+    ])), [
       [_directive_foo]
     ])
   }

packages/compiler-core/__tests__/transforms/transformElement.spec.ts

@@ -1202,6 +1202,23 @@ describe('compiler: element transform', () => {
     })
   })
 
+  test('force block for runtime custom directive w/ children', () => {
+    const { node } = parseWithElementTransform(`<div v-foo>hello</div>`)
+    expect(node.isBlock).toBe(true)
+  })
+
+  test('force block for inline before-update handlers w/ children', () => {
+    expect(
+      parseWithElementTransform(`<div @vnode-before-update>hello</div>`).node
+        .isBlock
+    ).toBe(true)
+
+    expect(
+      parseWithElementTransform(`<div @vnodeBeforeUpdate>hello</div>`).node
+        .isBlock
+    ).toBe(true)
+  })
+
   // #938
   test('element with dynamic keys should be forced into blocks', () => {
     const ast = parse(`<div><div :key="foo" /></div>`)

packages/compiler-core/src/parse.ts

@@ -11,7 +11,7 @@ import {
   advancePositionWithMutation,
   advancePositionWithClone,
   isCoreComponent,
-  isBindKey
+  isStaticArgOf
 } from './utils'
 import {
   Namespaces,
@@ -681,7 +681,7 @@ function isComponent(
       } else if (
         // :is on plain element - only treat as component in compat mode
         p.name === 'bind' &&
-        isBindKey(p.arg, 'is') &&
+        isStaticArgOf(p.arg, 'is') &&
         __COMPAT__ &&
         checkCompatEnabled(
           CompilerDeprecationTypes.COMPILER_IS_ON_ELEMENT,

packages/compiler-core/src/transforms/hoistStatic.ts

@@ -168,6 +168,13 @@ export function getConstantType(
       if (codegenNode.type !== NodeTypes.VNODE_CALL) {
         return ConstantTypes.NOT_CONSTANT
       }
+      if (
+        codegenNode.isBlock &&
+        node.tag !== 'svg' &&
+        node.tag !== 'foreignObject'
+      ) {
+        return ConstantTypes.NOT_CONSTANT
+      }
       const flag = getPatchFlag(codegenNode)
       if (!flag) {
         let returnType = ConstantTypes.CAN_STRINGIFY

packages/compiler-core/src/transforms/transformElement.ts

@@ -56,7 +56,7 @@ import {
   toValidAssetId,
   findProp,
   isCoreComponent,
-  isBindKey,
+  isStaticArgOf,
   findDir,
   isStaticExp
 } from '../utils'
@@ -120,10 +120,7 @@ export const transformElement: NodeTransform = (node, context) => {
         // updates inside get proper isSVG flag at runtime. (#639, #643)
         // This is technically web-specific, but splitting the logic out of core
         // leads to too much unnecessary complexity.
-        (tag === 'svg' ||
-          tag === 'foreignObject' ||
-          // #938: elements with dynamic keys should be forced into blocks
-          findProp(node, 'key', true)))
+        (tag === 'svg' || tag === 'foreignObject'))
 
     // props
     if (props.length > 0) {
@@ -138,6 +135,10 @@ export const transformElement: NodeTransform = (node, context) => {
               directives.map(dir => buildDirectiveArgs(dir, context))
             ) as DirectiveArguments)
           : undefined
+
+      if (propsBuildResult.shouldUseBlock) {
+        shouldUseBlock = true
+      }
     }
 
     // children
@@ -386,12 +387,15 @@ export function buildProps(
   directives: DirectiveNode[]
   patchFlag: number
   dynamicPropNames: string[]
+  shouldUseBlock: boolean
 } {
-  const { tag, loc: elementLoc } = node
+  const { tag, loc: elementLoc, children } = node
   const isComponent = node.tagType === ElementTypes.COMPONENT
   let properties: ObjectExpression['properties'] = []
   const mergeArgs: PropsExpression[] = []
   const runtimeDirectives: DirectiveNode[] = []
+  const hasChildren = children.length > 0
+  let shouldUseBlock = false
 
   // patchFlag analysis
   let patchFlag = 0
@@ -526,7 +530,7 @@ export function buildProps(
       if (
         name === 'is' ||
         (isVBind &&
-          isBindKey(arg, 'is') &&
+          isStaticArgOf(arg, 'is') &&
           (isComponentTag(tag) ||
             (__COMPAT__ &&
               isCompatEnabled(
@@ -541,6 +545,16 @@ export function buildProps(
         continue
       }
 
+      if (
+        // #938: elements with dynamic keys should be forced into blocks
+        (isVBind && isStaticArgOf(arg, 'key')) ||
+        // inline before-update hooks need to force block so that it is invoked
+        // before children
+        (isVOn && hasChildren && isStaticArgOf(arg, 'vnodeBeforeUpdate', true))
+      ) {
+        shouldUseBlock = true
+      }
+
       // special case for v-bind and v-on with no argument
       if (!arg && (isVBind || isVOn)) {
         hasDynamicKeys = true
@@ -633,6 +647,11 @@ export function buildProps(
       } else {
         // no built-in transform, this is a user custom directive.
         runtimeDirectives.push(prop)
+        // custom dirs may use beforeUpdate so they need to force blocks
+        // to ensure before-update gets called before children update
+        if (hasChildren) {
+          shouldUseBlock = true
+        }
       }
     }
 
@@ -700,6 +719,7 @@ export function buildProps(
     }
   }
   if (
+    !shouldUseBlock &&
     (patchFlag === 0 || patchFlag === PatchFlags.HYDRATE_EVENTS) &&
     (hasRef || hasVnodeHook || runtimeDirectives.length > 0)
   ) {
@@ -784,7 +804,8 @@ export function buildProps(
     props: propsExpression,
     directives: runtimeDirectives,
     patchFlag,
-    dynamicPropNames
+    dynamicPropNames,
+    shouldUseBlock
   }
 }
 

packages/compiler-core/src/transforms/transformSlotOutlet.ts

@@ -7,7 +7,7 @@ import {
   SlotOutletNode,
   createFunctionExpression
 } from '../ast'
-import { isSlotOutlet, isBindKey, isStaticExp } from '../utils'
+import { isSlotOutlet, isStaticArgOf, isStaticExp } from '../utils'
 import { buildProps, PropsExpression } from './transformElement'
 import { createCompilerError, ErrorCodes } from '../errors'
 import { RENDER_SLOT } from '../runtimeHelpers'
@@ -75,7 +75,7 @@ export function processSlotOutlet(
         }
       }
     } else {
-      if (p.name === 'bind' && isBindKey(p.arg, 'name')) {
+      if (p.name === 'bind' && isStaticArgOf(p.arg, 'name')) {
         if (p.exp) slotName = p.exp
       } else {
         if (p.name === 'bind' && p.arg && isStaticExp(p.arg)) {

packages/compiler-core/src/utils.ts

@@ -42,7 +42,14 @@ import {
   WITH_MEMO,
   OPEN_BLOCK
 } from './runtimeHelpers'
-import { isString, isObject, hyphenate, extend, NOOP } from '@vue/shared'
+import {
+  isString,
+  isObject,
+  hyphenate,
+  extend,
+  NOOP,
+  camelize
+} from '@vue/shared'
 import { PropsExpression } from './transforms/transformElement'
 import { parseExpression } from '@babel/parser'
 import { Expression } from '@babel/types'
@@ -282,15 +289,23 @@ export function findProp(
     } else if (
       p.name === 'bind' &&
       (p.exp || allowEmpty) &&
-      isBindKey(p.arg, name)
+      isStaticArgOf(p.arg, name)
     ) {
       return p
     }
   }
 }
 
-export function isBindKey(arg: DirectiveNode['arg'], name: string): boolean {
-  return !!(arg && isStaticExp(arg) && arg.content === name)
+export function isStaticArgOf(
+  arg: DirectiveNode['arg'],
+  name: string,
+  camel?: boolean
+): boolean {
+  return !!(
+    arg &&
+    isStaticExp(arg) &&
+    (camel ? camelize(arg.content) : arg.content) === name
+  )
 }
 
 export function hasDynamicKeyVBind(node: ElementNode): boolean {
@@ -371,7 +386,8 @@ export function injectProp(
    *
    * we need to get the real props before normalization
    */
-  let props = node.type === NodeTypes.VNODE_CALL ? node.props : node.arguments[2]
+  let props =
+    node.type === NodeTypes.VNODE_CALL ? node.props : node.arguments[2]
   let callPath: CallExpression[] = []
   let parentCall: CallExpression | undefined
   if (

packages/compiler-ssr/src/transforms/ssrTransformElement.ts

@@ -22,7 +22,7 @@ import {
   TextNode,
   hasDynamicKeyVBind,
   MERGE_PROPS,
-  isBindKey,
+  isStaticArgOf,
   createSequenceExpression,
   InterpolationNode,
   isStaticExp,
@@ -335,7 +335,7 @@ function isTextareaWithValue(
   return !!(
     node.tag === 'textarea' &&
     prop.name === 'bind' &&
-    isBindKey(prop.arg, 'value')
+    isStaticArgOf(prop.arg, 'value')
   )
 }