box/box-node-sdk

Make PagingIterator support iterable protocol as well

KKamishima opened this issue · 2 comments

Is your feature request related to a problem? Please describe.

ES2018 supports for-await-of statements, which conveniently iterates over an async iterable object.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for-await...of

PagingIterators returned by enumerating methods of the SDK when config.iterators set to true, however, cannot be directly used in such statements since PagingIterator is just an async iterator, not an async iterable.

const pageit = await client.folders.getItems(folder_id);
for await (const item of pageit) { // won't work
}

Describe the solution you'd like

Among several approaches that we may come up with, the simplest one shall be making PagingIterator support the async iterable protocol as well.
It is as simple as adding the following method to PagingIterator class.

[Symbol.asyncIterator]() { return this; }

Note: I am not sure this code is compatible with pre-ES2018 runtime which lacks definition for Symbol.asyncIterator.

I am following an idea described in the note in the corresponding MDN article.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols

Describe alternatives you've considered

The suggested solution above has a flaw as the users of an iterable object might expect getting a fresh iterator every time one calls @@asyncIterator method, although it is not mandated by the specification.

It might be more pedantically correct if we had a separate iterable object and making all enumerating SDK methods return it instead of directly returning an iterator, but it is a breaking change.

Additional context

@KKamishima This is an issue I have an encountered as well. Iterable support would be nice, but as you mentioned it may be a breaking change. We will look into this, but it may be a while before we are able to introduce a breaking change. Thanks for the feedback!

@sujaygarlanka Thanks for taking a look into it.
Please keep in mind that the solution I'd recommend is hardly a breaking change. Existing clients would just ignore an extra property [System.asyncIteraotor].
I have just mentioned the breaking change in the explanation for an alternative solution (which I like less.)