[Fix] no-unknown-property: properly recognize unknown HTML/DOM attributes

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.

Author: sjarva

CHANGELOG.md

@@ -9,6 +9,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 * [`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 recognize valid data- and aria- attributes ([#3377][] @sjarva)
+* [`no-unknown-property`]: properly recognize unknown HTML/DOM attributes ([#3377][] @sjarva)
 
 ### Changed
 * [Docs] [`jsx-sort-props`]: replace ref string with ref variable ([#3375][] @Luccasoli)

docs/rules/no-unknown-property.md

@@ -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:
@@ -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

lib/rules/no-unknown-property.js

@@ -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);
- },
- });
  },
  };
  },

tests/lib/rules/no-unknown-property.js

@@ -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>;',