Skip to content

Rule Proposal: vue/component-tags-order #140

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

Closed
julientechdev opened this issue Aug 8, 2017 · 12 comments · Fixed by #763
Closed

Rule Proposal: vue/component-tags-order #140

julientechdev opened this issue Aug 8, 2017 · 12 comments · Fixed by #763

Comments

@julientechdev
Copy link

Please describe what the rule should do:

This rule warns about the order of the script, template & style tags.

What category of rule is this? (place an "X" next to just one item)

[x] Enforces code style
[ ] Warns about a potential error
[ ] Suggests an alternate way of doing something
[ ] Other (please specify:)

Provide 2-3 code examples that this rule will warn about:

vue/component-tags-order: ["warning", order: [ "script", "template", "style" ] ];

<template></template>
<script></script>
<style></style>

However, the following should be considered fine:

<script></script>
<template></template>
<style></style>
@julientechdev julientechdev changed the title Rule Proposal: vue/component-tag-order Rule Proposal: vue/component-tags-order Aug 8, 2017
@julientechdev julientechdev changed the title Rule Proposal: vue/component-tags-order Rule Proposal: vue/component-tags-order Aug 8, 2017
@mysticatea
Copy link
Member

mysticatea commented Aug 13, 2017

Thank you for this proposal!

Sounds good to me.
Additionally, we should consider about custom blocks.

@chrisvfritz
Copy link
Contributor

chrisvfritz commented Nov 27, 2017

We'll have to discuss the best default for this. I think template, script, style is the most common, because it's what we usually demonstrate in the docs currently, but I've been somewhat recently convinced that script, template, style makes more sense, because:

  • The script is probably the least likely tag to be missing
  • The script defines the interface for the component, which is probably the most referred-to information
  • You don't have to jump over the script when adding new classes to the template and style

I'll ping some other contributors for their thoughts here.

@Akryum
Copy link
Member

Akryum commented Nov 27, 2017

@chrisvfritz This make sense.

@julientechdev
Copy link
Author

julientechdev commented Nov 27, 2017

@chrisvfritz We switched to script, template, style a while back at my company for the reasons you mentionned

@posva
Copy link
Member

posva commented Nov 27, 2017

In my experience template is the least likely tag to be missing, then script, then style
If I don't need a template tag it's because I'm writing a more advanced component that doesn't have style and prefer to write a js or jsx file

I've written components with no script, only HTML but I've never written components with no template and a style

@pi0
Copy link

pi0 commented Nov 27, 2017

Some small thoughts on this enforcement. While consistency is a really good thing, there are some conditions that not one rule suits for every use case and component.

template > style > script

Consider when a component has a LOT of styles. Every time we need to check the relation between VM and template we need to either fold style tag or scroll a LOT to the end of the file to see the js part.

template > script > style

The same as previous, when there are LOTS of js logic. Adding any simple style needs LOTS of scrolling between template and style parts.

Personally, I use a rule of the thumb that count/estimate lines of the code for each section in component and put the longest part to the end so the code is more READABLE. Suggestion for this rule proposal:

  • Enforcing template to be the first tag if exists
  • Counting Lines of other tags (Including custom tags) and making a threshold for lines diff (Like 100 lines) and making soft warns recommending to reorder from less to more for more readability.

@julientechdev
Copy link
Author

@pi0 This is considerably more complicated, and does not respond to the initial demand. Why not, but I think it should be added later as an option for this rule.

@mario-d-s
Copy link

I would like to have a rule like this. Is there anything I can do to help this to completion?

@chrisvfritz
Copy link
Contributor

@mario-d-s You'd be welcome to develop it! I'm thinking that for now though, this rule would remain uncategorized and therefore would have no default. Eventually, I'd probably like to make sure this rule supports autofix, then add it to the style guide under recommended with the default of script, template, style.

The only thing holding me back is that we've mostly encouraged template, script, style up until now and it does take people some getting used to for relatively little benefit if code folding is used.

@chrisvfritz
Copy link
Contributor

chrisvfritz commented Jul 31, 2018

Actually, we could add it to recommended right now as long as this kind of ordering was supported (and the default): [['script', 'template'], 'style']. That would mean script and template must come first, but in any order, then style must be after both of those.

@mario-d-s
Copy link

@chrisvfritz I'd be happy to give it a shot. Your suggestion about "optional" ordering looks good.

Do note that I have zero experience writing ESLint rules so this will probably take me some time.

@michalsnik
Copy link
Member

michalsnik commented Aug 8, 2018

Unfortunately it's not possible to implement at this moment @mario-d-s The vue-eslint-parser returns an original AST with an extra templateBody property that contain AST describing Vue template. But we don't know anything about style tag.

Could we perhaps get the following AST @mysticatea?

{
  type: 'Program'
  body: [
    {
      type: 'VElement',
      name: 'template',
      children: [
        {
          type: 'VText",
          ...
        }
      ]
    },
    {
      type: 'VElement',
      name: 'script',
      children: [
        {
          type: 'VariableDeclaration',
          declaration: { ... }
        },
        {
          type: 'ExportDefaultDeclaration',
          declaration: { ... }
        }
      ]
    },
    {
      type: 'VElement',
      name: 'style',
      children: null
    }
  ]
}

It would make things easier, and maybe we wouldn't even need an extra template body visitor (though it's just my lucky guess).

edit: Now that I think about it I can imagine other plugins and rules might've already use selector that this kind of AST might break (e.g. Program > ExportDefaultDeclaration), so we probably can't change it with 100% certainty.

Alternatively we might add an extra field next to templateBody with informations about top-level tags - it would most likely be backwards compatible change. We already have access to the root AST from HTML Parser so it should be pretty straightforward to implement in vue-eslint-parser.

Looking once again at templateBody got me thinking 🤔Do you remember @mysticatea what was the reason behind assigning result.ast.templateBody = templateBody instead of straight result.ast.templateRoot = rootAST? Looks like it might just be the solution - it would also allow us to report too many lines between tags and warn about using not scoped styles later on.

I'm happy to help and hear your thoughts @mysticatea

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.

8 participants