evaera / roblox-lua-promise

Promise implementation for Roblox
https://eryn.io/roblox-lua-promise/
MIT License
275 stars 74 forks source link

Expand Promise.all() signature to accept ...Promise<T> #71

Closed matthargett closed 2 years ago

evaera commented 2 years ago

Could you talk about your motivations for this PR? I'm not sure that expanding the API surface is worth it for convenience alone, and I think we do get some value out of the usage of Promise.all being consistent.

The counterparts in JavaScript, native Promise.all and that of bluebird, don't offer this as an option, they only accept an array (or iterable object).

I think simply wrapping curly braces around the parameter list (e.g. Promise.all({p1, p2, p3})) is easy enough as an alternative to this.

This API currently doesn't have a nice error message if you were to use it in the way that the proposed change would allow (I think it would still error with the "non-promise in list" error), but I think it would be reasonable to accept a dummy argument in the second position of Promise.all, and if it exists, emit a more helpful error instructing the user to wrap it in curly braces.

Let me know what you think.

evaera commented 2 years ago

Closing as stale.