fix(compiler-core): v-if key error should only be checking same key o…

…n different branches
vuejs · Aug 4, 2020 · de0c8a7 · de0c8a7
1 parent c3f8c78
commit de0c8a7
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 15 deletions.
diff --git a/packages/compiler-core/__tests__/transforms/vIf.spec.ts b/packages/compiler-core/__tests__/transforms/vIf.spec.ts
@@ -285,10 +285,23 @@ describe('compiler: v-if', () => {
 
     test('error on user key', () => {
       const onError = jest.fn()
-      parseWithIfTransform(`<div v-if="ok" :key="1" />`, { onError })
+      // dynamic
+      parseWithIfTransform(
+        `<div v-if="ok" :key="a + 1" /><div v-else :key="a + 1" />`,
+        { onError }
+      )
       expect(onError.mock.calls[0]).toMatchObject([
         {
-          code: ErrorCodes.X_V_IF_KEY
+          code: ErrorCodes.X_V_IF_SAME_KEY
+        }
+      ])
+      // static
+      parseWithIfTransform(`<div v-if="ok" key="1" /><div v-else key="1" />`, {
+        onError
+      })
+      expect(onError.mock.calls[1]).toMatchObject([
+        {
+          code: ErrorCodes.X_V_IF_SAME_KEY
         }
       ])
     })

diff --git a/packages/compiler-core/src/ast.ts b/packages/compiler-core/src/ast.ts
@@ -243,6 +243,7 @@ export interface IfBranchNode extends Node {
   type: NodeTypes.IF_BRANCH
   condition: ExpressionNode | undefined // else
   children: TemplateChildNode[]
+  userKey?: AttributeNode | DirectiveNode
 }
 
 export interface ForNode extends Node {

diff --git a/packages/compiler-core/src/errors.ts b/packages/compiler-core/src/errors.ts
@@ -63,7 +63,7 @@ export const enum ErrorCodes {
 
   // transform errors
   X_V_IF_NO_EXPRESSION,
-  X_V_IF_KEY,
+  X_V_IF_SAME_KEY,
   X_V_ELSE_NO_ADJACENT_IF,
   X_V_FOR_NO_EXPRESSION,
   X_V_FOR_MALFORMED_EXPRESSION,
@@ -136,10 +136,7 @@ export const errorMessages: { [code: number]: string } = {
 
   // transform errors
   [ErrorCodes.X_V_IF_NO_EXPRESSION]: `v-if/v-else-if is missing expression.`,
-  [ErrorCodes.X_V_IF_KEY]:
-    `v-if branches must use compiler generated keys. ` +
-    `In many cases, you can simply remove this key. ` +
-    `If this tag is inside of a <template v-for="...">, then you can move the key up to the parent <template>.`,
+  [ErrorCodes.X_V_IF_SAME_KEY]: `v-if/else branches must use unique keys.`,
   [ErrorCodes.X_V_ELSE_NO_ADJACENT_IF]: `v-else/v-else-if has no adjacent v-if.`,
   [ErrorCodes.X_V_FOR_NO_EXPRESSION]: `v-for is missing expression.`,
   [ErrorCodes.X_V_FOR_MALFORMED_EXPRESSION]: `v-for has invalid expression.`,

diff --git a/packages/compiler-core/src/transforms/vIf.ts b/packages/compiler-core/src/transforms/vIf.ts
@@ -18,7 +18,8 @@ import {
   IfConditionalExpression,
   BlockCodegenNode,
   IfNode,
-  createVNodeCall
+  createVNodeCall,
+  AttributeNode
 } from '../ast'
 import { createCompilerError, ErrorCodes } from '../errors'
 import { processExpression } from './transformExpression'
@@ -111,11 +112,6 @@ export function processIf(
     validateBrowserExpression(dir.exp as SimpleExpressionNode, context)
   }
 
-  const userKey = /*#__PURE__*/ findProp(node, 'key')
-  if (userKey) {
-    context.onError(createCompilerError(ErrorCodes.X_V_IF_KEY, userKey.loc))
-  }
-
   if (dir.name === 'if') {
     const branch = createIfBranch(node, dir)
     const ifNode: IfNode = {
@@ -146,6 +142,24 @@ export function processIf(
         if (__DEV__ && comments.length) {
           branch.children = [...comments, ...branch.children]
         }
+
+        // check if user is forcing same key on different branches
+        if (__DEV__ || !__BROWSER__) {
+          const key = branch.userKey
+          if (key) {
+            sibling.branches.forEach(({ userKey }) => {
+              if (isSameKey(userKey, key)) {
+                context.onError(
+                  createCompilerError(
+                    ErrorCodes.X_V_IF_SAME_KEY,
+                    branch.userKey!.loc
+                  )
+                )
+              }
+            })
+          }
+        }
+
         sibling.branches.push(branch)
         const onExit = processCodegen && processCodegen(sibling, branch, false)
         // since the branch was removed, it will not be traversed.
@@ -174,7 +188,8 @@ function createIfBranch(node: ElementNode, dir: DirectiveNode): IfBranchNode {
     children:
       node.tagType === ElementTypes.TEMPLATE && !findDir(node, 'for')
         ? node.children
-        : [node]
+        : [node],
+    userKey: findProp(node, `key`)
   }
 }
 
@@ -256,3 +271,32 @@ function createChildrenCodegenNode(
     return vnodeCall
   }
 }
+
+function isSameKey(
+  a: AttributeNode | DirectiveNode | undefined,
+  b: AttributeNode | DirectiveNode
+): boolean {
+  if (!a || a.type !== b.type) {
+    return false
+  }
+  if (a.type === NodeTypes.ATTRIBUTE) {
+    if (a.value!.content !== (b as AttributeNode).value!.content) {
+      return false
+    }
+  } else {
+    // directive
+    const exp = a.exp!
+    const branchExp = (b as DirectiveNode).exp!
+    if (exp.type !== branchExp.type) {
+      return false
+    }
+    if (
+      exp.type !== NodeTypes.SIMPLE_EXPRESSION ||
+      (exp.isStatic !== (branchExp as SimpleExpressionNode).isStatic ||
+        exp.content !== (branchExp as SimpleExpressionNode).content)
+    ) {
+      return false
+    }
+  }
+  return true
+}
diff --git a/packages/compiler-core/src/utils.ts b/packages/compiler-core/src/utils.ts
@@ -161,7 +161,11 @@ export function findProp(
       if (p.name === name && (p.value || allowEmpty)) {
         return p
       }
-    } else if (p.name === 'bind' && p.exp && isBindKey(p.arg, name)) {
+    } else if (
+      p.name === 'bind' &&
+      (p.exp || allowEmpty) &&
+      isBindKey(p.arg, name)
+    ) {
       return p
     }
   }