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] Add jsx-props-no-spreading rule #2191

Merged
merged 1 commit into from Mar 12, 2019
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -2497,3 +2497,4 @@ If you're still not using React 15 you can keep the old behavior by setting the
[`jsx-props-no-multi-spaces`]: docs/rules/jsx-props-no-multi-spaces.md
[`no-unsafe`]: docs/rules/no-unsafe.md
[`jsx-fragments`]: docs/rules/jsx-fragments.md
[`jsx-props-no-spreading`]: docs/rules/jsx-props-no-spreading.md
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -172,6 +172,7 @@ Enable the rules that you would like to use.
* [react/jsx-fragments](docs/rules/jsx-fragments.md): Enforce shorthand or standard form for React fragments
* [react/jsx-pascal-case](docs/rules/jsx-pascal-case.md): Enforce PascalCase for user-defined JSX components
* [react/jsx-props-no-multi-spaces](docs/rules/jsx-props-no-multi-spaces.md): Disallow multiple spaces between inline JSX props (fixable)
* [react/jsx-props-no-spreading](docs/rules/jsx-props-no-spreading.md): Disallow JSX props spreading
* [react/jsx-sort-default-props](docs/rules/jsx-sort-default-props.md): Enforce default props alphabetical sorting
* [react/jsx-sort-props](docs/rules/jsx-sort-props.md): Enforce props alphabetical sorting (fixable)
* [react/jsx-space-before-closing](docs/rules/jsx-space-before-closing.md): Validate spacing before closing bracket in JSX (fixable)
Expand Down
109 changes: 109 additions & 0 deletions docs/rules/jsx-props-no-spreading.md
@@ -0,0 +1,109 @@
# Disallow JSX props spreading (react/jsx-props-no-spreading)

Enforces that there is no spreading for any JSX attribute. This enhances readability of code by being more explicit about what props are received by the component. It is also good for maintainability by avoiding passing unintentional extra props and allowing react to emit warnings when invalid HTML props are passed to HTML elements.

## Rule Details

The following patterns are considered warnings:

```jsx
<App {...props} />
<MyCustomComponent {...props} some_other_prop={some_other_prop} />
<img {...props} />
```

The following patterns are **not** considered warnings:

```jsx
const {src, alt} = props;
const {one_prop, two_prop} = otherProps;
<MyCustomComponent one_prop={one_prop} two_prop={two_prop} />
<img src={src} alt={alt} />
```


## Rule Options

```js
...
"react/jsx-props-no-spreading": [{
"html": "ignore" / "enforce",
"custom": "ignore" / "enforce",
"exceptions": [<string>]
}]
...
```

### html

`html` set to `ignore` will ignore all html jsx tags like `div`, `img` etc. Default is set to `enforce`.

The following patterns are **not** considered warnings when `html` is set to `ignore`:

```jsx
<img {...props} />
```

The following patterns are still considered warnings:

```jsx
<MyCustomComponent {...props} />
```

### custom

`custom` set to `ignore` will ignore all custom jsx tags like `App`, `MyCustomComponent` etc. Default is set to `enforce`.

The following patterns are **not** considered warnings when `custom` is set to `ignore`:

```jsx
<MyCustomComponent {...props} />
```

The following patterns are still considered warnings:
```jsx
<img {...props} />
```

### exceptions

An "exception" will always flip the resulting html or custom setting for that component - ie, html set to `ignore`, with an exception of `div` will enforce on an `div`; custom set to `enforce` with an exception of `Foo` will ignore `Foo`.

```js
{ "exceptions": ["Image", "img"] }
```

The following patterns are **not** considered warnings:

```jsx
const {src, alt} = props;
<Image {...props} />
<img {...props} />
```

The following patterns are considered warnings:
```jsx
<MyCustomComponent {...props} />
```

```js
{ "html": "ignore", "exceptions": ["MyCustomComponent", "img"] }
```

The following patterns are **not** considered warnings:

```jsx
const {src, alt} = props;
const {one_prop, two_prop} = otherProps;
<img src={src} alt={alt} />
<MyCustomComponent {...otherProps} />
```

The following patterns are considered warnings:
```jsx
<img {...props} />
```

## When Not To Use It

If you are not using JSX or have lots of props to be passed or the props spreading is used inside HOC.
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -38,6 +38,7 @@ const allRules = {
'jsx-pascal-case': require('./lib/rules/jsx-pascal-case'),
'jsx-fragments': require('./lib/rules/jsx-fragments'),
'jsx-props-no-multi-spaces': require('./lib/rules/jsx-props-no-multi-spaces'),
'jsx-props-no-spreading': require('./lib/rules/jsx-props-no-spreading'),
'jsx-sort-default-props': require('./lib/rules/jsx-sort-default-props'),
'jsx-sort-props': require('./lib/rules/jsx-sort-props'),
'jsx-space-before-closing': require('./lib/rules/jsx-space-before-closing'),
Expand Down
96 changes: 96 additions & 0 deletions lib/rules/jsx-props-no-spreading.js
@@ -0,0 +1,96 @@
/**
* @fileoverview Prevent JSX prop spreading
* @author Ashish Gambhir
*/
'use strict';

const docsUrl = require('../util/docsUrl');

// ------------------------------------------------------------------------------
// Constants
// ------------------------------------------------------------------------------

const OPTIONS = {ignore: 'ignore', enforce: 'enforce'};
const DEFAULTS = {html: OPTIONS.enforce, custom: OPTIONS.enforce, exceptions: []};

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'Prevent JSX prop spreading',
category: 'Best Practices',
recommended: false,
url: docsUrl('jsx-props-no-spreading')
},
schema: [{
allOf: [{
type: 'object',
properties: {
html: {
enum: [OPTIONS.enforce, OPTIONS.ignore]
},
custom: {
enum: [OPTIONS.enforce, OPTIONS.ignore]
},
exceptions: {
type: 'array',
items: {
type: 'string',
uniqueItems: true
}
}
}
}, {
not: {
type: 'object',
required: ['html', 'custom'],
properties: {
html: {
enum: [OPTIONS.ignore]
},
custom: {
enum: [OPTIONS.ignore]
},
exceptions: {
type: 'array',
minItems: 0,
maxItems: 0
}
}
}
}]
}]
},

create: function (context) {
const configuration = context.options[0] || {};
const ignoreHtmlTags = (configuration.html || DEFAULTS.html) === OPTIONS.ignore;
const ignoreCustomTags = (configuration.custom || DEFAULTS.custom) === OPTIONS.ignore;
const exceptions = configuration.exceptions || DEFAULTS.exceptions;
const isException = (tag, allExceptions) => allExceptions.indexOf(tag) !== -1;
return {
JSXSpreadAttribute: function (node) {
const tagName = node.parent.name.name;
const isHTMLTag = tagName && tagName[0] !== tagName[0].toUpperCase();
const isCustomTag = tagName && tagName[0] === tagName[0].toUpperCase();
if (isHTMLTag &&
((ignoreHtmlTags && !isException(tagName, exceptions)) ||
(!ignoreHtmlTags && isException(tagName, exceptions)))) {
return;
}
if (isCustomTag &&
((ignoreCustomTags && !isException(tagName, exceptions)) ||
(!ignoreCustomTags && isException(tagName, exceptions)))) {
return;
}
context.report({
node: node,
message: 'Prop spreading is forbidden'
});
}
};
}
};
157 changes: 157 additions & 0 deletions tests/lib/rules/jsx-props-no-spreading.js
@@ -0,0 +1,157 @@
/**
* @fileoverview Tests for jsx-props-no-spreading
*/
'use strict';

// -----------------------------------------------------------------------------
// Requirements
// -----------------------------------------------------------------------------

const rule = require('../../../lib/rules/jsx-props-no-spreading');
const RuleTester = require('eslint').RuleTester;

const parserOptions = {
ecmaVersion: 2018,
sourceType: 'module',
ecmaFeatures: {
jsx: true
}
};

// -----------------------------------------------------------------------------
// Tests
// -----------------------------------------------------------------------------

const ruleTester = new RuleTester({parserOptions});
const expectedError = {message: 'Prop spreading is forbidden'};

ruleTester.run('jsx-props-no-spreading', rule, {
valid: [{
code: [
'const {one_prop, two_prop} = props;',
'<App one_prop={one_prop} two_prop={two_prop}/>'
].join('\n')
}, {
code: [
'const {one_prop, two_prop} = props;',
'<div one_prop={one_prop} two_prop={two_prop}></div>'
].join('\n')
}, {
code: [
'const newProps = {...props};',
'<App one_prop={newProps.one_prop} two_prop={newProps.two_prop} style={{...styles}}/>'
].join('\n')
}, {
code: [
'const props = {src: "dummy.jpg", alt: "dummy"};',
'<App>',
' <Image {...props}/>',
' <img {...props}/>',
'</App>'
].join('\n'),
options: [{exceptions: ['Image', 'img']}]
}, {
code: [
'const props = {src: "dummy.jpg", alt: "dummy"};',
'const { src, alt } = props;',
'<App>',
' <Image {...props}/>',
' <img src={src} alt={alt}/>',
'</App>'
].join('\n'),
options: [{custom: 'ignore'}]
}, {
code: [
'const props = {src: "dummy.jpg", alt: "dummy"};',
'const { src, alt } = props;',
'<App>',
' <Image {...props}/>',
' <img {...props}/>',
'</App>'
].join('\n'),
options: [{custom: 'enforce', html: 'ignore', exceptions: ['Image']}]
}, {
code: [
'const props = {src: "dummy.jpg", alt: "dummy"};',
'const { src, alt } = props;',
'<App>',
' <img {...props}/>',
' <Image src={src} alt={alt}/>',
' <div {...someOtherProps}/>',
'</App>'
].join('\n'),
options: [{html: 'ignore'}]
}],

invalid: [{
code: [
'<App {...props}/>'
].join('\n'),
errors: [expectedError]
}, {
code: [
'<div {...props}></div>'
].join('\n'),
errors: [expectedError]
}, {
code: [
'<App {...props} some_other_prop={some_other_prop}/>'
].join('\n'),
errors: [expectedError]
}, {
code: [
'const props = {src: "dummy.jpg", alt: "dummy"};',
'<App>',
' <Image {...props}/>',
' <span {...props}/>',
'</App>'
].join('\n'),
options: [{exceptions: ['Image', 'img']}],
errors: [expectedError]
}, {
code: [
'const props = {src: "dummy.jpg", alt: "dummy"};',
'const { src, alt } = props;',
'<App>',
' <Image {...props}/>',
' <img {...props}/>',
'</App>'
].join('\n'),
options: [{custom: 'ignore'}],
errors: [expectedError]
}, {
code: [
'const props = {src: "dummy.jpg", alt: "dummy"};',
'const { src, alt } = props;',
'<App>',
' <Image {...props}/>',
' <img {...props}/>',
'</App>'
].join('\n'),
options: [{html: 'ignore', exceptions: ['Image', 'img']}],
errors: [expectedError]
}, {
code: [
'const props = {src: "dummy.jpg", alt: "dummy"};',
'const { src, alt } = props;',
'<App>',
' <Image {...props}/>',
' <img {...props}/>',
' <div {...props}/>',
'</App>'
].join('\n'),
options: [{custom: 'ignore', html: 'ignore', exceptions: ['Image', 'img']}],
errors: [expectedError, expectedError]
}, {
code: [
'const props = {src: "dummy.jpg", alt: "dummy"};',
'const { src, alt } = props;',
'<App>',
' <img {...props}/>',
' <Image {...props}/>',
'</App>'
].join('\n'),
options: [{html: 'ignore'}],
errors: [expectedError]
}]
});