From 27cb8bf379333d78d7a5bb7d201e533c61fcbf7d Mon Sep 17 00:00:00 2001 From: Jeremy Walker Date: Mon, 8 Jun 2020 08:59:33 -0700 Subject: [PATCH 1/2] Added support (without options ... which are still to come) for spread (ie. ...) arguments with keys to be considered as valid as explicit key attributes Part of https://github.com/yannickcr/eslint-plugin-react/issues/1753 --- lib/rules/jsx-key.js | 20 +++++++++++++++++++- tests/lib/rules/jsx-key.js | 2 ++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/rules/jsx-key.js b/lib/rules/jsx-key.js index 25405bb13b..467d39cf79 100644 --- a/lib/rules/jsx-key.js +++ b/lib/rules/jsx-key.js @@ -45,7 +45,25 @@ module.exports = { const fragmentPragma = pragmaUtil.getFragmentFromContext(context); function checkIteratorElement(node) { - if (node.type === 'JSXElement' && !hasProp(node.openingElement.attributes, 'key')) { + if (node.type === 'JSXElement') { + const hasAKeyAttribute = hasProp(node.openingElement.attributes, 'key'); + + const hasASpreadArgumentWithAKeyProperty = node.openingElement + && node.openingElement.attributes + && node.openingElement.attributes.some( + ({argument}) => argument && argument.properties && argument.properties.some( + (property) => property.key.name === 'key')); + + const hasAnObjectSpreadArgument = node.openingElement + && node.openingElement.attributes + && node.openingElement.attributes.some( + ({argument}) => argument && argument.type === 'Identifier'); + + const isValidElement = hasAKeyAttribute + || hasASpreadArgumentWithAKeyProperty + || hasAnObjectSpreadArgument; + if (isValidElement) return; + context.report({ node, message: 'Missing "key" prop for element in iterator' diff --git a/tests/lib/rules/jsx-key.js b/tests/lib/rules/jsx-key.js index 49632f59e9..50d8d7660c 100644 --- a/tests/lib/rules/jsx-key.js +++ b/tests/lib/rules/jsx-key.js @@ -39,6 +39,8 @@ ruleTester.run('jsx-key', rule, { {code: 'fn()'}, {code: '[1, 2, 3].map(function () {})'}, {code: ';'}, + {code: '[1, 2, 3].map(x => );'}, + {code: '[1, 2, 3].map(x => );'}, {code: '[, ];'}, {code: '[1, 2, 3].map(function(x) { return });'}, {code: '[1, 2, 3].map(x => );'}, From 8be1cd39696d5b52d1b45cf561985ecc47ace4ce Mon Sep 17 00:00:00 2001 From: Jeremy Walker Date: Tue, 9 Jun 2020 10:11:42 -0700 Subject: [PATCH 2/2] Added allowSpreadKeys option to control whether spread-based keys are allowed --- lib/rules/jsx-key.js | 12 +++++++++--- tests/lib/rules/jsx-key.js | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/rules/jsx-key.js b/lib/rules/jsx-key.js index 467d39cf79..8a05a31621 100644 --- a/lib/rules/jsx-key.js +++ b/lib/rules/jsx-key.js @@ -15,7 +15,8 @@ const pragmaUtil = require('../util/pragma'); // ------------------------------------------------------------------------------ const defaultOptions = { - checkFragmentShorthand: false + checkFragmentShorthand: false, + allowSpreadKeys: true // TODO: When React separates keys from props this should become false }; module.exports = { @@ -32,6 +33,10 @@ module.exports = { checkFragmentShorthand: { type: 'boolean', default: defaultOptions.checkFragmentShorthand + }, + allowSpreadKeys: { + type: 'boolean', + default: defaultOptions.allowSpreadKeys } }, additionalProperties: false @@ -41,6 +46,7 @@ module.exports = { create(context) { const options = Object.assign({}, defaultOptions, context.options[0]); const checkFragmentShorthand = options.checkFragmentShorthand; + const allowSpreadKeys = options.allowSpreadKeys; const reactPragma = pragmaUtil.getFromContext(context); const fragmentPragma = pragmaUtil.getFragmentFromContext(context); @@ -48,13 +54,13 @@ module.exports = { if (node.type === 'JSXElement') { const hasAKeyAttribute = hasProp(node.openingElement.attributes, 'key'); - const hasASpreadArgumentWithAKeyProperty = node.openingElement + const hasASpreadArgumentWithAKeyProperty = allowSpreadKeys && node.openingElement && node.openingElement.attributes && node.openingElement.attributes.some( ({argument}) => argument && argument.properties && argument.properties.some( (property) => property.key.name === 'key')); - const hasAnObjectSpreadArgument = node.openingElement + const hasAnObjectSpreadArgument = allowSpreadKeys && node.openingElement && node.openingElement.attributes && node.openingElement.attributes.some( ({argument}) => argument && argument.type === 'Identifier'); diff --git a/tests/lib/rules/jsx-key.js b/tests/lib/rules/jsx-key.js index 50d8d7660c..a3a983ddd0 100644 --- a/tests/lib/rules/jsx-key.js +++ b/tests/lib/rules/jsx-key.js @@ -39,8 +39,14 @@ ruleTester.run('jsx-key', rule, { {code: 'fn()'}, {code: '[1, 2, 3].map(function () {})'}, {code: ';'}, - {code: '[1, 2, 3].map(x => );'}, - {code: '[1, 2, 3].map(x => );'}, + { + code: '[1, 2, 3].map(x => );', + options: [{allowSpreadKeys: true}] + }, + { + code: '[1, 2, 3].map(x => );', + options: [{allowSpreadKeys: true}] + }, {code: '[, ];'}, {code: '[1, 2, 3].map(function(x) { return });'}, {code: '[1, 2, 3].map(x => );'}, @@ -90,5 +96,14 @@ ruleTester.run('jsx-key', rule, { options: [{checkFragmentShorthand: true}], settings, errors: [{message: 'Missing "key" prop for element in array. Shorthand fragment syntax does not support providing keys. Use Act.Frag instead'}] + }, { + code: '[1, 2, 3].map(x => );', + options: [{allowSpreadKeys: false}], + errors: [{message: 'Missing "key" prop for element in iterator'}] + }, + { + code: '[1, 2, 3].map(x => );', + options: [{allowSpreadKeys: false}], + errors: [{message: 'Missing "key" prop for element in iterator'}] }) });