Skip to content

Commit

Permalink
[New]: jsx-key: added checkKeyMustBeforeSpread option for new jsx…
Browse files Browse the repository at this point in the history
… transform

Fixes #2830.
  • Loading branch information
morlay authored and ljharb committed Oct 19, 2020
1 parent 01c7f5e commit 9f0d5c4
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 2 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,11 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

## Unreleased

### Added
* [`jsx-key`]: added `checkKeyMustBeforeSpread` option for new jsx transform ([#2835][] @morlay)

[#2835]: https://github.com/yannickcr/eslint-plugin-react/pull/2835

## [7.21.5] - 2020.10.19

### Fixed
Expand Down
10 changes: 10 additions & 0 deletions docs/rules/jsx-key.md
Expand Up @@ -47,6 +47,16 @@ The following patterns are considered warnings:
data.map(x => <>{x}</>);
```

### `checkKeyMustBeforeSpread` (default: `false`)

When `true` the rule will check if key prop after spread to avoid [createElement fallback](https://github.com/facebook/react/issues/20031#issuecomment-710346866).

The following patterns are considered warnings:

```jsx
<span {...spread} key={"key-after-spread"} />;
```

## When not to use

If you are not using JSX then you can disable this rule.
Expand Down
29 changes: 28 additions & 1 deletion lib/rules/jsx-key.js
Expand Up @@ -6,6 +6,7 @@
'use strict';

const hasProp = require('jsx-ast-utils/hasProp');
const propName = require('jsx-ast-utils/propName');
const docsUrl = require('../util/docsUrl');
const pragmaUtil = require('../util/pragma');

Expand All @@ -14,7 +15,8 @@ const pragmaUtil = require('../util/pragma');
// ------------------------------------------------------------------------------

const defaultOptions = {
checkFragmentShorthand: false
checkFragmentShorthand: false,
checkKeyMustBeforeSpread: false
};

module.exports = {
Expand All @@ -31,6 +33,10 @@ module.exports = {
checkFragmentShorthand: {
type: 'boolean',
default: defaultOptions.checkFragmentShorthand
},
checkKeyMustBeforeSpread: {
type: 'boolean',
default: defaultOptions.checkKeyMustBeforeSpread
}
},
additionalProperties: false
Expand All @@ -40,6 +46,7 @@ module.exports = {
create(context) {
const options = Object.assign({}, defaultOptions, context.options[0]);
const checkFragmentShorthand = options.checkFragmentShorthand;
const checkKeyMustBeforeSpread = options.checkKeyMustBeforeSpread;
const reactPragma = pragmaUtil.getFromContext(context);
const fragmentPragma = pragmaUtil.getFragmentFromContext(context);

Expand All @@ -61,9 +68,29 @@ module.exports = {
return body.filter((item) => item.type === 'ReturnStatement')[0];
}

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';
});
}

return {
JSXElement(node) {
if (hasProp(node.openingElement.attributes, 'key')) {
if (checkKeyMustBeforeSpread && isKeyAfterSpread(node.openingElement.attributes)) {
context.report({
node,
message: '`key` prop must before any `{...spread}, to avoid conflicting with React’s new JSX transform: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html`'
});
}
return;
}

Expand Down
31 changes: 30 additions & 1 deletion tests/lib/rules/jsx-key.js
Expand Up @@ -48,7 +48,12 @@ ruleTester.run('jsx-key', rule, {
{code: '[1, 2, 3].map(function(x) { return; });'},
{code: 'foo(() => <div />);'},
{code: 'foo(() => <></>);', parser: parsers.BABEL_ESLINT},
{code: '<></>;', parser: parsers.BABEL_ESLINT}
{code: '<></>;', parser: parsers.BABEL_ESLINT},
{code: '<App {...{}} />;', parser: parsers.BABEL_ESLINT},
{code: '<App key="keyBeforeSpread" {...{}} />;', parser: parsers.BABEL_ESLINT, options: [{checkKeyMustBeforeSpread: true}]},
{code: '<App key="keyBeforeSpread" {...{}} />;', parser: parsers.TYPESCRIPT_ESLINT, options: [{checkKeyMustBeforeSpread: true}]},
{code: '<div key="keyBeforeSpread" {...{}} />;', parser: parsers.BABEL_ESLINT, options: [{checkKeyMustBeforeSpread: true}]},
{code: '<div key="keyBeforeSpread" {...{}} />;', parser: parsers.TYPESCRIPT_ESLINT, options: [{checkKeyMustBeforeSpread: true}]}
],
invalid: [].concat({
code: '[<App />];',
Expand Down Expand Up @@ -88,5 +93,29 @@ 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: '[<App {...obj} key="keyAfterSpread" />];',
parser: parsers.BABEL_ESLINT,
options: [{checkKeyMustBeforeSpread: true}],
settings,
errors: [{message: '`key` prop must before any `{...spread}, to avoid conflicting with React’s new JSX transform: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html`'}]
}, {
code: '[<App {...obj} key="keyAfterSpread" />];',
parser: parsers.TYPESCRIPT_ESLINT,
options: [{checkKeyMustBeforeSpread: true}],
settings,
errors: [{message: '`key` prop must before any `{...spread}, to avoid conflicting with React’s new JSX transform: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html`'}]
}, {
code: '[<div {...obj} key="keyAfterSpread" />];',
parser: parsers.BABEL_ESLINT,
options: [{checkKeyMustBeforeSpread: true}],
settings,
errors: [{message: '`key` prop must before any `{...spread}, to avoid conflicting with React’s new JSX transform: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html`'}]
}, {
code: '[<div {...obj} key="keyAfterSpread" />];',
parser: parsers.TYPESCRIPT_ESLINT,
options: [{checkKeyMustBeforeSpread: true}],
settings,
errors: [{message: '`key` prop must before any `{...spread}, to avoid conflicting with React’s new JSX transform: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html`'}]
})
});

0 comments on commit 9f0d5c4

Please sign in to comment.