vuejs / eslint-plugin-vue

Official ESLint plugin for Vue.js
https://eslint.vuejs.org/
MIT License
4.41k stars 659 forks source link

`vue/no-multiple-template-root` false negative with comment #2316

Open matthew-dean opened 8 months ago

matthew-dean commented 8 months ago

Checklist

Tell us about your environment

Please show your full configuration:

module.exports = {
  extends: ['../../../.eslintrc.js', 'plugin:storybook/recommended'],
  root: true,
  env: {
    node: true,
    browser: true
  },
  overrides: [
    {
      files: ['*.ts', '*.tsx', '*.vue'],
      parser: 'vue-eslint-parser',
      parserOptions: {
        parser: '@typescript-eslint/parser',
        project: './tsconfig.json',
        extraFileExtensions: ['.vue']
      },
      globals: {
        definePageMeta: 'readonly',
        $fetch: 'readonly',
        Stripe: 'readonly'
      },
      rules: {
        /**
         * This has unexpected behavior when used with other TS settings which produce
         * unexpected undefines, such as a map.get after testing for map.has
         */
        '@typescript-eslint/no-non-null-assertion': 'off'
      }
    },
    {
      files: ['**/pages/**/*.vue'],
      rules: {
        /**
         * Even though this is allowed in Vue3,
         * both Nuxt and Knockout need a single
         * root element.
         */
        'vue/no-multiple-template-root': 'error'
      }
    }
  ]
}

What did you do?

<template>
  <!-- this is a comment -->
   <div></div>
</template>

IMPORTANT CONTEXT: the Vue compiler has comments: true turned on (as a necessity). What this means is that the root template in this mode actually has multiple roots, including a blank text node, a comment node, and the div. I want to prevent developers from making this mistake, so I need a way to flag this as an error.

What did you expect to happen?

I expected an ESLint error when there are multiple possible roots.

What actually happened?

There is no error, hence no output.

Repository to reproduce this issue

https://stackblitz.com/edit/vitejs-vite-mhdbqk?file=src%2FApp.vue

FloEdelmann commented 8 months ago

I see that you are using Vue 3 in your reproduction repository, and manually enable the vue/no-multiple-template-root rule. Since Vue 3 does actually support multiple template root elements, the additional comment should not cause an error, right?

So do I understand correctly that this is a convention that you would like to enforce?

I guess that setting the comments: true compiler option is somewhat an edge case, and additionally enforcing the convention that there should not be any comments in the template root is probably even rarer.

matthew-dean commented 8 months ago

@FloEdelmann The rule itself is not that much of an edge case in a Vue 3 environment, considering Nuxt also advises you to have a single root for pages / layouts if you want any sort of transitions. Yes, comments: true is somewhat an edge case, but in that event, I would like a linting rule to say whether or not the template actually has a single root. When a comment node is present, it doesn't.

FloEdelmann commented 8 months ago

Nuxt also advises you to have a single root for pages / layouts if you want any sort of transitions

Good to know; would you fancy opening a PR to add this (with a link to the relevant Nuxt docs) to the rule docs?

I would like a linting rule to say whether or not the template actually has a single root. When a comment node is present, it doesn't.

We don't know in ESLint whether the comments Vue compiler option is actually set, so we'd either have to always include comment nodes in the rule, or add a rule option includeComments that changes the behavior. I'm leaning towards the latter, given that a warning about a comment node might be confusing for most users who don't have the comments compiler option set.

brc-dd commented 5 months ago

These are the corresponding Nuxt docs (https://nuxt.com/docs/guide/directory-structure/pages#usage):

image
SimonBackx commented 3 months ago

When you have a comment in the root of your template in vue 3, using $el will point to the first text comment instead of the actual DOM element. This can cause unwanted bugs, certainly because the comments are only stripped in production.

<template>
    <!-- Oops -->
    <div>
        Hello world
    </div>
</template>

<script lang="ts">
import { defineComponent } from "vue";

export default defineComponent({
    mounted() {
        console.log(this.$el); // logs `#text ""`
    }
});
</script>
FloEdelmann commented 1 month ago

I guess that we can add an option to the vue/no-multiple-template-root rule that can be explicitly enabled, e.g.

{
  "vue/no-multiple-template-root": ["error", {
    disallowComments: true // default: false
  }]
}

@SimonBackx That sounds like a bug in Vue itself though, don't you think? Did you report it there already?