Eslint-plugin-react: New Rule Proposal: `no-unnecessary-html-attributes`

Created on 3 Dec 2020  路  15Comments  路  Source: yannickcr/eslint-plugin-react

Hi there 馃憢

Thanks for the continued work on this great plugin!

I had an idea for a rule today, and wanted to test the waters here on a no-unnecessary-html-attributes rule. It originally was inspired due to the default value type="text" on input elements, but due to maintainer feedback, it was modified to include default attributes on more elements.

Example of incorrect code:

<input type="text" />

<button type="submit">submit</button>

<ol type="1">
  <li type="1">first</li>
</ol>

<style type="text/css">{`a { color: red; }`}</style>

<link type="text/css" rel="stylesheet" href="https://example.com/main.css" />

<form method="GET"></form>
<form enctype="application/x-www-form-urlencoded"></form>
<form target="_self"></form>

<img loading="eager" src="https://example.com/dog.jpg" alt="Doge" />

<iframe loading="eager" src="https://example.com"></iframe>

<textarea wrap="soft"></textarea>

Examples of correct code:

<input />
<input type="number" />

<button>submit</button>
<button type="button">submit</button>

<ol>
  <li>first</li>
</ol>

<style>{`a { color: red; }`}</style>

<link rel="stylesheet" href="https://example.com/main.css" />

<form></form>

<img src="https://example.com/dog.jpg" alt="Doge" />

<iframe src="https://example.com"></iframe>

<textarea></textarea>
new rule question

All 15 comments

It's far, far better to always require explicitness, even when things have defaults. I'd rather see a rule that forbids <input /> than one that requires people to keep an extra thing in their heads (that the default type is "text").

It's far, far better to always require explicitness, even when things have defaults

I understand the mindset, and in some cases, I agree.

But under some circumstances, it's nice to not have to write the verbose thing. And if you have a lot of code with inputs, this will reduce the amount of code that needs to be written / reviewed.

Also, and maybe more importantly, this is an opinion - other individuals and teams will have different ones :)

So I guess my question would be: why not both? (one option value for what I'm proposing eg: never, one for your proposal, eg. always) There's a lot of prior art of ESLint rules that offer options for both opinions.

I don't think that just "input" is sufficient to warrant an entire rule. There's also already a rule, react/button-has-type, for button elements, but that's because the default is unintuitive.

Are there other HTML elements that this would apply to, where the default can be omitted and also is unsurprising? A single rule that encompasses a bunch of HTML elements in this way, autofixable, with both options, seems like it might be worth the complexity.

Are there other HTML elements that this would apply to, where the default can be omitted and also is unsurprising?

Looks like <ol> and the <li> elements contained within have type attributes:

  • a for lowercase letters
  • A for uppercase letters
  • i for lowercase Roman numerals
  • I for uppercase Roman numerals
  • 1 for numbers (default)

<ol> on MDN

The <ul> element also has a type attribute, but this attribute is deprecated.

Also (but maybe less interestingly):

  • <style> - default: type="text/css"
  • <script> - default: type="text/javascript"

huh, i've never heard of those being used - only the CSS.

In React, <script> doesn't really make sense to address. So that would basically leave us with button, input, and style - not all that many :-/

Hmm... and what's your opinion on other default props / HTML attributes? Things that can be omitted and the functionality is the same?

Eg:

  • <form method="GET">
  • <form enctype="application/x-www-form-urlencoded">

Can also search for more if this would be a way forward.

I think the longer a list we build up, the more convincing a rule for it becomes.

Ok, let me do a bit of research, and I will come back with a longer list. I'll change the issue title and description in the meantime.

Ok, so I've updated the list above with more examples.

Talking with @wooorm, it seems like there are some interesting resources in the rehype-minify-enumerated-attribute package which could help with finding the attributes with default values.

I've asked if publishing a separate package with this data would be possible at https://github.com/rehypejs/rehype-minify/issues/37

I have https://npmjs.com/html-element-map, which won鈥檛 help here but is a similar concept re extracted data as a package, so that would likely work well.

Yeah I鈥檓 fine publishing it separately! The Q then is: what format do you need? Does the current schema make sense?

Another thing is that the schema currently uses hast casing, which is in rare cases different from react casing (although the translation is relatively simple), and both are different from HTML.

Note that publishing it separately is most compelling with other projects can start using it - that way it ensures it will stay correct :-)

I have no idea what "hast" is, but it seems reasonable for the package to provide html casing, and for consumers to adapt it as needed (or, for another package to wrap it and provide the react version, eg)

I have no idea what "hast" is, but it seems reasonable for the package to provide html casing, and for consumers to adapt it as needed (or, for another package to wrap it and provide the react version, eg)

hast is an html ast. Like estree but for HTML. Sometimes but not always used with mdast (markdown).
It鈥檚 relevant here because the HTML minifier in question uses hast.

Transformation between react / hast / html / dom is all not very complex, but does need a bit of a look up table. As we鈥檙e currently interested in react / hast (almost identical, close to DOM props), using that sounds like the simplest path forward to me?

Anyway, I鈥檓 not blocking this, just preferences

Ideally I'd like this plugin to use something that's very simple and barebones - just DOM to react jsx - that shares a common core with what the minifier uses, rather than using something multi-purpose that will have more version churn. Is that a possibility?

Some more discussion about publishing the new package over here: https://github.com/rehypejs/rehype-minify/issues/37

Was this page helpful?
0 / 5 - 0 ratings