add allThen function #3165
add allThen function #3165
Conversation
There was a problem hiding this comment.
Although this is a clearly useful function (and, I might add, a nicely done pull request), I'm very hesitant to add this.
The Ramda team has repeatedly expressed its reservation about Promises. Most of the reasons are captured in Broken Promises -- but the biggest one is that they are so close to lawful abstract types such as Functor and Chain, but they blow it, which means that you have to be very careful when trying to use them with generic code.
So Ramda has managed to reduce its direct support for Promises to only the two functions andThen and otherwise. It's not that Promises don't work with Ramda code, just that Ramda does not go out of its way to work with them. This makes me really hesitant to increase that support.
So although this review suggests that changes are needed, I would suggest holding off on them until and unless you first convince us that it's worth offering any additional Promise support, especially when this is trivial for users to write as
const allThen = (fn) => (ps) => Promise. all (ps) .then (fn)
source/allThen.js
Outdated
| * R.of, | ||
| * R.ap([p1, p2]), | ||
| * R.allThen(R.join('|')) // => 'test1|test2' | ||
| * ); |
There was a problem hiding this comment.
Although we're not entirely consistent with this, I think it's better if our examples rarely use any Ramda functions outside the one being demonstrated. While join is probably clear to most readers, both of and ap might cause confusion.
Perhaps better would be something like this:
R .allThen (xs => xs .join ('|')) ([Promise .resolve ('foo'), Promise .resolve ('bar')])
.then (console .log) //~> "foo|bar"There was a problem hiding this comment.
Thanks, it's really more concise.
| var allThen = _curry2(function allThen(f, ps) { | ||
| if (Promise == null || !_isFunction(Promise.all)) { | ||
| throw new TypeError('Promise is not defined'); | ||
| } |
There was a problem hiding this comment.
I'll be glad when we can stop doing these tests!
|
|
||
| if (!(_isString(ps) || _isArrayLike(ps) || _isIterator(ps))) { | ||
| ps = _of(ps); | ||
| } |
There was a problem hiding this comment.
Ramda usually strives for simplicity. I don't think allowing for either scalars or arrays really fits.
There was a problem hiding this comment.
I know this will cause confusion, but It's more convenient, determine whether to use andThen.
I will remove it.
|
Do you want to argue in favor of this? Or can we just go ahead and close it? |
use Promise.all after ap, raise error #3157
add allThen function, simplify
Promise.all.bind(Promise).