Rule proposal: no duplicate attributes

[mattlewis92]

When investigating switching from ViewEngine to ivy we stumbled on a problem where we had components with the same attribute being passed in twice and the behaviour is different between view engine and ivy. e.g. consider the component:

@Component({
  selector: 'foo',
  template: '{{ bar }}',
})
export class FooComponent {
  @Input() bar: string
}

Is used like this:

<foo [bar]="'bam'" [bar]="'baz'"></foo>

ViewEngine: uses the first input and prints bam
Ivy: uses the second input and prints baz

So I think this would make sense as a lint rule to detect these duplicate attributes so that they can be removed (usually they are added by accident by merge conflicts). I did a quick check with AST explorer and it looks like they are represented in the
AST so it should be possible to write a lint rule to detect them.

I would be happy to work on this myself, I think it should be straightforward to implement. If anyone has any suggestions on a descriptive name for the rule that would be very helpful 🚲

[rafaelss95]

Hey @mattlewis92, thanks for opening this proposal here (it looks like the same I proposed in Codelyzer) and I would love to see this rule here also :). If it gets approved, it would be nice to handle not only BoundAttribute, but also TextAttribute and BoundEvent, so all these cases should have error/warn:

<input (change)="changeHandler($event)" (change)="changeHandler($event)"><!-- BoundEvent + BoundEvent -->
<input [(ngModel)]="model" [(ngModel)]="model"><!-- BoundEvent + BoundEvent -->
<foo [bar]="'bam'" bar="baz"></foo><!-- BoundAttribute + TextAttribute -->

...and if that's the case, in order to describe the intent better, the name would need to be more descriptive.

---

Hmm, another case came to my mind:

<input [(ngModel)]="model" (ngModelChange)="model = $event">

In this case, the assignment would happen two times, since [(ngModel)] is just a short-cut to [ngModel]="model" (ngModelChange)="model = $event", as detailed in angular/angular#11234 (comment), but if it's considered much restrictive for this rule, could we have an option for this then?

---

Also, about the input order, there's an opened issue in Angular repo for this: angular/angular#40007.

[mattlewis92]

Hey @mattlewis92, thanks for opening this proposal here (it looks like the same I proposed in Codelyzer) and I would love to see this rule here also :). If it gets approved, it would be nice to handle not only BoundAttribute, but also TextAttribute and BoundEvent, so all these cases should have error/warn:

<input (change)="changeHandler($event)" (change)="changeHandler($event)"><!-- BoundEvent + BoundEvent -->
<input [(ngModel)]="model" [(ngModel)]="model"><!-- BoundEvent + BoundEvent -->
<foo [bar]="'bam'" bar="baz"></foo><!-- BoundAttribute + TextAttribute -->

...and if that's the case, in order to describe the intent better, the name would need to be more descriptive.

Excellent idea!

Hmm, another case came to my mind:

<input [(ngModel)]="model" (ngModelChange)="model = $event">

In this case, the assignment would happen two times, since [(ngModel)] is just a short-cut to [ngModel]="model" (ngModelChange)="model = $event", as detailed in angular/angular#11234 (comment), but if it's considered much restrictive for this rule, could we have an option for this then?

I think this would be better as an option, it's sometimes handy when you want to do something when the model value is changed (but also want to keep the assignment) e.g.

<input [(ngModel)]="model" (ngModelChange)="doSomethingWhenModelValueChanged()">

Also, about the input order, there's an opened issue in Angular repo for this: angular/angular#40007.

Thank you for sharing that, was really interesting to read up more about it 😄

[JamesHenry]

This seems like a good idea for a rule! PRs very welcome