From 91700fbec2cd2c4e3448190b71ae93f751e4c3eb Mon Sep 17 00:00:00 2001 From: Evan You Date: Thu, 20 Aug 2020 11:43:34 -0400 Subject: [PATCH] refactor(compiler-core): simplify hoistStatic check for nodes without patchFlag close #1912 --- .../__snapshots__/hoistStatic.spec.ts.snap | 16 ++++++++ .../__tests__/transforms/hoistStatic.spec.ts | 8 ++++ .../src/transforms/hoistStatic.ts | 37 ++++++++----------- packages/compiler-core/src/transforms/vIf.ts | 5 ++- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/packages/compiler-core/__tests__/transforms/__snapshots__/hoistStatic.spec.ts.snap b/packages/compiler-core/__tests__/transforms/__snapshots__/hoistStatic.spec.ts.snap index 69267083..5e3fdf58 100644 --- a/packages/compiler-core/__tests__/transforms/__snapshots__/hoistStatic.spec.ts.snap +++ b/packages/compiler-core/__tests__/transforms/__snapshots__/hoistStatic.spec.ts.snap @@ -271,6 +271,22 @@ return function render(_ctx, _cache) { }" `; +exports[`compiler: hoistStatic transform prefixIdentifiers should NOT hoist keyed template v-for with plain element child 1`] = ` +"const _Vue = Vue + +return function render(_ctx, _cache) { + with (_ctx) { + const { renderList: _renderList, Fragment: _Fragment, openBlock: _openBlock, createBlock: _createBlock, createVNode: _createVNode } = _Vue + + return (_openBlock(), _createBlock(\\"div\\", null, [ + (_openBlock(true), _createBlock(_Fragment, null, _renderList(items, (item) => { + return (_openBlock(), _createBlock(\\"span\\", { key: item })) + }), 128 /* KEYED_FRAGMENT */)) + ])) + } +}" +`; + exports[`compiler: hoistStatic transform should NOT hoist components 1`] = ` "const _Vue = Vue diff --git a/packages/compiler-core/__tests__/transforms/hoistStatic.spec.ts b/packages/compiler-core/__tests__/transforms/hoistStatic.spec.ts index 5751b63f..c13f1c48 100644 --- a/packages/compiler-core/__tests__/transforms/hoistStatic.spec.ts +++ b/packages/compiler-core/__tests__/transforms/hoistStatic.spec.ts @@ -600,5 +600,13 @@ describe('compiler: hoistStatic transform', () => { }).code ).toMatchSnapshot() }) + + test('should NOT hoist keyed template v-for with plain element child', () => { + const root = transformWithHoist( + `
` + ) + expect(root.hoists.length).toBe(0) + expect(generate(root).code).toMatchSnapshot() + }) }) }) diff --git a/packages/compiler-core/src/transforms/hoistStatic.ts b/packages/compiler-core/src/transforms/hoistStatic.ts index baf1a2af..240f9bac 100644 --- a/packages/compiler-core/src/transforms/hoistStatic.ts +++ b/packages/compiler-core/src/transforms/hoistStatic.ts @@ -7,13 +7,12 @@ import { PlainElementNode, ComponentNode, TemplateNode, - ElementNode, VNodeCall, ParentNode } from '../ast' import { TransformContext } from '../transform' import { PatchFlags, isString, isSymbol } from '@vue/shared' -import { isSlotOutlet, findProp } from '../utils' +import { isSlotOutlet } from '../utils' export function hoistStatic(root: RootNode, context: TransformContext) { walk( @@ -93,8 +92,7 @@ function walk( (!flag || flag === PatchFlags.NEED_PATCH || flag === PatchFlags.TEXT) && - !hasDynamicKeyOrRef(child) && - !hasCachedProps(child) + !hasNonHoistableProps(child) ) { const props = getNodeProps(child) if (props) { @@ -156,7 +154,7 @@ export function getStaticType( return StaticType.NOT_STATIC } const flag = getPatchFlag(codegenNode) - if (!flag && !hasDynamicKeyOrRef(node) && !hasCachedProps(node)) { + if (!flag && !hasNonHoistableProps(node)) { // element self is static. check its children. let returnType = StaticType.FULL_STATIC for (let i = 0; i < node.children.length; i++) { @@ -238,28 +236,23 @@ export function getStaticType( } } -function hasDynamicKeyOrRef(node: ElementNode): boolean { - return !!(findProp(node, 'key', true) || findProp(node, 'ref', true)) -} - -function hasCachedProps(node: PlainElementNode): boolean { - if (__BROWSER__) { - return false - } +/** + * Even for a node with no patch flag, it is possible for it to contain + * non-hoistable expressions that refers to scope variables, e.g. compiler + * injected keys or cached event handlers. Therefore we need to always check the + * codegenNode's props to be sure. + */ +function hasNonHoistableProps(node: PlainElementNode): boolean { const props = getNodeProps(node) if (props && props.type === NodeTypes.JS_OBJECT_EXPRESSION) { const { properties } = props for (let i = 0; i < properties.length; i++) { - const val = properties[i].value - if (val.type === NodeTypes.JS_CACHE_EXPRESSION) { - return true - } - // merged event handlers + const { key, value } = properties[i] if ( - val.type === NodeTypes.JS_ARRAY_EXPRESSION && - val.elements.some( - e => !isString(e) && e.type === NodeTypes.JS_CACHE_EXPRESSION - ) + key.type !== NodeTypes.SIMPLE_EXPRESSION || + !key.isStatic || + (value.type !== NodeTypes.SIMPLE_EXPRESSION || + (!value.isStatic && !value.isConstant)) ) { return true } diff --git a/packages/compiler-core/src/transforms/vIf.ts b/packages/compiler-core/src/transforms/vIf.ts index cab3dd92..bebf3eb9 100644 --- a/packages/compiler-core/src/transforms/vIf.ts +++ b/packages/compiler-core/src/transforms/vIf.ts @@ -19,7 +19,8 @@ import { BlockCodegenNode, IfNode, createVNodeCall, - AttributeNode + AttributeNode, + locStub } from '../ast' import { createCompilerError, ErrorCodes } from '../errors' import { processExpression } from './transformExpression' @@ -222,7 +223,7 @@ function createChildrenCodegenNode( const { helper } = context const keyProperty = createObjectProperty( `key`, - createSimpleExpression(`${keyIndex}`, false) + createSimpleExpression(`${keyIndex}`, false, locStub, true) ) const { children } = branch const firstChild = children[0]