Support async sequence types#278
Conversation
|
@domenic Could you review, or assign someone to review? Thanks. 🙏 |
| ${name}[Symbol.asyncIterator] !== undefined || | ||
| ${name}[Symbol.iterator] !== undefined |
There was a problem hiding this comment.
This is supposed to use GetMethod which throws for non-functions, as per step 11.1.1.1. But for sequences and frozen arrays, we seem to just check for undefined? Should we fix this?
There was a problem hiding this comment.
I think we should fix this, and ideally have some tests.
There was a problem hiding this comment.
Agreed, this need more tests. This is tricky stuff! 😅
I think I'll also port some of the input validation tests from readable-streams/from.any.js over to this project.
There was a problem hiding this comment.
I'm OK with this project not having any runtime tests, but I'm just thinking about if this isn't covered by anything in WPT, then it's good to improve WPT coverage.
There was a problem hiding this comment.
I'll have a look around. The URLSearchParams constructor takes a union with a sequence type, maybe we can add a test there. Or perhaps the IDL harness tests already cover this case?
domenic
left a comment
domenic
left a comment
There was a problem hiding this comment.
Can you add a test case (just a snapshot test)?
| ${name}[Symbol.asyncIterator] !== undefined || | ||
| ${name}[Symbol.iterator] !== undefined |
There was a problem hiding this comment.
I think we should fix this, and ideally have some tests.
domenic
left a comment
domenic
left a comment
There was a problem hiding this comment.
LGTM with one question; after hearing back on that, I will merge and release unless you say otherwise.
2 participants
This PR adds support for async sequence types. It was first introduced as
async iterable<T>in whatwg/webidl#1397, and later renamed toasync_sequence<T>in whatwg/webidl#1500. (Note that this is different from asynchronously iterable declarations, which are already supported since #224.)The implementation is largely based on the Streams reference implementation (whatwg/streams#1083). Once this PR lands, we will update Streams to use webidl2js's version instead.