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

Make PagingIterator support iterable protocol as well #528

Open
KKamishima opened this issue Jul 30, 2020 · 2 comments
Open

Make PagingIterator support iterable protocol as well #528

KKamishima opened this issue Jul 30, 2020 · 2 comments

Comments

@KKamishima
Copy link

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

@sujaygarlanka
Copy link
Contributor

@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!

@KKamishima
Copy link
Author

@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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants