whatwg / dom

DOM Standard
https://dom.spec.whatwg.org/
Other
1.58k stars 295 forks source link

iterable declaration on NodeList is invalid #284

Open bzbarsky opened 8 years ago

bzbarsky commented 8 years ago

NodeList has:

getter Node? item(unsigned long index);
iterable<Node>;

but http://heycam.github.io/webidl/#idl-iterable says:

A value iterator MUST only be declared on an interface that supports indexed properties and has an integer-typed attribute named “length”. The value-type of the value iterator MUST be the same as the type returned by the indexed property getter.

In this case the types are different: one is Node and the other is Node?.

bzbarsky commented 8 years ago

The iterable declaration on DOMTokenList has the same problem.

annevk commented 8 years ago

It seems though that the iterator can never return null, right? Shouldn't we allow for the removal of nullability in that case?

bzbarsky commented 8 years ago

That's not at all obvious from the IDL. The interface says it has an indexed getter that can return null; the iterator will use the indexed getter internally, so can return null too. The fact that the indexed getter can't actually return null in this case is buried in prose.

If the interface had:

Node? item(unsigned long index);
getter Node (unsigned long index);

then you could write iterator<Node> without any qualms. Presumably you would define some shared operation that item and the indexed getter both invoke.

Now maybe we should assume that anything with an indexed getter is lying about the getter being able to return null, because it's layering the getter on top of an accessor function which can in fact return null (but only calling it in cases when it can't). I'd have to do a really careful audit of all the 40-ish indexed getters in the platform before being willing to make that assumption, and it would be rather fragile if people add something new...

annevk commented 8 years ago

Maybe it's not worthwhile since we don't want new APIs to use getters, but I might have a look before changing this IDL.