dom icon indicating copy to clipboard operation
dom copied to clipboard

Consider support for ES2015 iterator protocol for NodeIterator

Open Ginden opened this issue 8 years ago • 12 comments

ES2015 introduces support for iteration protocols. Iterators are objects with .next method which return object {value: value, done: true/false}. Iterables are objects with Symbol.iterator method that returns iterator.

Making NodeIterator an iterator would be non-breaking change that will allow simple iteration of object using for-of loop or converting results to static array by spread operator.

Simplest implementation would be:

NodeIterator.prototype.next = function() {
     const value = this.getNextNode();
     return {
          value,
          done: !!val
     };
};
NodeIterator.prototype[Symbol.iterator] = function() {
   return this;
}

I think real specification should include cloning "initial state" of NodeIterator to allow multiple iterations over the same NodeIterator.

Similar logic can be applied to XPathResult objects that expose iterateNext method.

Related Firefox bug.

Ginden avatar Jan 12 '16 10:01 Ginden

@bzbarsky doing this would require quite some IDL support I think.

Do we really care enough about these objects to continue to maintain them and add new features?

annevk avatar Jan 12 '16 12:01 annevk

I ran into this too. As there is no way to select Text Nodes with selectors, using Node Iterator is still something we need to fallback to. It mean having to create an node iterator, for which I then needed an ES iterator.

marcoscaceres avatar May 16 '16 00:05 marcoscaceres

I would love to see TreeWalker be iterable as well.

justinfagnani avatar Feb 14 '22 20:02 justinfagnani

Agree also about TreeWalker... Having to use a generator is not ideal. For example:

https://github.com/w3c/respec/blob/9859c7bb2e501a15d9836c509e75069680779b43/src/core/utils.js#L757-L773

marcoscaceres avatar Feb 20 '22 23:02 marcoscaceres

@annevk would adding iterable<Node>; to the NodeIterator idl suffice?

keithamus avatar Sep 19 '23 16:09 keithamus

You would also have to define the behavior. But if we wanted people to use these APIs we should also add constructors and modernize them a bit. It's just not clear to me they have the use that warrants such an investment. If you compare [javascript] NodeIterator and [javascript] querySelector on Stack Overflow the difference is quite stark. (There's also other drawbacks such as them executing script while traversing, making for expensive boundary hopping.)

annevk avatar Sep 20 '23 07:09 annevk

To me it's one of those areas where it's not used often but when it's needed there are no alternatives (same for TreeWalker) - that I know of.

I'm happy to put the work in to define what iterable<Node>; on both would do, along with browser patches. I'm also happy to modernise both APIs to become constructors, and maybe while we're at it clean up the whatToShow/filter params. Alternatively if you think there's a better direction to go in, for example defining a new mechanism for iterating over nodes, I'm happy to pursue that.

keithamus avatar Sep 20 '23 08:09 keithamus

I mean the alternative is to just write the iteration code yourself, right? That'll likely be faster due to the lack of boundary-crossing, and it'll be easier to read and understand without someone having to go look up this esoteric API. I think the first step here would be demonstrating why it's so bad to write loops in JavaScript.

domenic avatar Sep 20 '23 09:09 domenic

As in manually looping over childNodes?

keithamus avatar Sep 20 '23 10:09 keithamus

Yeah, creating your own tree walker. You could use childNodes, but also firstChild and nextSibling, etc.

annevk avatar Sep 20 '23 11:09 annevk

Do you think it is worth documenting this in MDN - discouraging use of TreeWalker/NodeIerator in favour of iterating over these properties?

keithamus avatar Sep 21 '23 10:09 keithamus

I agree, these APIs definitely definitely aren't perfect. There is confusing overlap between whatToShow and filter, the difference between NodeFilter.FILTER_REJECT and NodeFilter.FILTER_SKIP is definitely not intuitive, and the difference between TreeWalker and NodeIterator is surprisingly subtle. And, of course, they don't support the iterator protocol despite one of them literally being called 'Iterator' 😄.

I still think they can be useful -- it's nice to have a built-in API to give you the confidence you're not making a significant performance blunder. I wouldn't mind keeping them around with some improvements.

If you compare [javascript] NodeIterator and [javascript] querySelector on Stack Overflow the difference is quite stark.

I'm not convinced the comparison is valid here. If you need to iterate over a flat array of elements, querySelectorAll is a fine alternative. But NodeIterator isn't limited to HTMLElement -- it also can include text nodes, Attr nodes, CDATA, etc. The Text node use case in particular makes this API necessary in some scenarios.

I mean the alternative is to just write the iteration code yourself, right?

This is a fair point -- a simple lazy tree-walker can be built with a recursive generator:

function* walkNodeTree(root) {
  yield root;
  for (const node of root.childNodes) yield* walkNodeTree(node);
}

This isn't hard, but it's also not completely trivial. Add on support for skipping individual nodes / rejecting node subtrees and the logic starts to get a little more complex. And I doubt there are many browsers that will tail-call optimize a generator, so you'll probably want to add a trampoline.... there's plenty of subtle considerations here.

I think the first step here would be demonstrating why it's so bad to write loops in JavaScript.

It seems to me that the primary advantage of the original feature request (adding iterator support) would actually be that it makes these APIs easier to use with loops (for (const node of iterator {...}). So this request is an argument for loops, not against them.

iansan5653 avatar Sep 27 '23 22:09 iansan5653