Implement emscripten_promise_any in promise.h
#19153
Conversation
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
tlively
force-pushed
the
promise-any
branch
2 times, most recently
from
2354985 to
4ae1cc5
Compare
|
|
||
| typedef struct promise_any_state { | ||
| size_t size; | ||
| em_promise_t in[3]; |
There was a problem hiding this comment.
These are the (up to) 3 input promises. They're not actually needed after the initial call to emscripten_promise_any, but it's convenient to bundle all the input, output, and expected state together in a single struct.
There was a problem hiding this comment.
I'm confused, I don't see any mention of a limit of 3 on input promises in promise.h? The methods all take a num_promises without a mention of a limit in the comments.
There was a problem hiding this comment.
Right, this is just used in this test file, which arbitrarily chooses to call the API with three promises.
There was a problem hiding this comment.
Oh ok, sorry, that's what I was missing...
There was a problem hiding this comment.
Yeah, this C API is a little funky because 100% of the implementation is in JS. There's no .c implementation file for the API at all.
Similar to `emscripten_promise_all` and `emscripten_promise_all_settled`, this function propagates the first of its input promises to be fulfilled or is rejected with a list of reasons if its inputs are rejected.
|
The problem here is that the Node we ship with emsdk (and use on CI) does not include |
By completely coincidence I just updated that version: emscripten-core/emsdk#829 !!! However, I guess you should put some kind of node version check around your use of |
| @@ -221,6 +221,9 @@ mergeInto(LibraryManager.library, { | |||
| var promises = idsToPromises(idBuf, size); | |||
| #if RUNTIME_DEBUG | |||
| dbg('emscripten_promise_any: ' + promises); | |||
| #endif | |||
| #if ASSERTIONS | |||
| assert(typeof Promise.any !== 'undefined', "Promise.any does not exist"); | |||
There was a problem hiding this comment.
We could do something more like this.. but its probably not worth it?:
#if ENV_MAY_BE_NODE
if (ENV_IS_NODE && nodeIsOld) {
abort("helpful message")
}
#endif
There was a problem hiding this comment.
Yeah, just having this assertion seems more general. For example it would work in old browsers that don't support Promise.any as well. I'd be happy to make the assertion message more useful, but I think the current message at least delivers the most important piece of information.
|
This caused the auto-roller to fail: https://chromium-review.googlesource.com/c/emscripten-releases/+/4420909 I think we should revisit and find a way to make this test fail to link with bumping |
|
I'm working on a fix |
Similar to
emscripten_promise_allandemscripten_promise_all_settled, thisfunction propagates the first of its input promises to be fulfilled or is
rejected with a list of reasons if its inputs are rejected.