Add option to ignore comments in vue/multiline-html-element-content-newline

[dsl101]

Fixes #2179

I finally got around to looking at a PR. It seems deceptively simple, so not sure if it broke something elsewhere, but no other tests seemed affected.

[dsl101]

If the approach is OK, I'll add to the docs

[FloEdelmann]

Looks good to me 🙂
Please go ahead and adjust the docs now 😉

[FloEdelmann]

And could you also add the same option to vue/singleline-html-element-content-newline as well please?

[dsl101]

And could you also add the same option to vue/singleline-html-element-content-newline as well please?

I'm unsure how to do this, as a straight copy / paste of the changes into that rule doesn't quite work the same. For example, with ignoreComments: true this test failes:

  valid: [{
    code: `
      <template>
        <div attr> <!-- comment --> </div>
      </template>`,
    options: [
      {
        ignoreComments: true
      }
    ]
  }]

with these errors:

  1) singleLineRule
       valid

      <template>
        <div attr> <!-- comment --> </div>
      </template>:

      AssertionError [ERR_ASSERTION]: Should have no errors but had 2: [
  {
    ruleId: 'singleLineRule',
    severity: 1,
    message: 'Expected 1 line break after opening tag (`<div>`), but no line breaks found.',
    line: 3,
    column: 19,
    nodeType: 'HTMLTagClose',
    messageId: 'unexpectedAfterClosingBracket',
    endLine: 3,
    endColumn: 37,
    fix: { range: [Array], text: '\n' }
  },
  {
    ruleId: 'singleLineRule',
    severity: 1,
    message: 'Expected 1 line break before closing tag (`</div>`), but no line breaks found.',
    line: 3,
    column: 19,
    nodeType: 'HTMLEndTagOpen',
    messageId: 'unexpectedBeforeOpeningBracket',
    endLine: 3,
    endColumn: 37,
    fix: { range: [Array], text: '\n' }
  }
]

But if I remove the whitespace around the comment:

  valid: [{
    code: `
      <template>
        <div attr><!-- comment --></div>
      </template>`,
    options: [
      {
        ignoreComments: true
      }
    ]
  }]

the test passes fine. I didn't see this issue in the multiline rule.

In this section of the rule code here:

        if (
          ignoreWhenEmpty &&
          elem.children.length === 0 &&
          template.getFirstTokensBetween(
            elem.startTag,
            elem.endTag,
            getTokenOption
          ).length === 0
        ) {
          return
        }

I'm not sure why it's required to test elem.children.length === 0 and getFirstTokensBetween().length === 0—indeed, commenting that line out:

        if (
          ignoreWhenEmpty &&
          // elem.children.length === 0 &&
          template.getFirstTokensBetween(
            elem.startTag,
            elem.endTag,
            getTokenOption
          ).length === 0
        ) {
          return
        }

then makes this valid:

      <template>
        <div attr> <!-- comment --> </div>
      </template>`

but still reports errors for this:

      <template>
        <div attr> content <!-- comment --> </div>
      </template>`

[FloEdelmann]

I'm not sure why it's required to test elem.children.length === 0 and getFirstTokensBetween().length === 0

Probably it is indeed not needed, seems like both were added at the same time in #684. I guess if the other tests are passing, then removing the elem.children.length check should be fine.

[dsl101]

Ah, this is a bit more complicated than I thought. Without my proposed changes, the default settings for singleline-html-element-content-newline give these test results:

1. <div></div> → OK
2. <div> </div> → OK
3. <div attr></div> → OK
4. <div attr> </div> → Fail

I'm not sure if the behaviour here is really consistent—shouldn't 2 and 4 give the same result? Because the setting is ignoreWhenNoAttributes and defaults to true, I can't see a way to configure that case 4 passes the test. Is the only option here to just disable the singleline-html-element-content-newline rule altogether?

It complicates my use case, as it's very common to put white space around comments, but then that white space counts as 'content':

<div attr><!-- comment --></div> → OK
<div attr> <!-- comment --> </div> → Fail

But I think that's OK, and is consistent with the existing rules / tests. I'll push an update with documentation for you to see.

[FloEdelmann]

It seems that the simple fix is also not working correctly for multiline-html-element-content-newline, see https://deploy-preview-2581--eslint-plugin-vue.netlify.app/rules/multiline-html-element-content-newline.html#ignorecomments-true:

<template>
  <!-- should be good, but actually is reported because the div is now 2 line breaks below the template -->
  <div class="red">  <!-- no error is reported here -->
    content
  </div>
</template>

So apparently a more elaborate logic is needed after all, probably that logic can then also be used for the singleline-html-element-content-newline rule.

[FloEdelmann]

By the way, while you're at it: Could you please add a "Related rules" section here and link to vue/singleline-html-element-content-newline from this rule docs page?

And also vice-versa a link from vue/singleline-html-element-content-newline to vue/multiline-html-element-content-newline.

[dsl101]

Yes, since comments are ignored, you would almost always want allowEmptyLines: true since it's not clear how many empty lines you'll have. I couldn't see an easy way around that.

[dsl101]

I'd love to spend more time digging in, but I'm afraid I have to get back to the project I'm using this on! I guess I'll just have to disable those rules for now...