From 6f8a9a5402de2f2cc0d91a492a262df4d35bb3e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Hajnal?= Date: Tue, 9 Mar 2021 00:48:45 +0100 Subject: [PATCH 1/4] Improve error message when not providing a value for key --- .../src/create-plugin.js | 12 ++++++++++-- .../should-disallow-valueless-key/input.js | 2 ++ .../should-disallow-valueless-key/options.json | 3 +++ 3 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/input.js create mode 100644 packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/options.json diff --git a/packages/babel-plugin-transform-react-jsx/src/create-plugin.js b/packages/babel-plugin-transform-react-jsx/src/create-plugin.js index c0ed656736ab..7771b9512912 100644 --- a/packages/babel-plugin-transform-react-jsx/src/create-plugin.js +++ b/packages/babel-plugin-transform-react-jsx/src/create-plugin.js @@ -411,9 +411,17 @@ 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 path.buildCodeFrameError( + 'Provide an explicit value to be used as a key. You are not supposed to use the "key" shorthand for "key={true}".', + ); + } + + extracted[name] = keyValue; break; + } default: attribs.push(attr.node); } diff --git a/packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/input.js b/packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/input.js new file mode 100644 index 000000000000..7a0aa2002a15 --- /dev/null +++ b/packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/input.js @@ -0,0 +1,2 @@ + +var x = [
]; diff --git a/packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/options.json b/packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/options.json new file mode 100644 index 000000000000..001f323fafa5 --- /dev/null +++ b/packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/options.json @@ -0,0 +1,3 @@ +{ + "throws": "Provide an explicit value to be used as a key. You are not supposed to use the \"key\" shorthand for \"key={true}\"." +} From 78bcfc558ecc8b005f03a3ececb5f510e4066f43 Mon Sep 17 00:00:00 2001 From: hajnalbendeguz Date: Wed, 10 Mar 2021 21:16:41 +0100 Subject: [PATCH 2/4] Update packages/babel-plugin-transform-react-jsx/src/create-plugin.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Show location of attribute instead of only the path Co-authored-by: Huáng Jùnliàng --- packages/babel-plugin-transform-react-jsx/src/create-plugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-plugin-transform-react-jsx/src/create-plugin.js b/packages/babel-plugin-transform-react-jsx/src/create-plugin.js index 7771b9512912..c81cc149e147 100644 --- a/packages/babel-plugin-transform-react-jsx/src/create-plugin.js +++ b/packages/babel-plugin-transform-react-jsx/src/create-plugin.js @@ -414,7 +414,7 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`, case "key": { const keyValue = convertAttributeValue(attr.node.value); if (keyValue === null) { - throw path.buildCodeFrameError( + throw attr.buildCodeFrameError( 'Provide an explicit value to be used as a key. You are not supposed to use the "key" shorthand for "key={true}".', ); } From 737e16e012ce81f9210b86474d10df5af898e5ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Hajnal?= Date: Sat, 13 Mar 2021 19:18:43 +0100 Subject: [PATCH 3/4] Change error message to be less aggressive --- packages/babel-plugin-transform-react-jsx/src/create-plugin.js | 2 +- .../react-automatic/should-disallow-valueless-key/options.json | 2 +- 2 files changed, 2 insertions(+), 2 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 index c81cc149e147..7ba25bfa0bc4 100644 --- a/packages/babel-plugin-transform-react-jsx/src/create-plugin.js +++ b/packages/babel-plugin-transform-react-jsx/src/create-plugin.js @@ -415,7 +415,7 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`, const keyValue = convertAttributeValue(attr.node.value); if (keyValue === null) { throw attr.buildCodeFrameError( - 'Provide an explicit value to be used as a key. You are not supposed to use the "key" shorthand for "key={true}".', + 'Please provide an explicit key value. Using "key" as a shorthand for "key={true}" is not allowed.', ); } diff --git a/packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/options.json b/packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/options.json index 001f323fafa5..657af9974ca5 100644 --- a/packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/options.json +++ b/packages/babel-plugin-transform-react-jsx/test/fixtures/react-automatic/should-disallow-valueless-key/options.json @@ -1,3 +1,3 @@ { - "throws": "Provide an explicit value to be used as a key. You are not supposed to use the \"key\" shorthand for \"key={true}\"." + "throws": "Please provide an explicit key value. Using \"key\" as a shorthand for \"key={true}\" is not allowed." } From d0c94e12b2616fdeb9bf1d6c4cd56616620718cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20Hajnal?= Date: Sat, 13 Mar 2021 19:21:35 +0100 Subject: [PATCH 4/4] Throw error when runtime is "classic" --- .../src/create-plugin.js | 50 +++++++++++++------ .../should-disallow-valueless-key/input.js | 2 + .../options.json | 3 ++ 3 files changed, 40 insertions(+), 15 deletions(-) create mode 100644 packages/babel-plugin-transform-react-jsx/test/fixtures/react/should-disallow-valueless-key/input.js create mode 100644 packages/babel-plugin-transform-react-jsx/test/fixtures/react/should-disallow-valueless-key/options.json diff --git a/packages/babel-plugin-transform-react-jsx/src/create-plugin.js b/packages/babel-plugin-transform-react-jsx/src/create-plugin.js index 7ba25bfa0bc4..e7d257b6dc2f 100644 --- 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 (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(node.value)) { + 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; } @@ -423,10 +443,10 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`, break; } default: - attribs.push(attr.node); + attribs.push(attr); } } else { - attribs.push(attr.node); + attribs.push(attr); } } @@ -519,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/packages/babel-plugin-transform-react-jsx/test/fixtures/react/should-disallow-valueless-key/input.js b/packages/babel-plugin-transform-react-jsx/test/fixtures/react/should-disallow-valueless-key/input.js new file mode 100644 index 000000000000..7a0aa2002a15 --- /dev/null +++ b/packages/babel-plugin-transform-react-jsx/test/fixtures/react/should-disallow-valueless-key/input.js @@ -0,0 +1,2 @@ + +var x = [
]; diff --git a/packages/babel-plugin-transform-react-jsx/test/fixtures/react/should-disallow-valueless-key/options.json b/packages/babel-plugin-transform-react-jsx/test/fixtures/react/should-disallow-valueless-key/options.json new file mode 100644 index 000000000000..657af9974ca5 --- /dev/null +++ b/packages/babel-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." +}