Skip to content

Commit 986e00e

Browse files
lencionilencioni
committed
Add rule to forbid className and style being passed to Components
Passing `className` or `style` to your Components is a source of much hidden complexity that can be solved either by using a wrapper to add styling or by adding props for specific styling. This rule aims at preventing these props from being passed to your Components, to help minimize hidden complexity. The original issue proposed having this rule check propTypes. I considered that route, but I worry that it won't be as effective for people who do not have every prop used by the component listed in the propTypes. Additionally, propTypes seem to have waning interest from the React team in favor of Flow annotations, so I think in the long run checking for this at the callsite will be more stable. More information can be found by reading: https://medium.com/brigade-engineering/don-t-pass-css-classes-between-components-e9f7ab192785 Also: https://twitter.com/sebmarkbage/status/598971268403073024 Fixes #314
1 parent 0b2dcc6 commit 986e00e

File tree

5 files changed

+258
-0
lines changed

5 files changed

+258
-0
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#
8080
# List of supported rules
8181

8282
* [react/display-name](docs/rules/display-name.md): Prevent missing `displayName` in a React component definition
83+
* [react/forbid-component-props](docs/rules/forbid-component-props.md): Forbid certain props on Components
8384
* [react/forbid-prop-types](docs/rules/forbid-prop-types.md): Forbid certain propTypes
8485
* [react/no-danger](docs/rules/no-danger.md): Prevent usage of dangerous JSX properties
8586
* [react/no-deprecated](docs/rules/no-deprecated.md): Prevent usage of deprecated methods

‎docs/rules/forbid-component-props.md‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Forbid certain props on Components (forbid-component-props)
2+
3+
By default this rule prevents passing of [props that add lots of complexity](https://medium.com/brigade-engineering/don-t-pass-css-classes-between-components-e9f7ab192785) (`className`, `style`) to Components. This rule only applies to Components (e.g. `<Foo />`) and not DOM nodes (e.g. `<div />`). The list of forbidden props can be customized with the `forbid` option.
4+
5+
## Rule Details
6+
7+
This rule checks all JSX elements and verifies that no forbidden props are used
8+
on Components. This rule is off by default.
9+
10+
The following patterns are considered warnings:
11+
12+
```jsx
13+
<Hello className='foo' />
14+
```
15+
16+
```jsx
17+
<Hello style={{color: 'red'}} />
18+
```
19+
20+
The following patterns are not considered warnings:
21+
22+
```jsx
23+
<Hello name='Joe' />
24+
```
25+
26+
```jsx
27+
<div className='foo' />
28+
```
29+
30+
```jsx
31+
<div style={{color: 'red'}} />
32+
```
33+
34+
## Rule Options
35+
36+
```js
37+
...
38+
"forbid-component-props": [<enabled>, { "forbid": [<string>] }]
39+
...
40+
```
41+
42+
### `forbid`
43+
44+
An array of strings, with the names of props that are forbidden.

‎index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ var rules = {
4040
'jsx-closing-bracket-location': require('./lib/rules/jsx-closing-bracket-location'),
4141
'jsx-space-before-closing': require('./lib/rules/jsx-space-before-closing'),
4242
'no-direct-mutation-state': require('./lib/rules/no-direct-mutation-state'),
43+
'forbid-component-props': require('./lib/rules/forbid-component-props'),
4344
'forbid-prop-types': require('./lib/rules/forbid-prop-types'),
4445
'prefer-es6-class': require('./lib/rules/prefer-es6-class'),
4546
'jsx-key': require('./lib/rules/jsx-key'),

‎lib/rules/forbid-component-props.js‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* @fileoverview Forbid certain props on components
3+
* @author Joe Lencioni
4+
*/
5+
'use strict';
6+
7+
// ------------------------------------------------------------------------------
8+
// Constants
9+
// ------------------------------------------------------------------------------
10+
11+
var DEFAULTS = ['className', 'style'];
12+
13+
// ------------------------------------------------------------------------------
14+
// Rule Definition
15+
// ------------------------------------------------------------------------------
16+
17+
module.exports = function(context) {
18+
19+
function isForbidden(prop) {
20+
var configuration = context.options[0] || {};
21+
22+
var forbid = configuration.forbid || DEFAULTS;
23+
return forbid.indexOf(prop) >= 0;
24+
}
25+
26+
return {
27+
JSXAttribute: function(node) {
28+
var tag = node.parent.name.name;
29+
if (tag[0] !== tag[0].toUpperCase()) {
30+
// This is a DOM node, not a Component, so exit.
31+
return;
32+
}
33+
34+
var prop = node.name.name;
35+
36+
if (!isForbidden(prop)) {
37+
return;
38+
}
39+
40+
context.report({
41+
node: node,
42+
message: 'Prop `' + prop + '` is forbidden on Components'
43+
});
44+
}
45+
};
46+
};
47+
48+
module.exports.schema = [{
49+
type: 'object',
50+
properties: {
51+
forbid: {
52+
type: 'array',
53+
items: {
54+
type: 'string'
55+
}
56+
}
57+
},
58+
additionalProperties: true
59+
}];

‎tests/lib/rules/forbid-component-props.js‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
/**
2+
* @fileoverview Tests for forbid-component-props
3+
*/
4+
'use strict';
5+
6+
// -----------------------------------------------------------------------------
7+
// Requirements
8+
// -----------------------------------------------------------------------------
9+
10+
var rule = require('../../../lib/rules/forbid-component-props');
11+
var RuleTester = require('eslint').RuleTester;
12+
13+
var parserOptions = {
14+
ecmaVersion: 6,
15+
ecmaFeatures: {
16+
experimentalObjectRestSpread: true,
17+
jsx: true
18+
}
19+
};
20+
21+
require('babel-eslint');
22+
23+
// -----------------------------------------------------------------------------
24+
// Tests
25+
// -----------------------------------------------------------------------------
26+
27+
var CLASSNAME_ERROR_MESSAGE = 'Prop `className` is forbidden on Components';
28+
var STYLE_ERROR_MESSAGE = 'Prop `style` is forbidden on Components';
29+
30+
var ruleTester = new RuleTester();
31+
ruleTester.run('forbid-component-props', rule, {
32+
33+
valid: [{
34+
code: [
35+
'var First = React.createClass({',
36+
' render: function() {',
37+
' return <div className="foo" />;',
38+
' }',
39+
'});'
40+
].join('\n'),
41+
parserOptions: parserOptions
42+
}, {
43+
code: [
44+
'var First = React.createClass({',
45+
' render: function() {',
46+
' return <div style={{color: "red"}} />;',
47+
' }',
48+
'});'
49+
].join('\n'),
50+
options: [{forbid: ['style']}],
51+
parserOptions: parserOptions
52+
}, {
53+
code: [
54+
'var First = React.createClass({',
55+
' propTypes: externalPropTypes,',
56+
' render: function() {',
57+
' return <Foo bar="baz" />;',
58+
' }',
59+
'});'
60+
].join('\n'),
61+
parserOptions: parserOptions
62+
}, {
63+
code: [
64+
'var First = React.createClass({',
65+
' propTypes: externalPropTypes,',
66+
' render: function() {',
67+
' return <Foo className="bar" />;',
68+
' }',
69+
'});'
70+
].join('\n'),
71+
options: [{forbid: ['style']}],
72+
parserOptions: parserOptions
73+
}, {
74+
code: [
75+
'var First = React.createClass({',
76+
' propTypes: externalPropTypes,',
77+
' render: function() {',
78+
' return <Foo className="bar" />;',
79+
' }',
80+
'});'
81+
].join('\n'),
82+
options: [{forbid: ['style', 'foo']}],
83+
parserOptions: parserOptions
84+
}],
85+
86+
invalid: [{
87+
code: [
88+
'var First = React.createClass({',
89+
' propTypes: externalPropTypes,',
90+
' render: function() {',
91+
' return <Foo className="bar" />;',
92+
' }',
93+
'});'
94+
].join('\n'),
95+
parserOptions: parserOptions,
96+
errors: [{
97+
message: CLASSNAME_ERROR_MESSAGE,
98+
line: 4,
99+
column: 17,
100+
type: 'JSXAttribute'
101+
}]
102+
}, {
103+
code: [
104+
'var First = React.createClass({',
105+
' propTypes: externalPropTypes,',
106+
' render: function() {',
107+
' return <Foo style={{color: "red"}} />;',
108+
' }',
109+
'});'
110+
].join('\n'),
111+
parserOptions: parserOptions,
112+
errors: [{
113+
message: STYLE_ERROR_MESSAGE,
114+
line: 4,
115+
column: 17,
116+
type: 'JSXAttribute'
117+
}]
118+
}, {
119+
code: [
120+
'var First = React.createClass({',
121+
' propTypes: externalPropTypes,',
122+
' render: function() {',
123+
' return <Foo className="bar" />;',
124+
' }',
125+
'});'
126+
].join('\n'),
127+
parserOptions: parserOptions,
128+
options: [{forbid: ['className', 'style']}],
129+
errors: [{
130+
message: CLASSNAME_ERROR_MESSAGE,
131+
line: 4,
132+
column: 17,
133+
type: 'JSXAttribute'
134+
}]
135+
}, {
136+
code: [
137+
'var First = React.createClass({',
138+
' propTypes: externalPropTypes,',
139+
' render: function() {',
140+
' return <Foo style={{color: "red"}} />;',
141+
' }',
142+
'});'
143+
].join('\n'),
144+
parserOptions: parserOptions,
145+
options: [{forbid: ['className', 'style']}],
146+
errors: [{
147+
message: STYLE_ERROR_MESSAGE,
148+
line: 4,
149+
column: 17,
150+
type: 'JSXAttribute'
151+
}]
152+
}]
153+
});

0 commit comments

Comments
 (0)