Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule Proposal: no-invalid-attribute-name #1373

Closed
frederikadamse22 opened this issue Dec 10, 2020 · 10 comments · Fixed by #1851
Closed

Rule Proposal: no-invalid-attribute-name #1373

frederikadamse22 opened this issue Dec 10, 2020 · 10 comments · Fixed by #1851

Comments

@frederikadamse22
Copy link

Please describe what the rule should do:
no-invalid-attribute-name should disallow invalid attribute names for elements in the <template> tag.

What category should the rule belong to?

[ ] Enforces code style (layout)
[X] Warns about a potential error (problem)
[ ] Suggests an alternate way of doing something (suggestion)
[ ] Other (please specify:)

Provide 2-3 code examples that this rule should warn about:

<template>
  <div>
    <p v-if="condition" 0abc>
      {{ content }}
    </p>
  </div>
</template>

Here the linter should warn about the attribute 0abc, which is invalid and will cause a runtime error when vue attempts to insert it onto the dom element

<template>
  <div>
    <p v-if="condition" -->
      {{ content }}
    </p>
  </div>
</template>

Here the linter should warn about the attribute --, which is invalid and will cause a runtime error when vue attempts to insert it onto the dom element

Additional context
My team discovered this issue when a rogue 0 snuck its way through PR-review and onto an element behind a somewhat rare v-if condition. A linting rule like this would have immediately caught the problem.

@ota-meshi
Copy link
Member

Thank you for proposing this rule.

Do you know the specification of valid attribute names? If you know, please let me know. As far as I can see, 0abc also looks like a valid attribute name. (However, it does not work in Chrome.)
https://html.spec.whatwg.org/multipage/syntax.html#attributes-2

You can also report any attribute.
https://eslint.vuejs.org/rules/no-restricted-static-attribute.html

@frederikadamse22
Copy link
Author

According to the specification, the HTML name may be written using anything from in the ascii range a-z, and A-Z, but nothing is more specific. 0abc throws an error when used in setAttribute but ab0c does not.

@ota-meshi
Copy link
Member

Could you provide a link to the specs?

@frederikadamse22
Copy link
Author

It's right there in the specs you linked, https://html.spec.whatwg.org/multipage/syntax.html#attributes-2

attributeSpec

@ota-meshi
Copy link
Member

Oops. Thank you very much. I didn't understand the sentence correctly.
But it's an ambiguous sentence. It would be helpful if there were clear specifications...

@frederikadamse22
Copy link
Author

Agreed, there doesn't seem to be a clear cut definition here, other than large and small Ascii letters is ok.

doug-wade added a commit to doug-wade/eslint-plugin-vue that referenced this issue Apr 16, 2022
@doug-wade
Copy link
Contributor

doug-wade commented Apr 16, 2022

I looked into this one a little bit, and the spec says:

Any namespace-less attribute that is relevant to the element's functioning, as determined by the element's author, may be specified on an autonomous custom element, so long as the attribute name is XML-compatible and contains no ASCII upper alphas. The exception is the is attribute, which must not be specified on an autonomous custom element (and which will have no effect if it is).

The XML spec it is referring to (referring to what html calls attributes by their XML term "name") gives the following rules for names'construction:

Names and Tokens
[4] NameStartChar ::= ":" | [A-Z] | "_" | [a-z] | [#xC0-#xD6] | [#xD8-#xF6] | [#xF8-#x2FF] | [#x370-#x37D] | [#x37F-#x1FFF] | [#x200C-#x200D] | [#x2070-#x218F] | [#x2C00-#x2FEF] | [#x3001-#xD7FF] | [#xF900-#xFDCF] | [#xFDF0-#xFFFD] | [#x10000-#xEFFFF]
[4a] NameChar ::= NameStartChar | "-" | "." | [0-9] | #xB7 | [#x0300-#x036F] | [#x203F-#x2040]
[5] Name ::= NameStartChar (NameChar)*
[6] Names ::= Name (#x20 Name)*
[7] Nmtoken ::= (NameChar)+
[8] Nmtokens ::= Nmtoken (#x20 Nmtoken)*

I interpret that to the regex /^[_:a-z][_:.\-a-z0-9]+/, which I will admit is somewhat more restrictive than the XML rules, but can be expanded to include the other character ranges, should we decide to add this rule.

@ota-meshi
Copy link
Member

Thank you for checking the specifications.

Based on the information you gave me, I searched for a package to check it.
And I found a package that checks the names used by jsdom.
https://github.com/jsdom/xml-name-validator

I think we can use this, what do you think?

@doug-wade
Copy link
Contributor

I think we can. Should I change my implementation to use that and reopen the pr?

@ota-meshi
Copy link
Member

Yes thank you!

doug-wade added a commit to doug-wade/eslint-plugin-vue that referenced this issue Apr 21, 2022
@FloEdelmann FloEdelmann linked a pull request Apr 21, 2022 that will close this issue
ota-meshi added a commit that referenced this issue May 17, 2022
* Fix #1373: Add rule no-invalid-attribute-name

* Remove stray newline

* Apply suggestions from code review

Co-authored-by: Flo Edelmann <florian-edelmann@online.de>

* #1373 Use xml-name-validator

* Fix linting error

* remove stray newline

* refactor test code

* Update lib/rules/no-invalid-attribute-name.js

Co-authored-by: Flo Edelmann <florian-edelmann@online.de>

* fix bad commit from github ui

* fix typechecking error

* Respond to PR feedback

* Include the added types in package.json

* check v-bind directives

* Update tests/lib/rules/no-invalid-attribute-name.js

Co-authored-by: Flo Edelmann <florian-edelmann@online.de>

* Fix failing unit test

* Update lib/rules/no-invalid-attribute-name.js

* Update lib/rules/no-invalid-attribute-name.js

* Update tests/lib/rules/no-invalid-attribute-name.js

* Update tests/lib/rules/no-invalid-attribute-name.js

Co-authored-by: Flo Edelmann <florian-edelmann@online.de>
Co-authored-by: Yosuke Ota <otameshiyo23@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants