-
Notifications
You must be signed in to change notification settings - Fork 24
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
Fix "wait for all" not resolving when given empty list of promises #58
Fix "wait for all" not resolving when given empty list of promises #58
Conversation
I think there could be a problem here. If I'm reading it correctly, we're calling successSteps synchronously when the list is empty, but asynchronously in all other cases. Maybe it needs to be something like "queue a microtask to run successSteps"? |
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.
Thank you for taking this on! Agreed we should queue a microtask.
Would this work?
I feel like it might be confusing whether "and return" is still part of the outer algorithm, or whether it's part of the steps inside the microtask. Would you prefer I change it to this instead?
|
Nested steps seems slightly better; thanks! I also need to investigate the build failures, hrm. |
@@ -269,6 +272,9 @@ To <dfn id="waiting-for-all" export lt="wait for all|waiting for all">wait for a | |||
1. Let |index| be 0. | |||
1. Let |total| be |promises|'s [=list/size=]. | |||
1. Let |result| be a <a>list</a> containing |total| null values. | |||
1. If |total| is 0, then: | |||
1. <a>Queue a microtask</a> to perform |successSteps| given |result|. |
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 hope I'm doing this autolinking right! 😅
I still don't know when to use "dfn autolinks" ([= ... =]
) and when to use manual <a>
elements though... 🤷♂
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.
They're equivalent. Basically [=...=]
is newer so some older specs like this one still use <a>
. (And also sometimes newer specs written by people with older habits.)
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.
Removed the link-defaults changes which should not be necessary.
…eventCancel and preventAbort works, a=testonly Automatic update from web-platform-tests Streams: verify that aborting a pipe with both preventCancel and preventAbort works See whatwg/streams#1004 and w3ctag/promises-guide#58. -- wpt-commits: 0da3476dcd5fd3148041d090d2330cf8d412d7f9 wpt-pr: 17816
…eventCancel and preventAbort works, a=testonly Automatic update from web-platform-tests Streams: verify that aborting a pipe with both preventCancel and preventAbort works See whatwg/streams#1004 and w3ctag/promises-guide#58. -- wpt-commits: 0da3476dcd5fd3148041d090d2330cf8d412d7f9 wpt-pr: 17816
When both the preventCancel and preventAbort flags are set, the abortAlgorithm passes an empty list of actions to "wait for all". This helper should immediately resolve the returned promise when no promises are given, but instead the promise remained pending forever. This fixes the reference implementation of the "wait for all" helper to resolve correctly when given an empty list of promises. This matches the spec change in w3ctag/promises-guide#58. Originally reported in #1004 (comment).
…eventCancel and preventAbort works, a=testonly Automatic update from web-platform-tests Streams: verify that aborting a pipe with both preventCancel and preventAbort works See whatwg/streams#1004 and w3ctag/promises-guide#58. -- wpt-commits: 0da3476dcd5fd3148041d090d2330cf8d412d7f9 wpt-pr: 17816 UltraBlame original commit: 40dd37dfbbfbc0659c81c2a386f5fe966b3c8203
…eventCancel and preventAbort works, a=testonly Automatic update from web-platform-tests Streams: verify that aborting a pipe with both preventCancel and preventAbort works See whatwg/streams#1004 and w3ctag/promises-guide#58. -- wpt-commits: 0da3476dcd5fd3148041d090d2330cf8d412d7f9 wpt-pr: 17816 UltraBlame original commit: 40dd37dfbbfbc0659c81c2a386f5fe966b3c8203
…eventCancel and preventAbort works, a=testonly Automatic update from web-platform-tests Streams: verify that aborting a pipe with both preventCancel and preventAbort works See whatwg/streams#1004 and w3ctag/promises-guide#58. -- wpt-commits: 0da3476dcd5fd3148041d090d2330cf8d412d7f9 wpt-pr: 17816 UltraBlame original commit: 40dd37dfbbfbc0659c81c2a386f5fe966b3c8203
From my comment on whatwg/streams#1004: