Skip to content

Commit

Permalink
[Fix] jsx-key: incorrect behavior for checkKeyMustBeforeSpread with…
Browse files Browse the repository at this point in the history
… map callbacks
  • Loading branch information
akulsr0 authored and ljharb committed Jun 18, 2024
1 parent 393bfa2 commit 6dc7803
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 18 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
* [`boolean-prop-naming`]: avoid a crash with a spread prop ([#3733][] @ljharb)
* [`jsx-boolean-value`]: `assumeUndefinedIsFalse` with `never` must not allow explicit `true` value ([#3757][] @6uliver)
* [`no-object-type-as-default-prop`]: enable rule for components with many parameters ([#3768][] @JulienR1)
* [`jsx-key`]: incorrect behavior for checkKeyMustBeforeSpread with map callbacks ([#3769][] @akulsr0)

[#3769]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3769
[#3768]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3768
[#3762]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3762
[#3757]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3757
Expand Down
42 changes: 24 additions & 18 deletions lib/rules/jsx-key.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,31 @@ module.exports = {
const reactPragma = pragmaUtil.getFromContext(context);
const fragmentPragma = pragmaUtil.getFragmentFromContext(context);

function isKeyAfterSpread(attributes) {
let hasFoundSpread = false;
return attributes.some((attribute) => {
if (attribute.type === 'JSXSpreadAttribute') {
hasFoundSpread = true;
return false;
}
if (attribute.type !== 'JSXAttribute') {
return false;

Check warning on line 84 in lib/rules/jsx-key.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/jsx-key.js#L84

Added line #L84 was not covered by tests
}
return hasFoundSpread && propName(attribute) === 'key';
});
}

function checkIteratorElement(node) {
if (node.type === 'JSXElement' && !hasProp(node.openingElement.attributes, 'key')) {
report(context, messages.missingIterKey, 'missingIterKey', {
node,
});
if (node.type === 'JSXElement') {
if (!hasProp(node.openingElement.attributes, 'key')) {
report(context, messages.missingIterKey, 'missingIterKey', { node });
} else {
const attrs = node.openingElement.attributes;

if (checkKeyMustBeforeSpread && isKeyAfterSpread(attrs)) {
report(context, messages.keyBeforeSpread, 'keyBeforeSpread', { node });
}
}
} else if (checkFragmentShorthand && node.type === 'JSXFragment') {
report(context, messages.missingIterKeyUsePrag, 'missingIterKeyUsePrag', {
node,
Expand Down Expand Up @@ -115,20 +135,6 @@ module.exports = {
return returnStatements;
}

function isKeyAfterSpread(attributes) {
let hasFoundSpread = false;
return attributes.some((attribute) => {
if (attribute.type === 'JSXSpreadAttribute') {
hasFoundSpread = true;
return false;
}
if (attribute.type !== 'JSXAttribute') {
return false;
}
return hasFoundSpread && propName(attribute) === 'key';
});
}

/**
* Checks if the given node is a function expression or arrow function,
* and checks if there is a missing key prop in return statement's arguments
Expand Down
15 changes: 15 additions & 0 deletions tests/lib/rules/jsx-key.js
Original file line number Diff line number Diff line change
Expand Up @@ -409,5 +409,20 @@ ruleTester.run('jsx-key', rule, {
{ messageId: 'missingIterKey' },
],
},
{
code: `
const TestCase = () => {
const list = [1, 2, 3, 4, 5];
return (
<div>
{list.map(x => <div {...spread} key={x} />)}
</div>
);
};
`,
options: [{ checkKeyMustBeforeSpread: true }],
errors: [{ messageId: 'keyBeforeSpread' }],
},
]),
});

0 comments on commit 6dc7803

Please sign in to comment.