Closed Bug 956197 Opened 6 years ago Closed 6 years ago

DOM Promise should support iterables

Categories

(Core :: DOM: Core & HTML, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: nsm, Assigned: nsm)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

The promise spec specifies iterables, but Spidermonkey currently does not have an API for dealing with iterables, presumably because the iterable spec is still in flux? FWIW Chromium doesn't support iterables either.
Bug 952873 is asking for the API you need here, right?
Depends on: 952873
Though whether the API added there will help you is hard to tell without knowing what exactly the spec says.  Can you please link to the relevant spec bit?
Yes, that should be sufficient. https://github.com/domenic/promises-unwrapping#promiserace--iterable-
similarly for Promise.all() above it.
Whiteboard: [mentor=nsm.nikhil@gmail.com]
Whiteboard: [mentor=nsm.nikhil@gmail.com] → [mentor=nsm.nikhil@gmail.com][lang=c++]
Blocks: 885333
With bug 952890 fixed, is there anything to do here?
Flags: needinfo?(nsm.nikhil)
No changes required, just adding tests.
Attachment #8398128 - Flags: review?(bzbarsky)
Assignee: nobody → nsm.nikhil
Flags: needinfo?(nsm.nikhil)
Whiteboard: [mentor=nsm.nikhil@gmail.com][lang=c++]
Comment on attachment 8398128 [details] [diff] [review]
Tests to check Promises can accept iterators.

>+      setTimeout(resolve.bind(undefined, 20), 20);

This could just be setTimeout(resolve, 20, 20), but either way.

>+    is(winner, 10, "Winner should be the one that finished earlier.");

This can fail, and then the sheriffs will hate you.  ;)  For example, say the process gets suspended for 11ms between the first setTimeout call and the second one.  Then the 20 will be scheduled before the 10.

You need to reverse the order of those calls to setTimeout.

r=me with that fixed.
Attachment #8398128 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/25d0630ab47d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.