refactor(compiler-core): simplify hoistStatic check for nodes without…

… patchFlag close vuejs#1912
fnlctrl · Aug 20, 2020 · 91700fb · 91700fb
1 parent 255a2bd
commit 91700fb
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 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
@@ -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
@@ -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(
+        `<div><template v-for="item in items" :key="item"><span/></template></div>`
+      )
+      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
@@ -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
@@ -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]