Skip to content

Commit

Permalink
[Fix] no-unknown-property: properly recognise unknown HTML/DOM attr…
Browse files Browse the repository at this point in the history
…ibutes

Refactored going through different scenarios and error cases to be more clear.
Also add some new unit tests, and move one should-have-been invalid test case from valid to invalid.
  • Loading branch information
sjarva authored and ljharb committed Aug 31, 2022
1 parent cdb76f2 commit 1554a91
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 34 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
### Fixed
* [`jsx-key`]: avoid a crash with optional chaining ([#3371][] @ljharb)
* [`jsx-sort-props`]: avoid a crash with spread props ([#3376][] @ljharb)
* [`no-unknown-property`]: properly recognise unknown HTML/DOM attributes ([#3377][] @sjarva)

### Changed
* [Docs] [`jsx-sort-props`]: replace ref string with ref variable ([#3375][] @Luccasoli)
Expand Down
15 changes: 14 additions & 1 deletion docs/rules/no-unknown-property.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ Examples of **incorrect** code for this rule:
var React = require('react');

var Hello = <div class="hello">Hello World</div>;
var Alphabet = <div abc="something">Alphabet</div>;

// Invalid aria-* attribute
var IconButton = <div aria-foo="bar" />;
```

Examples of **correct** code for this rule:
Expand All @@ -23,14 +27,23 @@ Examples of **correct** code for this rule:
var React = require('react');

var Hello = <div className="hello">Hello World</div>;
var Button = <button disabled>Cannot click me</button>;
var Img = <img src={catImage} alt="A cat sleeping on a keyboard" />;

// aria-* attributes
var IconButton = <button aria-label="Close" onClick={this.close}>{closeIcon}</button>;


// data-* attributes
var Data = <div data-index={12}>Some data</div>;

// React components are ignored
var MyComponent = <App class="foo-bar"/>;
var AnotherComponent = <Foo.bar for="bar" />;

// Custom web components are ignored
var MyElem = <div class="foo" is="my-elem"></div>;
var AtomPanel = <atom-panel class="foo"></atom-panel>;

```

## Rule Options
Expand Down
73 changes: 48 additions & 25 deletions lib/rules/no-unknown-property.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,37 +366,60 @@ module.exports = {

const tagName = getTagName(node);

// 1. Some attributes are allowed on some tags only.
const allowedTags = has(ATTRIBUTE_TAGS_MAP, name) ? ATTRIBUTE_TAGS_MAP[name] : null;
if (tagName && allowedTags && /[^A-Z]/.test(tagName.charAt(0)) && allowedTags.indexOf(tagName) === -1) {
report(context, messages.invalidPropOnTag, 'invalidPropOnTag', {
// Let's dive deeper into tags that are HTML/DOM elements (`<button>`), and not React components (`<Button />`)
if (isValidHTMLTagInJSX(node)) {
// Some attributes are allowed on some tags only
const allowedTags = has(ATTRIBUTE_TAGS_MAP, name) ? ATTRIBUTE_TAGS_MAP[name] : null;
if (tagName && allowedTags) {
// Scenario 1A: Allowed attribute found where not supposed to, report it
if (allowedTags.indexOf(tagName) === -1) {
report(context, messages.invalidPropOnTag, 'invalidPropOnTag', {
node,
data: {
name,
tagName,
allowedTags: allowedTags.join(', '),
},
});
}
// Scenario 1B: There are allowed attributes on allowed tags, no need to report it
return;
}

// Let's see if the attribute is a close version to some standard property name
const standardName = getStandardName(name, context);

const hasStandardNameButIsNotUsed = standardName && standardName !== name;
const usesStandardName = standardName && standardName === name;

if (usesStandardName) {
// Scenario 2A: The attribute name is the standard name, no need to report it
return;
}

if (hasStandardNameButIsNotUsed) {
// Scenario 2B: The name of the attribute is close to a standard one, report it with the standard name
report(context, messages.unknownPropWithStandardName, 'unknownPropWithStandardName', {
node,
data: {
name,
standardName,
},
fix(fixer) {
return fixer.replaceText(node.name, standardName);
},
});
return;
}

// Scenario 3: We have an attribute that is unknown, report it
report(context, messages.unknownProp, 'unknownProp', {
node,
data: {
name,
tagName,
allowedTags: allowedTags.join(', '),
},
});
}

// 2. Otherwise, we'll try to find if the attribute is a close version
// of what we should normally have with React. If yes, we'll report an
// error. We don't want to report if the input attribute name is the
// standard name though!
const standardName = getStandardName(name, context);
if (!isValidHTMLTagInJSX(node) || !standardName || standardName === name) {
return;
}
report(context, messages.unknownPropWithStandardName, 'unknownPropWithStandardName', {
node,
data: {
name,
standardName,
},
fix(fixer) {
return fixer.replaceText(node.name, standardName);
},
});
},
};
},
Expand Down
77 changes: 69 additions & 8 deletions tests/lib/rules/no-unknown-property.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,40 +29,101 @@ const parserOptions = {
const ruleTester = new RuleTester({ parserOptions });
ruleTester.run('no-unknown-property', rule, {
valid: parsers.all([
// React components and their props/attributes should be fine
{ code: '<App class="bar" />;' },
{ code: '<App for="bar" />;' },
{ code: '<App someProp="bar" />;' },
{ code: '<Foo.bar for="bar" />;' },
{ code: '<App accept-charset="bar" />;' },
{ code: '<meta charset="utf-8" />;' },
{ code: '<meta charSet="utf-8" />;' },
{ code: '<App http-equiv="bar" />;' },
{
code: '<App xlink:href="bar" />;',
features: ['jsx namespace'],
},
{ code: '<App clip-path="bar" />;' },
// Some HTML/DOM elements with common attributes should work
{ code: '<div className="bar"></div>;' },
{ code: '<div onMouseDown={this._onMouseDown}></div>;' },
// data attributes should work
{ code: '<a href="someLink">Read more</a>' },
{ code: '<img src="cat_keyboard.jpeg" alt="A cat sleeping on a keyboard" />' },
{ code: '<input type="password" required />' },
{ code: '<input ref={this.input} type="radio" />' },
{ code: '<button disabled>You cannot click me</button>;' },
// Case ignored attributes, for `charset` discussion see https://github.com/jsx-eslint/eslint-plugin-react/pull/1863
{ code: '<meta charset="utf-8" />;' },
{ code: '<meta charSet="utf-8" />;' },
// Some custom web components that are allowed to use `class` instead of `className`
{ code: '<div class="foo" is="my-elem"></div>;' },
{ code: '<div {...this.props} class="foo" is="my-elem"></div>;' },
{ code: '<atom-panel class="foo"></atom-panel>;' },
// data-* attributes should work
{ code: '<div data-foo="bar"></div>;' },
{ code: '<div data-foo-bar="baz"></div>;' },
{ code: '<div data-parent="parent"></div>;' },
{ code: '<div data-index-number="1234"></div>;' },
{ code: '<div class="foo" is="my-elem"></div>;' },
{ code: '<div {...this.props} class="foo" is="my-elem"></div>;' },
{ code: '<atom-panel class="foo"></atom-panel>;' }, {
// Ignoring should work
{
code: '<div class="bar"></div>;',
options: [{ ignore: ['class'] }],
},
// aria attributes should work
{
code: '<div someProp="bar"></div>;',
options: [{ ignore: ['someProp'] }],
},

// aria-* attributes should work
{ code: '<button aria-haspopup="true">Click me to open pop up</button>;' },
{ code: '<button aria-label="Close" onClick={someThing.close} />;' },
// Attributes on allowed elements should work
{ code: '<script crossOrigin />' },
{ code: '<audio crossOrigin />' },
{ code: '<div hasOwnProperty="should not be allowed tag" />' },
{ code: '<svg><image crossOrigin /></svg>' },
]),
invalid: parsers.all([
{
code: '<div hasOwnProperty="should not be allowed property"></div>;',
errors: [
{
messageId: 'unknownProp',
data: {
name: 'hasOwnProperty',
},
},
],
},
{
code: '<div abc="should not be allowed property"></div>;',
errors: [
{
messageId: 'unknownProp',
data: {
name: 'abc',
},
},
],
},
{
code: '<div aria-fake="should not be allowed property"></div>;',
errors: [
{
messageId: 'unknownProp',
data: {
name: 'aria-fake',
},
},
],
},
{
code: '<div someProp="bar"></div>;',
errors: [
{
messageId: 'unknownProp',
data: {
name: 'someProp',
},
},
],
},
{
code: '<div class="bar"></div>;',
output: '<div className="bar"></div>;',
Expand Down

0 comments on commit 1554a91

Please sign in to comment.