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

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

Open
2 tasks done
matthew-dean opened this issue Nov 16, 2023 · 6 comments
Open
2 tasks done

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

matthew-dean opened this issue Nov 16, 2023 · 6 comments

Comments

@matthew-dean
Copy link

matthew-dean commented Nov 16, 2023

Checklist

  • I have tried restarting my IDE and the issue persists.
  • I have read the FAQ and my problem is not listed.

Tell us about your environment

  • ESLint version: 8.50.0
  • eslint-plugin-vue version: 9.15.1
  • Node version: 18.13.0
  • Operating System: macOS

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 FloEdelmann changed the title vue/no-multiple-template-root doesn't throw an error when it should vue/no-multiple-template-root false negative with comment Nov 16, 2023
@FloEdelmann
Copy link
Member

FloEdelmann commented Nov 16, 2023

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
Copy link
Author

@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
Copy link
Member

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
Copy link
Member

brc-dd commented Jan 30, 2024

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

image

@SimonBackx
Copy link

SimonBackx commented Apr 27, 2024

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
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants