[Fix] no-unknown-property: properly recognise unknown HTML/DOM attributes
#3377
Conversation
…arate 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
…so one word properties Also regroup the properties to global, element specific and React or Safari/Apple specific
…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.
Codecov Report
@@ Coverage Diff @@
## master #3377 +/- ##
=======================================
Coverage 97.57% 97.57%
=======================================
Files 123 123
Lines 8932 8954 +22
Branches 3260 3268 +8
=======================================
+ Hits 8715 8737 +22
Misses 217 217
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
ljharb
changed the title
no-unknown-property not recognising unknown propertiesno-unknown-property: properly recognise unknown HTML/DOM attributes
ljharb
force-pushed
the
no-unknown-property-bug
branch
2 times, most recently
from
1554a91
to
f54b391
Compare
|
Thanks! |
|
I'm commenting here, because I think this is the best place to do it. I'm aware of the issues and errors that these changes caused. When making this PR, I was sure that I have missed some common scenarios that the community would find, and I was wondering should I ask or comment that could you @ljharb as a maintainer run some additional checks (like the smoke test) before releasing this. Being still a new contributor, I was shy and didn't dare to ask. Looking back now, I should have asked, and I will so better in the future. Also, I am on another timezone than you are, so I wasn't awake when you released this and tagged me to the first issue. I'm happy that someone else from the community provided a quick fix 🛠️ 🤗 |
~(PR jsx-eslint/eslint-plugin-react#3377) introduced a change in `eslint-plugin-react@7.31.2` that will now show an error when unknown properties appear on elements. We can opt out of this by overriding the default.~ As discussed internally, we are turning `react/no-unknown-property` off, as it might be confusing even if different props are being used, (eg.: `css` for `emotion`). It's easy to fix https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/no-unknown-property.md#rule-options, but it might not be clear at first glance that Next.js is using `eslint-plugin-react` internally. If the user wants to enforce this rule, they can still add it to their own `rules` config. Fixes #40321, ref: #40269, #38333
|
Hi! I want to say huge thanks for writing this change — it spotted some cases in our codebase that we would've never gotten to find without the help of this linter! I figured it's nice to also hear some encouraging words and not merely bug reports 😄 I for one am thankful for the work you've done here 💞 |
Thank you so much @valscion! Yes, it is nice to hear that it helped find bugs 🐛 💕 And I really appreciate you taking the time to write this comment, it really made my day 🤗 |
Closes #757 by...
unknownProptounknownPropWithStandardName), since it reports an unknown property that is close (one lower/higher case letter) to an allowed (standard) nameunknownProp) that is used when an unknown property (without aaria-*attributesdata-*attributeWhat was not implemented in this PR:
ATTRIBUTE_TAGS_MAP(e.g. attributesoptimum,highandlowshould be used only on<meter>element)While implementing these changes, I felt like I cannot be sure that this PR catches all valid HTML/DOM attributes. There are some listing on MDN and on React documentation, but I could still find attributes that weren't on these listing that I felt would be beneficial to be listed in this rule (for example, the Safari/Apple related
autoCorrectandautoSave, and React relatedrefanddefaultValue). So, I'm sure there will be cases that this PR has missed, and the community will find them if these changes are merged and published.Feel free to tag some who knows HTML/DOM attributes well and can help with reviewing and possibly suggesting more new unit test cases.