mudge / pacta

An algebraic implementation of ECMAScript 2015 and Promises/A+ Promises in JavaScript for as many browsers and Node.js versions as possible
https://npmjs.org/package/pacta
BSD 3-Clause "New" or "Revised" License
71 stars 6 forks source link

Functor broken #1

Closed puffnfresh closed 11 years ago

puffnfresh commented 11 years ago

Breaks for this example:

p.concat(p2).concat(p3).map(function(a) { return a; });
mudge commented 11 years ago

This is a tricky one: at the moment, promises with multiple values will yield them as separate arguments to map instead of together as an array. This was purely because there is no elegant destructuring syntax in node yet. However, you're right that this breaks identity as defined.

I can change the behaviour of map to concatenate promise values into an array that is returned as a single argument during map.

puffnfresh commented 11 years ago

You shouldn't make map always take an array. That's not a restriction that Fantasy Land imposes. Fantasy Land's is much more relaxed:

map :: f a -> (a -> b) -> f b

(i.e. you should be able to pass any function that takes an a and returns a b)

The best option I can see is making map run f over each of the resolved values.

mudge commented 11 years ago

I've been experimenting with this a little more; an issue is what it means to concat two Promises. At the moment, I have it like so:

concat :: Promise a -> Promise b -> Promise [a b]

But this means that, strictly, speaking, concating several promises in a row results in:

p.concat(p2) //=> Promise [a b]
p.concat(p2).concat(p3) //=> Promise [[a b] c]
p.concat(p2).concat(p3).concat(p4) //=> Promise [[[a b] c] d]

While consistent, my original use case was to have a way to combine multiple promises into one that only yields when all values are resolved. You would then need to make those values available and this interface was very nice to use:

promisedHttpRequest.concat(anotherPromisedHttpRequest).map(function (response, anotherResponse) {
    // Do something with the two response bodies.
});

As you say, this violates the Functor interface so I wanted to change it to be compliant:

promisedHttpRequest.concat(anotherPromisedHttpRequest).map(function (responses) {
    var response = responses[0],
        anotherResponse = responses[1];

    // Do something with the two response bodies.
});

However, the current definition of concat makes this surprising beyond two promises:

promisedHttpRequest.concat(anotherPromisedHttpRequest).concat(yetAnotherPromisedHttpRequest).map(function (responses) {
    var response = responses[0][0],
        anotherResponse = responses[0][1],
        yetAnotherResponse = response[1];

    // Do something with the two response bodies.
});

Perhaps defining the result of concat as strictly Promise [a b] is wrong as it would be nice have (Promise a) concat (Promise b) concat (Promise c) -> Promise a b c or perhaps I am abusing map in my above examples?

puffnfresh commented 11 years ago

Ah, concat is broken. It should like this:

concat :: Promise a -> Promise a -> Promise a
mudge commented 11 years ago

So it's only possible to concat things that can themselves be concatenated (e.g. arrays, etc.)?

puffnfresh commented 11 years ago

@mudge that's one solution, or you could say that Promises always represent multiple values (of the same type) and then concat them internally.

So Promise could be something like an async Array, I guess.

mudge commented 11 years ago

I like the idea of Promises as a kind of Eventual type: they might not have a value now but they will eventually. If I define concat in terms of concatenating contents (so only supporting promises of semigroups) then I could build the interface I wanted with map on top of those primitives.

mudge commented 11 years ago

I've just pushed some changes to the interface to bring it back in line with the specification, please feel free to review and let me know if there are any issues.

I added a test explicitly for the case mentioned (identity of a semigroup): https://github.com/mudge/pacta/blob/master/test/pacta_test.js#L131-L142