fix(compiler): do not hoist element with dynamic key (vuejs#187)

Author: xanf

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

@@ -1,6 +1,23 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`compiler: hositStatic transform hoist nested static tree 1`] = `
+exports[`compiler: hoistStatic transform hoist element with static key 1`] = `
+"const _Vue = Vue
+const _createVNode = Vue.createVNode
+
+const _hoisted_1 = _createVNode(\\"div\\", { key: \\"foo\\" })
+
+return function render() {
+  with (this) {
+    const { createVNode: _createVNode, createBlock: _createBlock, openBlock: _openBlock } = _Vue
+    
+    return (_openBlock(), _createBlock(\\"div\\", null, [
+      _hoisted_1
+    ]))
+  }
+}"
+`;
+
+exports[`compiler: hoistStatic transform hoist nested static tree 1`] = `
 "const _Vue = Vue
 const _createVNode = Vue.createVNode
 
@@ -20,7 +37,7 @@ return function render() {
 }"
 `;
 
-exports[`compiler: hositStatic transform hoist siblings with common non-hoistable parent 1`] = `
+exports[`compiler: hoistStatic transform hoist siblings with common non-hoistable parent 1`] = `
 "const _Vue = Vue
 const _createVNode = Vue.createVNode
 
@@ -39,7 +56,7 @@ return function render() {
 }"
 `;
 
-exports[`compiler: hositStatic transform hoist simple element 1`] = `
+exports[`compiler: hoistStatic transform hoist simple element 1`] = `
 "const _Vue = Vue
 const _createVNode = Vue.createVNode
 
@@ -56,7 +73,7 @@ return function render() {
 }"
 `;
 
-exports[`compiler: hositStatic transform hoist static props for elements with directives 1`] = `
+exports[`compiler: hoistStatic transform hoist static props for elements with directives 1`] = `
 "const _Vue = Vue
 const _createVNode = Vue.createVNode
 
@@ -77,7 +94,7 @@ return function render() {
 }"
 `;
 
-exports[`compiler: hositStatic transform hoist static props for elements with dynamic text children 1`] = `
+exports[`compiler: hoistStatic transform hoist static props for elements with dynamic text children 1`] = `
 "const _Vue = Vue
 const _createVNode = Vue.createVNode
 
@@ -94,7 +111,7 @@ return function render() {
 }"
 `;
 
-exports[`compiler: hositStatic transform hoist static props for elements with unhoistable children 1`] = `
+exports[`compiler: hoistStatic transform hoist static props for elements with unhoistable children 1`] = `
 "const _Vue = Vue
 const _createVNode = Vue.createVNode
 
@@ -115,7 +132,7 @@ return function render() {
 }"
 `;
 
-exports[`compiler: hositStatic transform should NOT hoist components 1`] = `
+exports[`compiler: hoistStatic transform should NOT hoist components 1`] = `
 "const _Vue = Vue
 
 return function render() {
@@ -131,7 +148,21 @@ return function render() {
 }"
 `;
 
-exports[`compiler: hositStatic transform should NOT hoist element with dynamic props 1`] = `
+exports[`compiler: hoistStatic transform should NOT hoist element with dynamic key 1`] = `
+"const _Vue = Vue
+
+return function render() {
+  with (this) {
+    const { createVNode: _createVNode, createBlock: _createBlock, openBlock: _openBlock } = _Vue
+    
+    return (_openBlock(), _createBlock(\\"div\\", null, [
+      _createVNode(\\"div\\", { key: foo })
+    ]))
+  }
+}"
+`;
+
+exports[`compiler: hoistStatic transform should NOT hoist element with dynamic props 1`] = `
 "const _Vue = Vue
 
 return function render() {
@@ -145,7 +176,7 @@ return function render() {
 }"
 `;
 
-exports[`compiler: hositStatic transform should NOT hoist root node 1`] = `
+exports[`compiler: hoistStatic transform should NOT hoist root node 1`] = `
 "const _Vue = Vue
 
 return function render() {
@@ -157,7 +188,7 @@ return function render() {
 }"
 `;
 
-exports[`compiler: hositStatic transform should hoist v-for children if static 1`] = `
+exports[`compiler: hoistStatic transform should hoist v-for children if static 1`] = `
 "const _Vue = Vue
 const _createVNode = Vue.createVNode
 
@@ -179,7 +210,7 @@ return function render() {
 }"
 `;
 
-exports[`compiler: hositStatic transform should hoist v-if props/children if static 1`] = `
+exports[`compiler: hoistStatic transform should hoist v-if props/children if static 1`] = `
 "const _Vue = Vue
 const _createVNode = Vue.createVNode
 

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

@@ -42,7 +42,7 @@ function transformWithHoist(template: string) {
   }
 }
 
-describe('compiler: hositStatic transform', () => {
+describe('compiler: hoistStatic transform', () => {
   test('should NOT hoist root node', () => {
     // if the whole tree is static, the root still needs to be a block
     // so that it's patched in optimized mode to skip children
@@ -187,6 +187,52 @@ describe('compiler: hositStatic transform', () => {
     expect(generate(root).code).toMatchSnapshot()
   })
 
+  test('hoist element with static key', () => {
+    const { root, args } = transformWithHoist(`<div><div key="foo"/></div>`)
+    expect(root.hoists.length).toBe(1)
+    expect(root.hoists).toMatchObject([
+      {
+        type: NodeTypes.JS_CALL_EXPRESSION,
+        callee: CREATE_VNODE,
+        arguments: [`"div"`, createObjectMatcher({ key: 'foo' })]
+      }
+    ])
+    expect(args).toMatchObject([
+      `"div"`,
+      `null`,
+      [
+        {
+          type: NodeTypes.ELEMENT,
+          codegenNode: {
+            type: NodeTypes.SIMPLE_EXPRESSION,
+            content: `_hoisted_1`
+          }
+        }
+      ]
+    ])
+    expect(generate(root).code).toMatchSnapshot()
+  })
+
+  test('should NOT hoist element with dynamic key', () => {
+    const { root, args } = transformWithHoist(`<div><div :key="foo"/></div>`)
+    expect(root.hoists.length).toBe(0)
+    expect(args[2]).toMatchObject([
+      {
+        type: NodeTypes.ELEMENT,
+        codegenNode: {
+          callee: CREATE_VNODE,
+          arguments: [
+            `"div"`,
+            createObjectMatcher({
+              key: `[foo]`
+            })
+          ]
+        }
+      }
+    ])
+    expect(generate(root).code).toMatchSnapshot()
+  })
+
   test('hoist static props for elements with directives', () => {
     const { root, args } = transformWithHoist(
       `<div><div id="foo" v-foo/></div>`

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

@@ -6,12 +6,18 @@ import {
   ElementCodegenNode,
   PlainElementNode,
   ComponentNode,
-  TemplateNode
+  TemplateNode,
+  ElementNode
 } from '../ast'
 import { TransformContext } from '../transform'
 import { APPLY_DIRECTIVES } from '../runtimeHelpers'
 import { PatchFlags } from '@vue/shared'
-import { isSlotOutlet } from '../utils'
+import { isSlotOutlet, findProp } from '../utils'
+
+function hasDynamicKey(node: ElementNode) {
+  const keyProp = findProp(node, 'key')
+  return keyProp && keyProp.type === NodeTypes.DIRECTIVE
+}
 
 export function hoistStatic(root: RootNode, context: TransformContext) {
   walk(
@@ -47,7 +53,11 @@ function walk(
       child.type === NodeTypes.ELEMENT &&
       child.tagType === ElementTypes.ELEMENT
     ) {
-      if (!doNotHoistNode && isStaticNode(child, resultCache)) {
+      if (
+        !doNotHoistNode &&
+        isStaticNode(child, resultCache) &&
+        !hasDynamicKey(child)
+      ) {
         // whole tree is static
         child.codegenNode = context.hoist(child.codegenNode!)
         continue
@@ -56,9 +66,10 @@ function walk(
         // hoisting.
         const flag = getPatchFlag(child)
         if (
-          !flag ||
-          flag === PatchFlags.NEED_PATCH ||
-          flag === PatchFlags.TEXT
+          (!flag ||
+            flag === PatchFlags.NEED_PATCH ||
+            flag === PatchFlags.TEXT) &&
+          !hasDynamicKey(child)
         ) {
           let codegenNode = child.codegenNode as ElementCodegenNode
           if (codegenNode.callee === APPLY_DIRECTIVES) {