Activation of template related ESLint rules

[philippwaller]

This PR activates the following ESLint rules and provides the necessary code changes:

• vue/attributes-order
• vue/html-closing-bracket-newline
• vue/html-closing-bracket-spacing
• vue/html-self-closing
• vue/max-attributes-per-line
• vue/multiline-html-element-content-newline
• vue/mustache-interpolation-spacing
• vue/singleline-html-element-content-newline
• vue/v-on-style
• vue/v-slot-style

[ghys]

For the record:
Regarding vue/max-attributes-per-line & vue/attributes-order, these are very opinionated and you know my position, I'm more on the "apply responsibly when it makes sense" side than the "enforce it" side.
I understand the benefits and appeal to some. I also recognize they're "strongly recommended" by the Vue style guide.
On the other hand, I don't think it always improves readability, and even sometimes hampers it, I believe it's more difficult to see the structure when all attributes are "exploded" vs. a more "compact" one.
(+14,503 −3,456 changed lines with this PR, a 10k increase!)
For example, between:

<template>
  <f7-page @page:afterin="onPageAfterIn" name="channel-add">
    <f7-navbar title="Add Channel" back-link="Cancel">
      <f7-nav-right class="if-not-aurora">
        <f7-link @click="save()" v-if="$theme.md" icon-md="material:save" icon-only></f7-link>
        <f7-link @click="save()" v-if="!$theme.md">Done</f7-link>
      </f7-nav-right>
    </f7-navbar>
    <f7-block class="block-narrow">
      <f7-col>
        <f7-list inline-labels no-hairlines-md>
...

and:

<template>
  <f7-page
    name="channel-add"
    @page:afterin="onPageAfterIn"
  >
    <f7-navbar
      title="Add Channel"
      back-link="Cancel"
    >
      <f7-nav-right class="if-not-aurora">
        <f7-link
          v-if="$theme.md"
          icon-md="material:save"
          icon-only
          @click="save()"
        />
        <f7-link
          v-if="!$theme.md"
          @click="save()"
        >
          Done
        </f7-link>
      </f7-nav-right>
    </f7-navbar>
    <f7-block class="block-narrow">
      <f7-col>
        <f7-list
          inline-labels
          no-hairlines-md
        >
...

I must admit I prefer the former. To each their own I guess.

Also, I fear it will reduce the barrier to entry for new contributors as it would basically require the proper tooling to autofix problems constantly during development or fight ESLint constantly (this is of course also true for other rules but most others can be assimilated and just respected during typing, I'm not so true for these ones.)

I don't want to spark a big debate, but here's some (heated) ones + additional info:
There is Prettier's controversial and long-standing stubbornness NOT to add this even as an option, which would conflict with this rule being enforced in ESLint:
prettier/prettier#5501
prettier/eslint-plugin-prettier#94
(participants in these debates are mostly on the "pro" side).
On the other side, there are also calls to remove from the Vue style guide.
On the other side mitigations for Prettier exist:
https://github.com/meteorlxy/eslint-plugin-prettier-vue
This could also be considered: prettier/eslint-plugin-prettier#94 (comment)

That being said:
I'm willing to let myself be convinced if you add at least this in a new bundles/org.openhab.ui/web/.vscode/settings.json:

{
  "vetur.format.defaultFormatter.js": "vscode-typescript",
  "vetur.format.defaultFormatter.html": "js-beautify-html",
  "javascript.format.insertSpaceBeforeFunctionParenthesis": true,
  "editor.codeActionsOnSave": {
    "source.fixAll.eslint": true
  },
  "eslint.validate": ["vue", "html", "javascript"]
}

Then we can at least give instructions to install the proper VS Code extensions (Vetur, ESLint) and get autofix on save without hassle.

Also interested in the opinion of past, current and future contributors who see this!

[philippwaller]

Also interested in the opinion of past, current and future contributors who see this!

Since we have not received any further feedback I have adopted your suggestion and disabled the rules vue/max-attributes-per-line & vue/attributes-order.

[ghys]

Thanks @philippwaller - just know I do appreciate your initiative to have a predictable/enforceable template style, and I'm convinced some sort of it should be made in the future, but for now I would like to encourage contributors, and these 2 rules might be a bit too far in my opinion as I think it would be cumbersome to those who don't have an IDE that can format/lint on save.

Also perhaps I've seen too much XML in my life to grow accustomed of this one-attribute-per-line formatting and not find it odd in places, but I'm ready to take one for the team if the consensus points that way! :)

The rest of the rules are perfectly fine and I'm happy to accept them.

Allow me a few days to do a thorough check before merging as sometimes whitespace matters, so I want to make sure there aren't obvious regressions, but I don't think there will be (the code is minified anyways).

[philippwaller]

@ghys no worries, as we discussed right at the beginning, there is no right or wrong. Take you time and let me know if you find any corner cases we need to work on. I am happy to help :).

[ghys]

Also there's something somewhere causing the gray bar behind the menu buttons to be misaligned:

[hubsif]

Also interested in the opinion of past, current and future contributors who see this!

My two cents:
Linting - besides checking for syntactic errors - is I think to make code more structured and readable.
And I'm fully with @ghys that code "exploded" by vue/max-attributes-per-line (and set to 1) is much worse readable, so I'd also skip that.

As for vue/attributes-order:
Generally a good idea, though I don't like its ordering. I started getting used to putting `v-if´ first, so you can see quickly when an element gets rendered and when it doesn't. And in some places I've put the label/text first, so you can easily identify e.g. menu list items.

And I don't like vue/singleline-html-element-content-newline too much. I can't see why a single line element should not be coded in a single line (like its sample for bad: <div attr>content</div>) and why the presence of a (short) attribute should make a difference (option ignoreWhenNoAttributes with default true, which makes <div>content</div> allowed).

All other rules are just fine I think.

P.S.: @ghys are you sure your last comment went into the correct issue?

[ghys]

P.S.: @ghys are you sure your last comment went into the correct issue?

No ;)

Also FYI I have set the ESLint webpack plugin to run only in production mode, because the compilation time would increase to maybe 30 seconds on every hot reload and that became very irritating.
So you'll have to rely on your IDE for showing you style errors when you develop; when you submit the PR though, if you still have linting errors:

• the build will fail
• (new) you'll now get annotations on GitHub, like these: https://github.com/openhab/openhab-webui/pull/938/files

[ghys]

@philippwaller I included the 2 commits of this PR in #942 so that I could merge everything in one go and fix the conflicts in the same process; therefore I will close this one. Thanks!

I could also do a quick check for obvious regressions and it looks fine.

FYI: I also changed the defaults of the vue/html-closing-bracket-newline, setting multiline to never as I don't think isolated closing brackets look too good without vue/max-attributes-per-line set to 1.
See 482e990

Also although I tend to agree with @hubsif on vue/singleline-html-element-content-newline, I don't feel too strongly about it, so I kept it activated.