Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[New]: jsx-key: added checkKeyMustBeforeSpread option for new jsx transform #2835

Merged
merged 1 commit into from Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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`'}]
})
});