Rule proposal: template/attributes-order

[SchnWalter]

It makes sense for structural directives like *ngIf to be first in the list of attributes:

<div [ngClass]="item.classes" my-decorator *ngIf="item.isVisible" [my-decorator-extras]="item.extras">
  {{ item.text | translate }}
</div>

I don't promise anything, but if I have the time, I'll look into it this month.

P.S. Some issue templates could be useful. I'll also look into this.

P.S.2 Thanks for all the hard work!

[rafaelss95]

Hey @SchnWalter, although I understand your concern about this specific order, I could imagine that some people would find another one better to them. Having this in mind, I believe that we could follow the proposal that we already have in Codelyzer, which gives the ability to configure the order to your liking. What do you think?

[xaosaki]

Hi @SchnWalter , do you have any updates?

[nickbullock]

Hey guys, I have just had an attempt to implement this rule, see this commit
I wanted to share with you some info about what I did and what I don't know how to do related to this rule, may be somebody will have an idea how to handle it.

To implement this rule I want to be able to identify following entities to sort them further:

enum OrderAttributeType {
  TEMPLATE_REFERENCE = 1,
  STRUCTURAL,
  CLASS,
  STYLE,
  ID,
  NAME,
  DATA,
  SRC,
  FOR,
  TYPE,
  HREF,
  VALUE,
  TITLE,
  ALT,
  ROLE,
  ARIA,
  INPUT,
  OUTPUT,
  BANANA,
  ANIMATION,
  DIRECTIVE,
  OTHER_ATTRIBUTES
}

Done:

• Identify plain html attributes like id, class, src, href, etc
• Identify outputs
• Identify template references
• Identify structural directives
• Identify animations

Not done, but can be done:

• Identify banana boxes

Not done and seems not possible to be done

• Distinguish component/directive inputs from attributes binding and directive usage

Example:

<app-component [class]="className"><app-component>

At the parsing moment the compiler doesn't know what exactly class property refers to. It can be one of the following:

1. component input

@Component({
  selector: 'app-component',
  template: ''
})
export class AppComponent {
  @Input() class: string;
}

2. directive usage

@Directive({
  selector: 'class'
})
export class ClassDirective {}

3. class attribute

And all of them are valid and possible cases, which can be known only after compilation.

I see these possible solutions:

• add more information to the import { parseTemplate } from '@angular/compiler' method, but it should to be a huge architectural change, since we know that angular apply components and directives much later at runtime.
• create this rule as a webpack plugin to get all the needed info from AngularCompilerPlugin, but in this case I guess it will not work in IDE (vscode/webstorm) and can be much slower

@JamesHenry @rafaelss95 any thoughts/advice on this?

[rafaelss95]

Hey @nickbullock, thanks for taking a look on this! I saw what you did and I would like to share some opinions.

• Identify plain html attributes like id, class, src, href, etc
• Identify outputs
• Identify template references
• Identify structural directives
• Identify animations

I guess the configuration depends as much on the level of granularity we may want with this rule.

I see that you created an option for each global attribute... but if we follow up this line of thinking, we would end up with a big list of configuration (see the full list of global attributes).

I think we should instead have an option for global attributes (following the link above), however I don't see the reason in separating global attributes from inputs (when not used with property binding of course, e.g. <input tabindex="-1" customId="1"). The same about animations.

I was thinking in something more like this:

{
  "rules": {
    "@angular-eslint/template/attributes-order": [
      "error", {
        "alphabetical": false,
        "order": [
          "STRUCTURAL_DIRECTIVE" // e.g. `*ngIf="true"`, `*ngFor="let item of items"`
          "TEMPLATE_REFERENCE_VARIABLE", // e.g. `<input #inputRef>`
          "ATTRIBUTE_BINDING", // e.g. `<input required>`, `id="3"`
          "INPUT_BINDING", // e.g. `[id]="3"`, `[attr.colspan]="colspan"`, [style.width.%]="100", [@triggerName]="expression", `bind-id="handleChange()"`
          "OUTPUT_BINDING", // e.g. `(idChange)="handleChange()"`, `on-id="handleChange()"`
          "TWO_WAY_BINDING", // e.g. `[(id)]="id"`, `bindon-id="id"
        ]
      }
    ]
  }
}

• Distinguish component/directive inputs from attributes binding and directive usage

I don't think we need to do this. We can only consider it according to what the compiler gives us (BoundAttribute and TextAttribute).

---

Note that this config is just an idea and both it and the names can be changed and I would love to see other opinions. It would also be interesting to discuss a default configuration.

[guilhermetod]

I'm with @rafaelss95 on the simplified version. But I think that it would be easier to make it more visually organized if we could disitinguish between bindings and plain assignments, even if it means mixing inputs and attributes, similar to the suggestion on the issue I opened (#186) when i failed to realize this one already existed.

[rafaelss95]

@guilhermetod if I got your statement correctly, ATTRIBUTE_BINDING and INPUT_BINDING handle this, no?

[guilhermetod]

@rafaelss95 From the examples you gave, both [attr.colspan]="colspanDefinedInTS" and colspan="2" would fall under ATTRIBUTE_BINDING. This could lead to a template sorted like:

<app-component
    aria-label="app"
    [attr.colspan]="colspanDefinedInTS"
    class="lits-item--collapsed"
    [inputValue]="inputValue"
></app-component>

I would suggest separating binding (bracket syntax) and plain assignment (like the atia-label and class attributes), so sorting the above would become:

<app-component
    aria-label="app"
    class="lits-item--collapsed"
    [attr.colspan]="colspanDefinedInTS" <!-- Group the bracket syntax --> 
    [inputValue]="inputValue"
></app-component>

[rafaelss95]

@guilhermetod Aha, I understand what you mean now!! Sorry about that, my example was wrong, thanks for noticing that. It should be correct now 😄

[nickbullock]

Thanks for such a blazing fast reaction guys.
The problem is that I wanted to be able to separate directives from attributes and inputs, so my ideal sorting is like:

<app-component
    #ref
    *ngIf="flag"
    class="className"
    [input1]="val1"
    [input2]="val2"
    (onChange)="handleChange($event)"
    matInput
    cdkTextareaAutosize
    [cdkCopyToClipboard]="text"
></app-component>

But as you can see it can't be achieved at the moment.
By the way, it works for vue guys because they have all directives prefixed with v- https://eslint.vuejs.org/rules/attributes-order.html

          "STRUCTURAL_DIRECTIVE" // e.g. `*ngIf="true"`, `*ngFor="let item of items"`
          "TEMPLATE_REFERENCE_VARIABLES", // e.g. `<input #inputRef>`
          "ATTRIBUTE_BINDING", // e.g. `<input required>`, `id="3"`
          "INPUT_BINDING", // e.g. `[id]="3"`, `[attr.colspan]="colspan"`, [style.width.%]="100", [@triggerName]="expression", `bind-id="handleChange()"`
          "OUTPUT_BINDING", // e.g. `(idChange)="handleChange()"`, `on-id="handleChange()"`
          "TWO_WAY_BINDING", // e.g. `[(id)]="id"`, `bindon-id="id"

I got your point @rafaelss95 @guilhermetod about sorting by brackets (kinda), looks good to me, unfortunately it's not what my team uses since this sorting will not help you to find components inputs, because they will be mixed with other [] stuff. But at least it can be done easily.

[nickbullock]

Hey guys let's discuss default order and other stuff. #605