From dd2bfb5a8f5b897a621b3ebb89a9fb1b8e4c63cd Mon Sep 17 00:00:00 2001 From: Evan You Date: Mon, 18 May 2020 10:04:03 -0400 Subject: [PATCH] fix(compiler-dom): should bail stringification on runtime constant regardless of position ref: vuejs/vite#157 --- .../src/transforms/hoistStatic.ts | 127 +++++++++++++----- .../src/transforms/transformElement.ts | 6 +- .../transforms/stringifyStatic.spec.ts | 4 +- .../src/transforms/stringifyStatic.ts | 11 -- 4 files changed, 99 insertions(+), 49 deletions(-) diff --git a/packages/compiler-core/src/transforms/hoistStatic.ts b/packages/compiler-core/src/transforms/hoistStatic.ts index 38ea88aed87..b3ac3aa7cce 100644 --- a/packages/compiler-core/src/transforms/hoistStatic.ts +++ b/packages/compiler-core/src/transforms/hoistStatic.ts @@ -37,13 +37,29 @@ export function isSingleElementRoot( ) } +const enum StaticType { + NOT_STATIC = 0, + FULL_STATIC, + HAS_RUNTIME_CONSTANT +} + function walk( children: TemplateChildNode[], context: TransformContext, - resultCache: Map, + resultCache: Map, doNotHoistNode: boolean = false ) { let hasHoistedNode = false + // Some transforms, e.g. trasnformAssetUrls from @vue/compiler-sfc, replaces + // static bindings with expressions. These expressions are guaranteed to be + // constant so they are still eligible for hoisting, but they are only + // available at runtime and therefore cannot be evaluated ahead of time. + // This is only a concern for pre-stringification (via transformHoist by + // @vue/compiler-dom), but doing it here allows us to perform only one full + // walk of the AST and allow `stringifyStatic` to stop walking as soon as its + // stringficiation threshold is met. + let hasRuntimeConstant = false + for (let i = 0; i < children.length; i++) { const child = children[i] // only plain elements & text calls are eligible for hoisting. @@ -51,7 +67,14 @@ function walk( child.type === NodeTypes.ELEMENT && child.tagType === ElementTypes.ELEMENT ) { - if (!doNotHoistNode && isStaticNode(child, resultCache)) { + let staticType + if ( + !doNotHoistNode && + (staticType = getStaticType(child, resultCache)) > 0 + ) { + if (staticType === StaticType.HAS_RUNTIME_CONSTANT) { + hasRuntimeConstant = true + } // whole tree is static ;(child.codegenNode as VNodeCall).patchFlag = PatchFlags.HOISTED + (__DEV__ ? ` /* HOISTED */` : ``) @@ -78,12 +101,15 @@ function walk( } } } - } else if ( - child.type === NodeTypes.TEXT_CALL && - isStaticNode(child.content, resultCache) - ) { - child.codegenNode = context.hoist(child.codegenNode) - hasHoistedNode = true + } else if (child.type === NodeTypes.TEXT_CALL) { + const staticType = getStaticType(child.content, resultCache) + if (staticType > 0) { + if (staticType === StaticType.HAS_RUNTIME_CONSTANT) { + hasRuntimeConstant = true + } + child.codegenNode = context.hoist(child.codegenNode) + hasHoistedNode = true + } } // walk further @@ -101,19 +127,19 @@ function walk( } } - if (hasHoistedNode && context.transformHoist) { + if (!hasRuntimeConstant && hasHoistedNode && context.transformHoist) { context.transformHoist(children, context) } } -export function isStaticNode( +export function getStaticType( node: TemplateChildNode | SimpleExpressionNode, - resultCache: Map = new Map() -): boolean { + resultCache: Map = new Map() +): StaticType { switch (node.type) { case NodeTypes.ELEMENT: if (node.tagType !== ElementTypes.ELEMENT) { - return false + return StaticType.NOT_STATIC } const cached = resultCache.get(node) if (cached !== undefined) { @@ -121,53 +147,88 @@ export function isStaticNode( } const codegenNode = node.codegenNode! if (codegenNode.type !== NodeTypes.VNODE_CALL) { - return false + return StaticType.NOT_STATIC } const flag = getPatchFlag(codegenNode) if (!flag && !hasDynamicKeyOrRef(node) && !hasCachedProps(node)) { // element self is static. check its children. + let returnType = StaticType.FULL_STATIC for (let i = 0; i < node.children.length; i++) { - if (!isStaticNode(node.children[i], resultCache)) { - resultCache.set(node, false) - return false + const childType = getStaticType(node.children[i], resultCache) + if (childType === StaticType.NOT_STATIC) { + resultCache.set(node, StaticType.NOT_STATIC) + return StaticType.NOT_STATIC + } else if (childType === StaticType.HAS_RUNTIME_CONSTANT) { + returnType = StaticType.HAS_RUNTIME_CONSTANT } } - // only svg/foreignObject could be block here, however if they are static - // then they don't need to be blocks since there will be no nested - // updates. + + // check if any of the props contain runtime constants + if (returnType !== StaticType.HAS_RUNTIME_CONSTANT) { + for (let i = 0; i < node.props.length; i++) { + const p = node.props[i] + if ( + p.type === NodeTypes.DIRECTIVE && + p.name === 'bind' && + p.exp && + (p.exp.type === NodeTypes.COMPOUND_EXPRESSION || + p.exp.isRuntimeConstant) + ) { + returnType = StaticType.HAS_RUNTIME_CONSTANT + } + } + } + + // only svg/foreignObject could be block here, however if they are + // stati then they don't need to be blocks since there will be no + // nested updates. if (codegenNode.isBlock) { codegenNode.isBlock = false } - resultCache.set(node, true) - return true + + resultCache.set(node, returnType) + return returnType } else { - resultCache.set(node, false) - return false + resultCache.set(node, StaticType.NOT_STATIC) + return StaticType.NOT_STATIC } case NodeTypes.TEXT: case NodeTypes.COMMENT: - return true + return StaticType.FULL_STATIC case NodeTypes.IF: case NodeTypes.FOR: case NodeTypes.IF_BRANCH: - return false + return StaticType.NOT_STATIC case NodeTypes.INTERPOLATION: case NodeTypes.TEXT_CALL: - return isStaticNode(node.content, resultCache) + return getStaticType(node.content, resultCache) case NodeTypes.SIMPLE_EXPRESSION: return node.isConstant + ? node.isRuntimeConstant + ? StaticType.HAS_RUNTIME_CONSTANT + : StaticType.FULL_STATIC + : StaticType.NOT_STATIC case NodeTypes.COMPOUND_EXPRESSION: - return node.children.every(child => { - return ( - isString(child) || isSymbol(child) || isStaticNode(child, resultCache) - ) - }) + let returnType = StaticType.FULL_STATIC + for (let i = 0; i < node.children.length; i++) { + const child = node.children[i] + if (isString(child) || isSymbol(child)) { + continue + } + const childType = getStaticType(child, resultCache) + if (childType === StaticType.NOT_STATIC) { + return StaticType.NOT_STATIC + } else if (childType === StaticType.HAS_RUNTIME_CONSTANT) { + returnType = StaticType.HAS_RUNTIME_CONSTANT + } + } + return returnType default: if (__DEV__) { const exhaustiveCheck: never = node exhaustiveCheck } - return false + return StaticType.NOT_STATIC } } diff --git a/packages/compiler-core/src/transforms/transformElement.ts b/packages/compiler-core/src/transforms/transformElement.ts index f1f30d5b89f..012b3801af0 100644 --- a/packages/compiler-core/src/transforms/transformElement.ts +++ b/packages/compiler-core/src/transforms/transformElement.ts @@ -46,7 +46,7 @@ import { findDir } from '../utils' import { buildSlots } from './vSlot' -import { isStaticNode } from './hoistStatic' +import { getStaticType } from './hoistStatic' // some directive transforms (e.g. v-model) may return a symbol for runtime // import, which should be used instead of a resolveDirective call. @@ -156,7 +156,7 @@ export const transformElement: NodeTransform = (node, context) => { const hasDynamicTextChild = type === NodeTypes.INTERPOLATION || type === NodeTypes.COMPOUND_EXPRESSION - if (hasDynamicTextChild && !isStaticNode(child)) { + if (hasDynamicTextChild && !getStaticType(child)) { patchFlag |= PatchFlags.TEXT } // pass directly if the only child is a text node @@ -292,7 +292,7 @@ export function buildProps( value.type === NodeTypes.JS_CACHE_EXPRESSION || ((value.type === NodeTypes.SIMPLE_EXPRESSION || value.type === NodeTypes.COMPOUND_EXPRESSION) && - isStaticNode(value)) + getStaticType(value) > 0) ) { // skip if the prop is a cached handler or has constant value return diff --git a/packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts b/packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts index 7c7f985dfbe..b4d96605c0b 100644 --- a/packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts +++ b/packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts @@ -161,10 +161,10 @@ describe('stringify static html', () => { test('should bail on runtime constant v-bind bindings', () => { const { ast } = compile( - `
${repeat( + `
${repeat( `foo`, StringifyThresholds.ELEMENT_WITH_BINDING_COUNT - )}
`, + )}
`, { hoistStatic: true, prefixIdentifiers: true, diff --git a/packages/compiler-dom/src/transforms/stringifyStatic.ts b/packages/compiler-dom/src/transforms/stringifyStatic.ts index a022d04091f..310c03e66a2 100644 --- a/packages/compiler-dom/src/transforms/stringifyStatic.ts +++ b/packages/compiler-dom/src/transforms/stringifyStatic.ts @@ -185,17 +185,6 @@ function analyzeNode(node: StringiableNode): [number, number] | false { ) { return bail() } - // some transforms, e.g. `transformAssetUrls` in `@vue/compiler-sfc` may - // convert static attributes into a v-bind with a constnat expresion. - // Such constant bindings are eligible for hoisting but not for static - // stringification because they cannot be pre-evaluated. - if ( - p.exp && - (p.exp.type === NodeTypes.COMPOUND_EXPRESSION || - p.exp.isRuntimeConstant) - ) { - return bail() - } } } for (let i = 0; i < node.children.length; i++) {