Implement Promise All Settled #872
Conversation
komyg
force-pushed
the
feature/promise.all_settled
branch
from
6271720 to
16237ab
Compare
komyg
force-pushed
the
feature/promise.all_settled
branch
from
a68dc67 to
fae782c
Compare
| }; | ||
|
|
||
| #[derive(Debug, Clone, Copy)] | ||
| pub enum PromiseGroupType { |
|
|
||
| let data = promise_all.get_mut(agent); | ||
|
|
||
| // i. Set remainingElementsCount.[[Value]] to remainingElementsCount.[[Value]] - 1. |
There was a problem hiding this comment.
suggestion: I think we can probably factor out this part and make it a method of the PromiseGroupRecord: take &mut self, reduce remaining_elements_count by 1, and return (Array, Option<Promise>) with the Promise only getting returned if remaining_elements_count == 0. Now the caller can do an assignment into the Array with their value and then resolve the Promise if they received one.
Also note: this way the code doesn't need to first read the result array from the PromiseGroupRecord, then write into that array, and then go back to reading from the PromiseGroupRecord to check the remaining elements count and promise. It can do all of those reads at the same time.
| capability.reject(agent, value.unbind(), gc); | ||
| } | ||
|
|
||
| pub(crate) fn on_promise_all_settled_fulfilled( |
There was a problem hiding this comment.
suggestion: allSettled doesn't deserve two functions that do nearly the same thing: just have one function with a boolean parameter that controls what kind of obj gets created.
... Or, it could actually just be one function shared between all's resolve, and allSettled's resolve and reject both: take an enum that defines if you want to store only the value, or the "fulfilled" value object, or the "rejected" value object.
| promise: promise.get(agent), | ||
| }) | ||
| .scope(agent, gc.nogc()); | ||
| let promise_group_reference = match promise_group_type { |
There was a problem hiding this comment.
nitpick (blocking): No reason to match here, just pass promise_group_type into the PromiseGroupRecord creation.
| promise_all.on_promise_rejected(agent, argument.unbind(), gc.nogc()); | ||
| } | ||
| let record = promise_group.get(agent); | ||
| match record.promise_group_type { |
There was a problem hiding this comment.
suggestion: I'd move this inside of a general promise_group.settle(agent, index, argument, reaction_type, gc) handler. That then effectively becomes the one function shared between allSettled and all I suggested above. The only difference is that all's reject side is there as well, and it's just on its own path.
2 participants
No description provided.