Improve error message when not providing a value for JSX key (#12983)

* Improve error message when not providing a value for key * Update packages/babel-plugin-transform-react-jsx/src/create-plugin.js Show location of attribute instead of only the path Co-authored-by: Huáng Jùnliàng <jlhwung@gmail.com> * Change error message to be less aggressive * Throw error when runtime is "classic" Co-authored-by: Huáng Jùnliàng <jlhwung@gmail.com>
babel · Mar 16, 2021 · 281acd6 · 281acd6
1 parent 6e1e003
commit 281acd6
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 17 deletions.
diff --git a/packages/babel-plugin-transform-react-jsx/src/create-plugin.js b/packages/babel-plugin-transform-react-jsx/src/create-plugin.js
@@ -341,9 +341,9 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,
       }
     }
 
-    function accumulateAttribute(array, node) {
-      if (t.isJSXSpreadAttribute(node)) {
-        const arg = node.argument;
+    function accumulateAttribute(array, attribute) {
+      if (t.isJSXSpreadAttribute(attribute.node)) {
+        const arg = attribute.node.argument;
         // Collect properties into props array if spreading object expression
         if (t.isObjectExpression(arg)) {
           array.push(...arg.properties);
@@ -353,26 +353,46 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,
         return array;
       }
 
-      const value = convertAttributeValue(node.value || t.booleanLiteral(true));
+      const value = convertAttributeValue(
+        attribute.node.name.name !== "key"
+          ? attribute.node.value || t.booleanLiteral(true)
+          : attribute.node.value,
+      );
 
-      if (t.isStringLiteral(value) && !t.isJSXExpressionContainer(node.value)) {
+      if (attribute.node.name.name === "key" && value === null) {
+        throw attribute.buildCodeFrameError(
+          'Please provide an explicit key value. Using "key" as a shorthand for "key={true}" is not allowed.',
+        );
+      }
+
+      if (
+        t.isStringLiteral(value) &&
+        !t.isJSXExpressionContainer(attribute.node.value)
+      ) {
         value.value = value.value.replace(/\n\s+/g, " ");
 
         // "raw" JSXText should not be used from a StringLiteral because it needs to be escaped.
         delete value.extra?.raw;
       }
 
-      if (t.isJSXNamespacedName(node.name)) {
-        node.name = t.stringLiteral(
-          node.name.namespace.name + ":" + node.name.name.name,
+      if (t.isJSXNamespacedName(attribute.node.name)) {
+        attribute.node.name = t.stringLiteral(
+          attribute.node.name.namespace.name +
+            ":" +
+            attribute.node.name.name.name,
         );
-      } else if (t.isValidIdentifier(node.name.name, false)) {
-        node.name.type = "Identifier";
+      } else if (t.isValidIdentifier(attribute.node.name.name, false)) {
+        attribute.node.name.type = "Identifier";
       } else {
-        node.name = t.stringLiteral(node.name.name);
+        attribute.node.name = t.stringLiteral(attribute.node.name.name);
       }
 
-      array.push(t.inherits(t.objectProperty(node.name, value), node));
+      array.push(
+        t.inherits(
+          t.objectProperty(attribute.node.name, value),
+          attribute.node,
+        ),
+      );
       return array;
     }
 
@@ -411,14 +431,22 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,
             case "__self":
               if (extracted[name]) throw sourceSelfError(path, name);
             /* falls through */
-            case "key":
-              extracted[name] = convertAttributeValue(attr.node.value);
+            case "key": {
+              const keyValue = convertAttributeValue(attr.node.value);
+              if (keyValue === null) {
+                throw attr.buildCodeFrameError(
+                  'Please provide an explicit key value. Using "key" as a shorthand for "key={true}" is not allowed.',
+                );
+              }
+
+              extracted[name] = keyValue;
               break;
+            }
             default:
-              attribs.push(attr.node);
+              attribs.push(attr);
           }
         } else {
-          attribs.push(attr.node);
+          attribs.push(attr);
         }
       }
 
@@ -511,7 +539,7 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,
         buildCreateElementOpeningElementAttributes(
           file,
           path,
-          openingPath.node.attributes,
+          openingPath.get("attributes"),
         ),
         ...t.react.buildChildren(path.node),
       ]);

diff --git a/...-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/input.js b/...-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/input.js
@@ -0,0 +1,2 @@
+
+var x = [<div key></div>];
diff --git a/...nsform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/options.json b/...nsform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/options.json
@@ -0,0 +1,3 @@
+{
+  "throws": "Please provide an explicit key value. Using \"key\" as a shorthand for \"key={true}\" is not allowed."
+}
diff --git a/...bel-plugin-transform-react-jsx/test/fixtures/react/should-disallow-valueless-key/input.js b/...bel-plugin-transform-react-jsx/test/fixtures/react/should-disallow-valueless-key/input.js
@@ -0,0 +1,2 @@
+
+var x = [<div key></div>];
diff --git a/...plugin-transform-react-jsx/test/fixtures/react/should-disallow-valueless-key/options.json b/...plugin-transform-react-jsx/test/fixtures/react/should-disallow-valueless-key/options.json
@@ -0,0 +1,3 @@
+{
+  "throws": "Please provide an explicit key value. Using \"key\" as a shorthand for \"key={true}\" is not allowed."
+}