Skip to content

Commit d6d84cc

Browse files
sjarvaljharb
sjarva
authored and
ljharb
committed
[Refactor] no-unknown-property: improve jsdoc; extract logic to separate functions
 - checking case ignored attributes to a function of its own Also move variable inside function definition to make JSDoc comment match correct block Also update the link to Web components spec
1 parent d3aa050 commit d6d84cc

File tree

3 files changed

+57
-24
lines changed

3 files changed

+57
-24
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
1010
* [`jsx-sort-props`]: avoid a crash with spread props ([#3376][] @ljharb)
1111

1212
### Changed
13-
* [Docs] [`jsx-sort-propts`]: replace ref string with ref variable ([#3375][] @Luccasoli)
13+
* [Docs] [`jsx-sort-props`]: replace ref string with ref variable ([#3375][] @Luccasoli)
14+
* [Refactor] [`no-unknown-property`]: improve jsdoc; extract logic to separate functions ([#3377][] @sjarva)
1415

16+
[#3377]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3377
1517
[#3376]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3376
1618
[#3375]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3375
1719
[#3371]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3371

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

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,9 @@ const DOM_PROPERTY_NAMES = [
134134
'autoSave',
135135
'itemProp', 'itemScope', 'itemType', 'itemRef', 'itemID',
136136
];
137+
138+
const DOM_PROPERTIES_IGNORE_CASE = ['charset'];
139+
137140
function getDOMPropertyNames(context) {
138141
// this was removed in React v16.1+, see https://github.com/facebook/react/pull/10823
139142
if (!testReactVersion(context, '>= 16.1.0')) {
@@ -147,24 +150,49 @@ function getDOMPropertyNames(context) {
147150
// ------------------------------------------------------------------------------
148151

149152
/**
150-
* Checks if a node matches the JSX tag convention. This also checks if a node
151-
* is extended as a webcomponent using the attribute "is".
152-
* @param {Object} node - JSX element being tested.
153+
* Checks if a node's parent is a JSX tag that is written with lowercase letters,
154+
* and is not a custom web component. Custom web components have a hyphen in tag name,
155+
* or have an `is="some-elem"` attribute.
156+
*
157+
* Note: does not check if a tag's parent against a list of standard HTML/DOM tags. For example,
158+
* a `<fake>`'s child would return `true` because "fake" is written only with lowercase letters
159+
* without a hyphen and does not have a `is="some-elem"` attribute.
160+
*
161+
* @param {Object} childNode - JSX element being tested.
153162
* @returns {boolean} Whether or not the node name match the JSX tag convention.
154163
*/
155-
const tagConvention = /^[a-z][^-]*$/;
156-
function isTagName(node) {
157-
if (tagConvention.test(node.parent.name.name)) {
158-
// https://www.w3.org/TR/custom-elements/#type-extension-semantics
159-
return !node.parent.attributes.some((attrNode) => (
164+
function isValidHTMLTagInJSX(childNode) {
165+
const tagConvention = /^[a-z][^-]*$/;
166+
if (tagConvention.test(childNode.parent.name.name)) {
167+
return !childNode.parent.attributes.some((attrNode) => (
160168
attrNode.type === 'JSXAttribute'
161169
&& attrNode.name.type === 'JSXIdentifier'
162170
&& attrNode.name.name === 'is'
171+
// To learn more about custom web components and `is` attribute,
172+
// see https://html.spec.whatwg.org/multipage/custom-elements.html#custom-elements-customized-builtin-example
173+
163174
));
164175
}
165176
return false;
166177
}
167178

179+
/**
180+
* Checks if the attribute name is included in the attributes that are excluded
181+
* from the camel casing.
182+
*
183+
* // returns true
184+
* @example isCaseIgnoredAttribute('charSet')
185+
*
186+
* Note - these exclusions are not made by React core team, but `eslint-plugin-react` community.
187+
*
188+
* @param {String} name - Attribute name to be tested
189+
* @returns {Boolean} Result
190+
*/
191+
192+
function isCaseIgnoredAttribute(name) {
193+
return DOM_PROPERTIES_IGNORE_CASE.some((element) => element === name.toLowerCase());
194+
}
195+
168196
/**
169197
* Extracts the tag name for the JSXAttribute
170198
* @param {JSXAttribute} node - JSXAttribute being tested.
@@ -215,7 +243,8 @@ function getStandardName(name, context) {
215243

216244
const messages = {
217245
invalidPropOnTag: 'Invalid property \'{{name}}\' found on tag \'{{tagName}}\', but it is only allowed on: {{allowedTags}}',
218-
unknownProp: 'Unknown property \'{{name}}\' found, use \'{{standardName}}\' instead',
246+
unknownPropWithStandardName: 'Unknown property \'{{name}}\' found, use \'{{standardName}}\' instead',
247+
unknownProp: 'Unknown property \'{{name}}\' found',
219248
};
220249

221250
module.exports = {
@@ -262,6 +291,8 @@ module.exports = {
262291
return;
263292
}
264293

294+
if (isCaseIgnoredAttribute(name)) { return; }
295+
265296
const tagName = getTagName(node);
266297

267298
// 1. Some attributes are allowed on some tags only.
@@ -282,10 +313,10 @@ module.exports = {
282313
// error. We don't want to report if the input attribute name is the
283314
// standard name though!
284315
const standardName = getStandardName(name, context);
285-
if (!isTagName(node) || !standardName || standardName === name) {
316+
if (!isValidHTMLTagInJSX(node) || !standardName || standardName === name) {
286317
return;
287318
}
288-
report(context, messages.unknownProp, 'unknownProp', {
319+
report(context, messages.unknownPropWithStandardName, 'unknownPropWithStandardName', {
289320
node,
290321
data: {
291322
name,

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ ruleTester.run('no-unknown-property', rule, {
6161
output: '<div className="bar"></div>;',
6262
errors: [
6363
{
64-
messageId: 'unknownProp',
64+
messageId: 'unknownPropWithStandardName',
6565
data: {
6666
name: 'class',
6767
standardName: 'className',
@@ -74,7 +74,7 @@ ruleTester.run('no-unknown-property', rule, {
7474
output: '<div htmlFor="bar"></div>;',
7575
errors: [
7676
{
77-
messageId: 'unknownProp',
77+
messageId: 'unknownPropWithStandardName',
7878
data: {
7979
name: 'for',
8080
standardName: 'htmlFor',
@@ -87,7 +87,7 @@ ruleTester.run('no-unknown-property', rule, {
8787
output: '<div acceptCharset="bar"></div>;',
8888
errors: [
8989
{
90-
messageId: 'unknownProp',
90+
messageId: 'unknownPropWithStandardName',
9191
data: {
9292
name: 'accept-charset',
9393
standardName: 'acceptCharset',
@@ -100,7 +100,7 @@ ruleTester.run('no-unknown-property', rule, {
100100
output: '<div httpEquiv="bar"></div>;',
101101
errors: [
102102
{
103-
messageId: 'unknownProp',
103+
messageId: 'unknownPropWithStandardName',
104104
data: {
105105
name: 'http-equiv',
106106
standardName: 'httpEquiv',
@@ -113,7 +113,7 @@ ruleTester.run('no-unknown-property', rule, {
113113
output: '<div accessKey="bar"></div>;',
114114
errors: [
115115
{
116-
messageId: 'unknownProp',
116+
messageId: 'unknownPropWithStandardName',
117117
data: {
118118
name: 'accesskey',
119119
standardName: 'accessKey',
@@ -126,7 +126,7 @@ ruleTester.run('no-unknown-property', rule, {
126126
output: '<div onClick="bar"></div>;',
127127
errors: [
128128
{
129-
messageId: 'unknownProp',
129+
messageId: 'unknownPropWithStandardName',
130130
data: {
131131
name: 'onclick',
132132
standardName: 'onClick',
@@ -139,7 +139,7 @@ ruleTester.run('no-unknown-property', rule, {
139139
output: '<div onMouseDown="bar"></div>;',
140140
errors: [
141141
{
142-
messageId: 'unknownProp',
142+
messageId: 'unknownPropWithStandardName',
143143
data: {
144144
name: 'onmousedown',
145145
standardName: 'onMouseDown',
@@ -152,7 +152,7 @@ ruleTester.run('no-unknown-property', rule, {
152152
output: '<div onMouseDown="bar"></div>;',
153153
errors: [
154154
{
155-
messageId: 'unknownProp',
155+
messageId: 'unknownPropWithStandardName',
156156
data: {
157157
name: 'onMousedown',
158158
standardName: 'onMouseDown',
@@ -166,7 +166,7 @@ ruleTester.run('no-unknown-property', rule, {
166166
features: ['jsx namespace'],
167167
errors: [
168168
{
169-
messageId: 'unknownProp',
169+
messageId: 'unknownPropWithStandardName',
170170
data: {
171171
name: 'xlink:href',
172172
standardName: 'xlinkHref',
@@ -179,7 +179,7 @@ ruleTester.run('no-unknown-property', rule, {
179179
output: '<rect clipPath="bar" />;',
180180
errors: [
181181
{
182-
messageId: 'unknownProp',
182+
messageId: 'unknownPropWithStandardName',
183183
data: {
184184
name: 'clip-path',
185185
standardName: 'clipPath',
@@ -191,7 +191,7 @@ ruleTester.run('no-unknown-property', rule, {
191191
code: '<script crossorigin />',
192192
errors: [
193193
{
194-
messageId: 'unknownProp',
194+
messageId: 'unknownPropWithStandardName',
195195
data: {
196196
name: 'crossorigin',
197197
standardName: 'crossOrigin',
@@ -204,7 +204,7 @@ ruleTester.run('no-unknown-property', rule, {
204204
code: '<div crossorigin />',
205205
errors: [
206206
{
207-
messageId: 'unknownProp',
207+
messageId: 'unknownPropWithStandardName',
208208
data: {
209209
name: 'crossorigin',
210210
standardName: 'crossOrigin',

0 commit comments

Comments
 (0)