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

Provide getAttributeNames() which returns an Array<DOMString> #115

Closed
esprehn opened this issue Nov 21, 2015 · 17 comments

Comments

@esprehn
Copy link

commented Nov 21, 2015

Element.prototype.attributes requires allocating all the Attr node instances and the NamedNodeMap itself which is expensive, lots of frameworks (ex. Polymer, Angular, others) just want to iterate the attributes at construction time of an element or when traversing the document and don't need these expensive persistent structures.

Instead we should provide getAttributeNames(), then authors can use that with getAttribute/setAttribute/removeAttribute which results in better memory usage and performance.

This would also get developers to stop using .attributes which means the usage of the exotic Node like thing would go down possibly allowing the removal of it in the future.

@domenic @foolip

@foolip

This comment has been minimized.

Copy link
Member

commented Nov 21, 2015

That sounds pretty nice, not unlike Object.getOwnPropertyNames.

My biggest question mark is around namespaces, without which this would be a simple API. Because of them, one can't return an array of strings where each string points to a unique attribute:

var e = document.createElement('span');
e.setAttributeNS('http://a.example.com/', 'foo', 'bar');
e.setAttributeNS('http://b.example.com/', 'foo', 'baz');
alert(e.getAttribute('foo'));
e.removeAttribute('foo');
alert(e.getAttribute('foo'));

I'm sure you could come up with a crazy case involving prefixes too.

Should the API throw an exception if any of the attributes are in a namespace, or return a tuple of strings instead of a string for those cases?

@annevk

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

I think we should just return the qualified names, those are the property names NamedNodeMap already exposes and what most algorithms operate on.

If you just stick to namespaceless attributes as you should, there will be no issue.

@foolip

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

With qualified names you could still have the same string appear multiple times, do we just live with that? It would be nice if one didn't have to resort to NamedNodeMap to guarantee that all attributes are found, but I see no other way to give that guarantee than to have a namespace+localName tuple :(

@annevk

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

Yeah, and to be fair, for mutation observers we do care about all attributes. Perhaps we should return an array of two-item-arrays ([namespace, localName])?

@foolip

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

That would certainly do the job, although it's unfortunate to waste memory on those arrays for the vast majority of attributes that aren't in a namespace. A mixture of strings and arrays isn't exactly perfect, either.

Any preferences, @esprehn?

@annevk

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

The alternative is to continue with precedent: getAttributeNames() and getAttributeNamesNS(). Such joy.

@foolip

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

Joy indeed, but that would probably work. One would lose the order between the two sets of attributes, but I'm not sure if that could possibly matter.

@foolip

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

To clarify, with the attributes API one can see if an attribute with a namespace comes before an attribute without a namespace and vice versa.

@zcorpan

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

The naming conflict issue is already there when using getAttribute(attributes[n].name). If people cared about it, they would use NS methods everywhere, but it seems like it's not really a problem.

@foolip

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

@annevk clarified getAttributeNames() and getAttributeNamesNS() on IRC, both would return all attributes. I was assuming that the attributes would be split into two buckets.

@domenic

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

I'd prefer just getAttributeNames() and wait for someone to ask for getAttributeNamesNS()...

@foolip

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

I'd be OK with that, if the spec has a note saying that it's not guaranteed that the values will be unique, so you can't use it to implement something like Live DOM Viewer.

@annevk

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

Yeah, this all sounds good. @smaug----, any concerns?

@smaug----

This comment has been minimized.

Copy link
Collaborator

commented Nov 24, 2015

So getAttributeNames() would return a new sequence each time?
Are we concerned about silly usage like
for (var i = 0; i < elem.getAttributeNames().length; ++i) {
// do something with elem.getAttributeNames()[i];
}

It is just that .attributes doesn't change, so fewer new objects there.

Would something like
readonly attribute FrozenArray attributeNames;
work, and in such way that it is replaced with a new array whenever an attribute is added or removed?

But I don't object getAttributeNames().

@annevk annevk closed this in fb45d52 Nov 25, 2015

@annevk

This comment has been minimized.

Copy link
Member

commented Nov 25, 2015

I went with getAttributeNames() and the hope that developers use it wisely. Adding another associated object to Element that needs to be updated correctly seems cumbersome.

@ArkadiuszMichalski

This comment has been minimized.

Copy link
Contributor

commented Dec 3, 2015

@annevk Return empty sequence when element has no any attributes is implicitly or should be explicitly defined in prose (like here: https://url.spec.whatwg.org/#dom-urlsearchparams-getall, https://fetch.spec.whatwg.org/#dom-headers-getall, https://xhr.spec.whatwg.org/#dom-formdata-getall)?

@zcorpan zcorpan reopened this Dec 3, 2015

@annevk annevk closed this in 7b5e052 Dec 15, 2015

@annevk

This comment has been minimized.

Copy link
Member

commented Dec 15, 2015

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
You can’t perform that action at this time.