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

Self closing elements break v-if rules #29

Closed
Akryum opened this issue Jun 16, 2017 · 13 comments · Fixed by #34
Closed

Self closing elements break v-if rules #29

Akryum opened this issue Jun 16, 2017 · 13 comments · Fixed by #34

Comments

@Akryum
Copy link
Member

Akryum commented Jun 16, 2017

Self-closing elements break the following rules:

  • vue/no-invalid-template-root
  • vue/no-invalid-v-else-if
  • vue/no-invalid-v-else

Expected

This should not be reported:

<template>
  <c1 v-if="1" />
  <c2 v-else-if="1" />
  <c3 v-else />
</template>

Happening

This works:

<template>
  <c1 v-if="1"></c1>
  <c2 v-else-if="1"></c2>
  <c3 v-else></c3>
</template>

This doesn't:

<template>
  <c1 v-if="1" />
  <c2 v-else-if="1" />
  <c3 v-else />
</template>

Here are the report messages:

  26:3  error  The template root requires the next element which has 'v-else' directives if it has 'v-if' directives     vue/no-invalid-template-root
  27:7  error  'v-else-if' directives require being preceded by the element which has a 'v-if' or 'v-else-if' directive  vue/no-invalid-v-else-if
  28:7  error  'v-else' directives require being preceded by the element which has a 'v-if' or 'v-else' directive        vue/no-invalid-v-else
@mysticatea
Copy link
Member

Thank you for this issue.

However, this is working correctly. HTML just ignores self-closing sign.

image

@yyx990803
Copy link
Member

Vue's template parser actually allows self closing components (when using string templates directly). Is there anyway to relax these rules?

@mysticatea
Copy link
Member

I got it.
Could you point the source code which transforms self-closing elements?

@yyx990803
Copy link
Member

There's no transform, but in https://github.com/vuejs/vue/blob/dev/src/compiler/parser/html-parser.js we just treat self closing tags as if they have a closing tag.

@mysticatea
Copy link
Member

mysticatea commented Jun 17, 2017

Thank you.

Hmm.
It seems to be hard that I implement it since I'm using a general HTML parser library. It might be realizable if I transform HTML text by regular expressions before parsing, but it's not safe. I will try to support self-closing after I make new HTML parser.

@yyx990803
Copy link
Member

@mysticatea have you thought of building the new parser on top of Vue's parser? I can expose the parse method in vue-template-compiler and you should be able to pretty easily transform the AST to the format suitable for ESLint. It would also be more accurate and kept up-to-date with Vue core updates.

@HerringtonDarkholme
Copy link
Member

HerringtonDarkholme commented Jun 17, 2017

@mysticatea I would like to integrate eslint-vue into https://github.com/octref/vetur, a editor tool for Vue.

If we can base eslint-vue and vetur on the same template parser, one additional parsing can be saved in editor! Also, fewer parsing incompatibility will be present.

@yyx990803 Currently vue-template-compiler will be both too much and too little for tooling. On one hand, generating render function string is unnecessary for tool. On the other, tools will need AST node range and location for traversal.

Let's see how can we improve.

@mysticatea
Copy link
Member

@yyx990803 It would be best if we use the same parser between Vue.js and eslint-plugin-vue. However, eslint-plugin-vue needs more information to lint than Vue.js; locations, tokens of AST nodes, comments, etc... vue-template-compiler looks to not provide those information.

@HerringtonDarkholme The parser that eslint-plugin-vue@3 is using is vue-eslint-parser, an independent package.

@yyx990803
Copy link
Member

yyx990803 commented Jun 17, 2017

@mysticatea oh yeah that's right...

In that case, is it possible to build the separate parser on top of Vue's own parser, enhancing it with Node location information? If that's possible, we can even use it to produce source maps for compiled render functions.

@yyx990803
Copy link
Member

yyx990803 commented Jun 17, 2017

^ I can think of two ways of doing this:

  1. Fork vue-template-compiler (or start from scratch) and maintain a parallel parser. This would be easier to implement, but keeping both parsers in sync could be extra maintenance burden.

  2. Improve the parser directly in Vue itself. The concern here is how much extra code this requires, because the compiler will be included in the full build so size is still a concern. Maybe we can implement it in a way that all the token range information logic is nested inside conditional blocks (similar to process.env.NODE_ENV) so that they can be stripped away in the browser oriented build?

@Akryum
Copy link
Member Author

Akryum commented Jun 17, 2017

Maybe we can implement it in a way that all the token range information logic is nested inside conditional blocks (similar to process.env.NODE_ENV) so that they can be stripped away in the browser oriented build?

Was about to suggest just that! 😄

mysticatea added a commit that referenced this issue Jun 18, 2017
- Make correct ASTs about self-closing elements (fixes #29)
- Improve the integration with eslint-plugin-import (refs #21)
@kazupon
Copy link
Member

kazupon commented Jun 18, 2017

Though it may be related to these, we might as well try to design plugin architecture of vue-template-compiler.

vuejs/vue#5703
vuejs/vue#5857

mysticatea added a commit that referenced this issue Jun 18, 2017
* Fix: upgrade vue-eslint-parser
  - Make correct ASTs about self-closing elements (fixes #29)
  - Improve the integration with eslint-plugin-import (refs #21)
* add comments to integration test
@mysticatea
Copy link
Member

@kazupon Thank you for the information.

After I re-thought this, I got idea to implement self-closing on the current parser, then I made it: #34.

pastelmind added a commit to cs374-riftguide/dp4 that referenced this issue Jun 8, 2020
- When the user clicks on a "View Guide" button in the Search Page, the
  app switches to the Guide Page and displays the contents of the guide.
- When the user clicks on the "Close Guide" button in the Guide Page,
  the app switches back to the Search Page.

Note: I must explicitly close component tags (<page-guide></page-guide>)
because using self-closing tags (<page-guide />) breaks v-if logic.
For more information on this issue, see:

- https://vuejs.org/v2/style-guide/#Self-closing-components-strongly-recommended
- vuejs/eslint-plugin-vue#29
doug-wade pushed a commit to doug-wade/eslint-plugin-vue that referenced this issue Apr 17, 2022
…npm_and_yarn/javascript-package/eslint-plugin-jest-26.1.4

Bump eslint-plugin-jest from 26.1.1 to 26.1.4 in /javascript-package
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants