cleanup how component id attr is set
#16561
Conversation
| } | ||
|
|
||
| if (seen.indexOf('id') === -1) { | ||
| operations.setAttribute('id', PrimitiveReference.create(component.elementId), true, null); |
There was a problem hiding this comment.
id was being set differently than when attributeBindings doesn't exist https://github.com/emberjs/ember.js/pull/16561/files#diff-1d6263d5fa618e89a735e9df0fe95f81L327
bekzod
force-pushed
the
fix-assymetry
branch
6 times, most recently
from
4280e9f
to
bbc29ca
Compare
| @@ -73,27 +73,26 @@ function applyAttributeBindings( | |||
| component: Component, | |||
| operations: ElementOperations | |||
| ) { | |||
| let seen: string[] = []; | |||
| let seen: Set<String> = new Set(); | |||
There was a problem hiding this comment.
I doubt Set to be actually faster for less than < 10 attributes and most elements don't have that many.
We should definitely try to test this.
There was a problem hiding this comment.
such thought crossed my mind too, so reverting this changes :P
bekzod
changed the title
id attr is setid attr is set
bekzod
changed the title
id attr is setid attr is set
|
not fixing here anything just avoiding calling |
|
The changes here seem good, but I did want to point out that |
there is asymmetry how
idattributes are set, this fixes it