mojolicious / mojo

:sparkles: Mojolicious - Perl real-time web framework
https://mojolicious.org
Artistic License 2.0
2.66k stars 576 forks source link

RFC: resolve aggregated Promises with Collections #2126

Closed guest20 closed 7 months ago

guest20 commented 8 months ago

Summary

pass Mojo::Collections to ->then callbacks when they receive the results of multiple Promises

Motivation

Being able to call ->first instead of using ->[0] on the array-ref I receive:

my $mojo = get_p('https://mojolicious.org');
my $cpan = get_p('https://metacpan.org');
Mojo::Promise->all($mojo, $cpan)->then(sub ($mojo, $cpan) {
  say $mojo->first->res->code;   # <-- Mojo::Collection::first
  say $cpan->[0]->res->code;     # <-- Just like all the code on earth has now
})->catch(sub ($err) {
  warn "Something went wrong: $err";
})->wait;

Other considerations

kraih commented 8 months ago

I'm afraid PRs without tests cannot be considered.

jberger commented 8 months ago

I do think we should do some work in this area, however, I would rather deprecate promises returning multiple values, opting instead to require promises returning only one value, which could be an array ref. Returning a collection of values would firm up the choice to allow multiple.

The reason is async/await. Consider the scalar context use currently:

# current
my $value = await get_p();
# collection
my $value = (await get_p())->first;

that seems like a clear loss in functionality. In list-ish context it does make some more sense I suppose but personally I'd rather just not deal with it by requiring a single return

jberger commented 8 months ago

Follow up: rereading, I see that your PR only affects each individual response from ::all but not the overall return from a promise, including the promise returned by ::all itself. I suppose that's less obtrusive than my example above, however, it still would set up that expectation that promises can return multiple values. If we wanted to do that, I'd probably want to be consistent with it and get a collection as the result of the all as well, that way I could easily call ->flatten on the whole thing, which is what I would usually do. Still I'd rather just require only one return and ::all could just return a naturally flat arrayref

jhthorsen commented 7 months ago

I haven't benchmarked this, but I expect a collection would slow down promises even more, so unless I'm mistaken I'm not very positive to this change.

kraih commented 7 months ago

Looks like no more updates are being made to the proposal. As it stands this seems like a pretty clear rejection.