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] forbid-dom-props: add disallowedFor option #3338

Merged
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange

### Added
* [`jsx-newline`]: add `allowMultiline` option when prevent option is true ([#3311][] @TildaDares)
* [`forbid-dom-props`]: add `disallowedFor` option ([#3338][] @TildaDares)

### Fixed
* [`jsx-no-literals`]: properly error on children with noAttributeStrings: true ([#3317][] @TildaDares)
Expand All @@ -32,6 +33,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
[#3347]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3347
[#3344]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3344
[#3339]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3339
[#3338]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3338
[#3335]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3335
[#3331]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3331
[#3328]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3328
Expand Down
5 changes: 3 additions & 2 deletions docs/rules/forbid-dom-props.md
Expand Up @@ -43,12 +43,13 @@ Examples of **correct** code for this rule:
### `forbid`

An array of strings, with the names of props that are forbidden. The default value of this option `[]`.
Each array element can either be a string with the property name or object specifying the property name and an optional
custom message:
Each array element can either be a string with the property name or object specifying the property name, an optional
custom message, and a DOM nodes disallowed list (e.g. `<div />`):

```js
{
"propName": "someProp",
"disallowedFor": ["DOMNode", "AnotherDOMNode"],
"message": "Avoid using someProp"
}
```
Expand Down
34 changes: 26 additions & 8 deletions lib/rules/forbid-dom-props.js
Expand Up @@ -18,6 +18,21 @@ const DEFAULTS = [];
// Rule Definition
// ------------------------------------------------------------------------------

/**
* @param {Map<string, object>} forbidMap // { disallowList: null | string[], message: null | string }
* @param {string} prop
* @param {string} tagName
* @returns {boolean}
*/
function isForbidden(forbidMap, prop, tagName) {
const options = forbidMap.get(prop);
return options && (
typeof tagName === 'undefined'
|| !options.disallowList
|| options.disallowList.indexOf(tagName) !== -1
);
}

const messages = {
propIsForbidden: 'Prop "{{prop}}" is forbidden on DOM Nodes',
};
Expand Down Expand Up @@ -47,6 +62,13 @@ module.exports = {
propName: {
type: 'string',
},
disallowedFor: {
type: 'array',
uniqueItems: true,
items: {
type: 'string',
},
},
message: {
type: 'string',
},
Expand All @@ -65,16 +87,12 @@ module.exports = {
const configuration = context.options[0] || {};
const forbid = new Map((configuration.forbid || DEFAULTS).map((value) => {
const propName = typeof value === 'string' ? value : value.propName;
const options = {
return [propName, {
disallowList: typeof value === 'string' ? null : (value.disallowedFor || null),
message: typeof value === 'string' ? null : value.message,
};
return [propName, options];
}];
}));

function isForbidden(prop) {
return forbid.has(prop);
}

return {
JSXAttribute(node) {
const tag = node.parent.name.name;
Expand All @@ -85,7 +103,7 @@ module.exports = {

const prop = node.name.name;

if (!isForbidden(prop)) {
if (!isForbidden(forbid, prop, tag)) {
return;
}

Expand Down
87 changes: 87 additions & 0 deletions tests/lib/rules/forbid-dom-props.js
Expand Up @@ -95,6 +95,23 @@ ruleTester.run('forbid-dom-props', rule, {
`,
options: [{ forbid: ['id'] }],
},
{
code: `
const First = (props) => (
<div otherProp="bar" />
);
`,
options: [
{
forbid: [
{
propName: 'otherProp',
disallowedFor: ['span'],
},
],
},
],
},
]),

invalid: parsers.all([
Expand Down Expand Up @@ -237,5 +254,75 @@ ruleTester.run('forbid-dom-props', rule, {
},
],
},
{
code: `
const First = (props) => (
<form accept='file'>
<input type="file" id="videoFile" accept="video/*" />
<input type="hidden" name="fullname" />
</form>
);
`,
options: [
{
forbid: [{
propName: 'accept',
disallowedFor: ['form'],
message: 'Avoid using the accept attribute on <form>',
}],
},
],
errors: [
{
message: 'Avoid using the accept attribute on <form>',
line: 3,
column: 17,
type: 'JSXAttribute',
},
],
},
{
code: `
const First = (props) => (
<div className="foo">
<input className="boo" />
<span className="foobar">Foobar</span>
<div otherProp="bar" />
</div>
);
`,
options: [
{
forbid: [
{
propName: 'className',
disallowedFor: ['div', 'span'],
message: 'Please use class instead of ClassName',
},
{ propName: 'otherProp', message: 'Avoid using otherProp' },
],
},
],
errors: [
{
message: 'Please use class instead of ClassName',
line: 3,
column: 16,
type: 'JSXAttribute',
},
{
message: 'Please use class instead of ClassName',
line: 5,
column: 19,
type: 'JSXAttribute',
},
{
message: 'Avoid using otherProp',
line: 6,
column: 18,
type: 'JSXAttribute',
},
],
},
]),
});