From 1ae0a993b5903e41cbd43d92170aaecde3608122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20V=C3=A1zquez=20P=C3=BAa?= Date: Wed, 22 Nov 2017 11:10:55 +0100 Subject: [PATCH 1/4] Add a forbid-dom-props rule A new rule forbid-dom-props was added. It is based on forbid-component-props but it only forbid property on DOM Node elements. This can be useful to enforce some good practices, as not using 'id' attributes or inline styles. --- README.md | 1 + docs/rules/forbid-dom-props.md | 45 ++++++++++++ index.js | 1 + lib/rules/forbid-dom-props.js | 68 ++++++++++++++++++ tests/lib/rules/forbid-dom-props.js | 104 ++++++++++++++++++++++++++++ 5 files changed, 219 insertions(+) create mode 100644 docs/rules/forbid-dom-props.md create mode 100644 lib/rules/forbid-dom-props.js create mode 100644 tests/lib/rules/forbid-dom-props.js diff --git a/README.md b/README.md index 9084d6ad27..1151b6136c 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](# * [react/destructuring-assignment](docs/rules/destructuring-assignment.md): Rule enforces consistent usage of destructuring assignment in component * [react/display-name](docs/rules/display-name.md): Prevent missing `displayName` in a React component definition * [react/forbid-component-props](docs/rules/forbid-component-props.md): Forbid certain props on Components +* [react/forbid-dom-props](docs/rules/forbid-dom-props.md): Forbid certain props on DOM Nodes * [react/forbid-elements](docs/rules/forbid-elements.md): Forbid certain elements * [react/forbid-prop-types](docs/rules/forbid-prop-types.md): Forbid certain propTypes * [react/forbid-foreign-prop-types](docs/rules/forbid-foreign-prop-types.md): Forbid foreign propTypes diff --git a/docs/rules/forbid-dom-props.md b/docs/rules/forbid-dom-props.md new file mode 100644 index 0000000000..9453758bc8 --- /dev/null +++ b/docs/rules/forbid-dom-props.md @@ -0,0 +1,45 @@ +# Forbid certain props on DOM Nodes (react/forbid-dom-props) + +This rule prevents passing of props to elements. This rule only applies to DOM Nodes (e.g. `
`) and not Components (e.g. ``). +The list of forbidden props can be customized with the `forbid` option. + +## Rule Details + +This rule checks all JSX elements and verifies that no forbidden props are used +on DOM Nodes. This rule is off by default. + +The following patterns are considered warnings: + +```jsx +// [1, { "forbid": ["id"] }] +
+``` + +```jsx +// [1, { "forbid": ["style"] }] +
+``` + +The following patterns are **not** considered warnings: + +```jsx +// [1, { "forbid": ["id"] }] + +``` + +```jsx +// [1, { "forbid": ["id"] }] + +``` + +## Rule Options + +```js +... +"react/forbid-dom-props": [, { "forbid": [] }] +... +``` + +### `forbid` + +An array of strings, with the names of props that are forbidden. The default value of this option `[]`. diff --git a/index.js b/index.js index 6c1a933ac9..6207992b8b 100644 --- a/index.js +++ b/index.js @@ -8,6 +8,7 @@ const allRules = { 'destructuring-assignment': require('./lib/rules/destructuring-assignment'), 'display-name': require('./lib/rules/display-name'), 'forbid-component-props': require('./lib/rules/forbid-component-props'), + 'forbid-dom-props': require('./lib/rules/forbid-dom-props'), 'forbid-elements': require('./lib/rules/forbid-elements'), 'forbid-prop-types': require('./lib/rules/forbid-prop-types'), 'forbid-foreign-prop-types': require('./lib/rules/forbid-foreign-prop-types'), diff --git a/lib/rules/forbid-dom-props.js b/lib/rules/forbid-dom-props.js new file mode 100644 index 0000000000..0bbd81116d --- /dev/null +++ b/lib/rules/forbid-dom-props.js @@ -0,0 +1,68 @@ +/** + * @fileoverview Forbid certain props on DOM Nodes + * @author David Vázquez + */ +'use strict'; + +// ------------------------------------------------------------------------------ +// Constants +// ------------------------------------------------------------------------------ + +const DEFAULTS = []; + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'Forbid certain props on DOM Nodes', + category: 'Best Practices', + recommended: false + }, + + schema: [{ + type: 'object', + properties: { + forbid: { + type: 'array', + items: { + type: 'string' + } + } + }, + additionalProperties: true + }] + }, + + create: function(context) { + function isForbidden(prop) { + const configuration = context.options[0] || {}; + + const forbid = configuration.forbid || DEFAULTS; + return forbid.indexOf(prop) >= 0; + } + + return { + JSXAttribute: function(node) { + const tag = node.parent.name.name; + if (!(tag && tag[0] !== tag[0].toUpperCase())) { + // This is a Component, not a DOM node, so exit. + return; + } + + const prop = node.name.name; + + if (!isForbidden(prop)) { + return; + } + + context.report({ + node: node, + message: `Prop \`${prop}\` is forbidden on DOM Nodes` + }); + } + }; + } +}; diff --git a/tests/lib/rules/forbid-dom-props.js b/tests/lib/rules/forbid-dom-props.js new file mode 100644 index 0000000000..6711d4ea59 --- /dev/null +++ b/tests/lib/rules/forbid-dom-props.js @@ -0,0 +1,104 @@ +/** + * @fileoverview Tests for forbid-dom-props + */ +'use strict'; + +// ----------------------------------------------------------------------------- +// Requirements +// ----------------------------------------------------------------------------- + +const rule = require('../../../lib/rules/forbid-dom-props'); +const RuleTester = require('eslint').RuleTester; + +const parserOptions = { + ecmaVersion: 8, + sourceType: 'module', + ecmaFeatures: { + experimentalObjectRestSpread: true, + jsx: true + } +}; + +require('babel-eslint'); + +// ----------------------------------------------------------------------------- +// Tests +// ----------------------------------------------------------------------------- + +const ID_ERROR_MESSAGE = 'Prop `id` is forbidden on DOM Nodes'; + +const ruleTester = new RuleTester({parserOptions}); +ruleTester.run('forbid-element-props', rule, { + + valid: [{ + code: [ + 'var First = createReactClass({', + ' render: function() {', + ' return ;', + ' }', + '});' + ].join('\n'), + options: [{forbid: ['id']}] + }, { + code: [ + 'var First = createReactClass({', + ' propTypes: externalPropTypes,', + ' render: function() {', + ' return ;', + ' }', + '});' + ].join('\n'), + options: [{forbid: ['style', 'id']}] + }, { + code: [ + 'var First = createReactClass({', + ' propTypes: externalPropTypes,', + ' render: function() {', + ' return ;', + ' }', + '});' + ].join('\n'), + options: [{forbid: ['id']}] + }, { + code: [ + 'class First extends createReactClass {', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n'), + options: [{forbid: ['id']}] + }, { + code: [ + 'const First = (props) => (', + ' ', + ');' + ].join('\n'), + options: [{forbid: ['id']}] + }, { + code: [ + 'const First = (props) => (', + '
', + ');' + ].join('\n'), + options: [{forbid: ['id']}] + }], + + invalid: [{ + code: [ + 'var First = createReactClass({', + ' propTypes: externalPropTypes,', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{forbid: ['id']}], + errors: [{ + message: ID_ERROR_MESSAGE, + line: 4, + column: 17, + type: 'JSXAttribute' + }] + }] +}); From 2c2d8310f5a914d75ea246f516a7e829ee0f439f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20V=C3=A1zquez=20P=C3=BAa?= Date: Wed, 22 Nov 2017 17:23:09 +0100 Subject: [PATCH 2/4] Add related rules link to both forbid-dom-props and forbid-component-props --- docs/rules/forbid-component-props.md | 5 +++++ docs/rules/forbid-dom-props.md | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/docs/rules/forbid-component-props.md b/docs/rules/forbid-component-props.md index 424d880f0b..eb557df8c2 100644 --- a/docs/rules/forbid-component-props.md +++ b/docs/rules/forbid-component-props.md @@ -42,3 +42,8 @@ The following patterns are **not** considered warnings: ### `forbid` An array of strings, with the names of props that are forbidden. The default value of this option is `['className', 'style']`. + + +### Related rules + +- [forbid-dom-props](./forbid-dom-props.md) diff --git a/docs/rules/forbid-dom-props.md b/docs/rules/forbid-dom-props.md index 9453758bc8..8dca2826ae 100644 --- a/docs/rules/forbid-dom-props.md +++ b/docs/rules/forbid-dom-props.md @@ -43,3 +43,8 @@ The following patterns are **not** considered warnings: ### `forbid` An array of strings, with the names of props that are forbidden. The default value of this option `[]`. + + +### Related rules + +- [forbid-component-props](./forbid-component-props.md) From 28654e29b1286d7166f7237a5b4107f5d9045d63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20V=C3=A1zquez=20P=C3=BAa?= Date: Wed, 22 Nov 2017 17:28:15 +0100 Subject: [PATCH 3/4] Add more invalid tests cases for forbid-dom-props --- tests/lib/rules/forbid-dom-props.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/lib/rules/forbid-dom-props.js b/tests/lib/rules/forbid-dom-props.js index 6711d4ea59..0e2b021ee8 100644 --- a/tests/lib/rules/forbid-dom-props.js +++ b/tests/lib/rules/forbid-dom-props.js @@ -100,5 +100,33 @@ ruleTester.run('forbid-element-props', rule, { column: 17, type: 'JSXAttribute' }] + }, { + code: [ + 'class First extends createReactClass {', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + options: [{forbid: ['id']}], + errors: [{ + message: ID_ERROR_MESSAGE, + line: 3, + column: 17, + type: 'JSXAttribute' + }] + }, { + code: [ + 'const First = (props) => (', + '
', + ');' + ].join('\n'), + options: [{forbid: ['id']}], + errors: [{ + message: ID_ERROR_MESSAGE, + line: 2, + column: 8, + type: 'JSXAttribute' + }] }] }); From 268a704437e73597eb9385c5153ece3312620fe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20V=C3=A1zquez=20P=C3=BAa?= Date: Thu, 23 Nov 2017 09:39:32 +0100 Subject: [PATCH 4/4] Refine schema for forbid-dom-props options --- lib/rules/forbid-dom-props.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/rules/forbid-dom-props.js b/lib/rules/forbid-dom-props.js index 0bbd81116d..324f6f27a0 100644 --- a/lib/rules/forbid-dom-props.js +++ b/lib/rules/forbid-dom-props.js @@ -28,11 +28,13 @@ module.exports = { forbid: { type: 'array', items: { - type: 'string' - } + type: 'string', + minLength: 1 + }, + uniqueItems: true } }, - additionalProperties: true + additionalProperties: false }] },