New: html-self-closing rule (fixes #31)
#129
Conversation
|
Thank you for the review. I added more tests. Also, I found a problem about comments then I fixed it. |
| @@ -0,0 +1,67 @@ | |||
| # Enforce self-closing style (html-self-closing-style) | |||
There was a problem hiding this comment.
Maybe we can shorten the name to just html-self-closing?
lib/rules/html-self-closing-style.js
Outdated
|
|
||
| /** | ||
| * Kind strings. | ||
| * This strings wil be displayed in error messages. |
lib/rules/html-self-closing-style.js
Outdated
| // ------------------------------------------------------------------------------ | ||
|
|
||
| /** | ||
| * Kind strings. |
There was a problem hiding this comment.
Let's remove this comment, it sounds like Nice strings which is funny :D Kinds of strings would sound better, but it's redundant anyway.
lib/rules/html-self-closing-style.js
Outdated
| */ | ||
| function parseOptions (options) { | ||
| return { | ||
| [KIND.NORMAL]: (options && options.html && options.html.normal) || 'never', |
There was a problem hiding this comment.
Seems like we could use https://github.com/letsgetrandy/brototype 😂
There was a problem hiding this comment.
Or sindresorhus' dot-prop. However, I'm not sure if this is enough worth adding a new dependency. This may be not stylish, but I think enough readable.
lib/rules/html-self-closing-style.js
Outdated
| */ | ||
| function isEmpty (node, sourceCode) { | ||
| const start = node.startTag.range[1] | ||
| const end = (node.endTag != null) ? node.endTag.range[0] : node.range[1] |
There was a problem hiding this comment.
I love == null, this is equivalent to (x === null || typeof x === "undefined") and readable.
lib/rules/html-self-closing-style.js
Outdated
|
|
||
| utils.registerTemplateBodyVisitor(context, { | ||
| 'VElement' (node) { | ||
| const kind = getKind(node) |
There was a problem hiding this comment.
I'd keep kind simple as: normal, void, component and so on, and only reference the longer (more descriptive) name in message using e.g. getKindName(kind) {} function.
Btw. I think we should rename kind and call it for example htmlElementType or nodeType, what do you think? kind itself doesn't mean much, kind of what?
There was a problem hiding this comment.
I'd keep kind simple as: normal, void, component and so on, and only reference the longer (more descriptive) name in message using e.g. getKindName(kind) {} function.
Hmm, I'm not sure if it increases maintainability/readability.
Btw. I think we should rename kind and call it for example htmlElementType or nodeType, what do you think? kind itself doesn't mean much, kind of what?
It's nice. My English skill is poor.
| <math><mspace></mspace></math> | ||
| <math><mspace/></math> | ||
| </template>` | ||
| const anyWith = (opts) => Object.assign( |
| { | ||
| code: '<template><div/></template>', | ||
| output: '<template><div></div></template>', | ||
| errors: ['Disallow self-closing on HTML elements.'] |
There was a problem hiding this comment.
What do you think about adding element name in the error?
Disallow self-closing on HTML elements (div).
| </template>`, | ||
| options: [anyWith({ svg: 'always' })], | ||
| errors: [ | ||
| { message: 'Require self-closing on SVG elements.', line: 8 } |
There was a problem hiding this comment.
No. I guess you see output property which is the result of auto-fix.
| <x-test></x-test> | ||
| <x-test/> | ||
| <svg><path></path></svg> | ||
| <svg><path></path></svg> |
There was a problem hiding this comment.
Again line 8 and 9 are the same, is it intentional?
There was a problem hiding this comment.
Yes. It's the result of auto-fix.
|
Thank you for review. I updated this PR. |
mysticatea
changed the title
html-self-closing-style rule (fixes #31)html-self-closing rule (fixes #31)
Fixes #31.
This PR adds
html-self-closing-stylerule which replaceshtml-no-self-closingrule.This rule would help us to unify self-closing style.