From 1c9a4810fcdd2b6c1c6c3be077aebbecbfcbcf1e Mon Sep 17 00:00:00 2001 From: Evan You Date: Fri, 10 Dec 2021 15:34:23 +0800 Subject: [PATCH] fix(compiler): force block for custom dirs and inline beforeUpdate hooks to ensure they are called before children updates --- .../__snapshots__/transformText.spec.ts.snap | 4 +- .../transforms/transformElement.spec.ts | 17 +++++++++ packages/compiler-core/src/parse.ts | 4 +- .../src/transforms/hoistStatic.ts | 7 ++++ .../src/transforms/transformElement.ts | 37 +++++++++++++++---- .../src/transforms/transformSlotOutlet.ts | 4 +- packages/compiler-core/src/utils.ts | 26 ++++++++++--- .../src/transforms/ssrTransformElement.ts | 4 +- 8 files changed, 82 insertions(+), 21 deletions(-) diff --git a/packages/compiler-core/__tests__/transforms/__snapshots__/transformText.spec.ts.snap b/packages/compiler-core/__tests__/transforms/__snapshots__/transformText.spec.ts.snap index ada1ac6f1ca..b17a825b7b8 100644 --- a/packages/compiler-core/__tests__/transforms/__snapshots__/transformText.spec.ts.snap +++ b/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] ]) } diff --git a/packages/compiler-core/__tests__/transforms/transformElement.spec.ts b/packages/compiler-core/__tests__/transforms/transformElement.spec.ts index eb29f346522..17e6b5bb14a 100644 --- a/packages/compiler-core/__tests__/transforms/transformElement.spec.ts +++ b/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(`
hello
`) + expect(node.isBlock).toBe(true) + }) + + test('force block for inline before-update handlers w/ children', () => { + expect( + parseWithElementTransform(`
hello
`).node + .isBlock + ).toBe(true) + + expect( + parseWithElementTransform(`
hello
`).node + .isBlock + ).toBe(true) + }) + // #938 test('element with dynamic keys should be forced into blocks', () => { const ast = parse(`
`) diff --git a/packages/compiler-core/src/parse.ts b/packages/compiler-core/src/parse.ts index f4d824f5d72..5eeb9f8c6ce 100644 --- a/packages/compiler-core/src/parse.ts +++ b/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, diff --git a/packages/compiler-core/src/transforms/hoistStatic.ts b/packages/compiler-core/src/transforms/hoistStatic.ts index 215792ed403..5c8a10923bf 100644 --- a/packages/compiler-core/src/transforms/hoistStatic.ts +++ b/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 diff --git a/packages/compiler-core/src/transforms/transformElement.ts b/packages/compiler-core/src/transforms/transformElement.ts index d88bd7b9f37..fb0ddd528c7 100644 --- a/packages/compiler-core/src/transforms/transformElement.ts +++ b/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 } } diff --git a/packages/compiler-core/src/transforms/transformSlotOutlet.ts b/packages/compiler-core/src/transforms/transformSlotOutlet.ts index 50f0e0bfc8c..394684b408d 100644 --- a/packages/compiler-core/src/transforms/transformSlotOutlet.ts +++ b/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)) { diff --git a/packages/compiler-core/src/utils.ts b/packages/compiler-core/src/utils.ts index 6e2443ea664..0c62f3043d9 100644 --- a/packages/compiler-core/src/utils.ts +++ b/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 ( diff --git a/packages/compiler-ssr/src/transforms/ssrTransformElement.ts b/packages/compiler-ssr/src/transforms/ssrTransformElement.ts index c9515d27baa..659537b2c82 100644 --- a/packages/compiler-ssr/src/transforms/ssrTransformElement.ts +++ b/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') ) }