-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Implement the Iterator
trait for JS iterators
#797
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments below about iterables that throw
crates/js-sys/src/lib.rs
Outdated
let val = Iterator::next(&*self) | ||
.expect("JS object didn't obey iterator protocol"); | ||
if val.done() { | ||
None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crates/js-sys/src/lib.rs
Outdated
|
||
fn next(&mut self) -> Option<JsValue> { | ||
let val = Iterator::next(&*self) | ||
.expect("JS object didn't obey iterator protocol"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting!
I don't think we could auto-fuse though unless we start setting a JS property on the iterator (which seems icky) or have a wrapper in Rust. I'd be ok returning a Result<JsValue, JsValue>
but that would make fusing difficult for sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the fusing is important for combinators and .collect()
to work correctly.
Perhaps we should implement IntoIterator
for &'a js_sys::Iterator
witha custom type that has a bool
for fusing?
This commit implements the standard library's `Iterator` trait for the `js_sys::Iterator` type, using the iterator protocol described on [MDN] Closes rustwasm#777 [MDN]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols
3138c14
to
f2608d3
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice -- thanks!
This commit implements the standard library's
Iterator
trait for thejs_sys::Iterator
type, using the iterator protocol described on MDNCloses #777