kriskowal / q

A promise library for JavaScript
MIT License
14.93k stars 1.2k forks source link

Remove Q.fulfill #205

Closed domenic closed 11 years ago

domenic commented 11 years ago

We don't like it anymore, I think. It should die before 0.9 goes out the door.

kriskowal commented 11 years ago

I’m going to leave it around in the off-chance it saves some-one’s bacon to be able to mark a value with a then method as a non-promise.

domenic commented 11 years ago

Code sample where it helps please, for historical purposes if nothing else?

kriskowal commented 11 years ago

Supposing you were using: https://github.com/polotek/procstreams

A procstream has a then method, but is not a promise.

function getStream() {
    return prepareToGetStream()
    .then(function () {
        return Q.fulfill(procstream("cat README.md"));
    })
}
domenic commented 11 years ago

Huh, for some reason I thought that wouldn't work, but somehow it does. I think this means I need to revise https://github.com/promises-aplus/promises-spec/pull/76 to take into account such "marked-as-non-thenable thenables."

kriskowal commented 11 years ago

At the end of the day, if this is the only valid use-case, naming it Q.notAPromise(value) might steer folks away from misery.

domenic commented 11 years ago

I like that. In which case, I'd say that deferred.fulfill(x) should die, replaced by deferred.resolve(Q.notAPromise(x)).

kriskowal commented 11 years ago

Yeah. Revising the name again: Q.nonPromise(x).

rkatic commented 11 years ago

Can I ask what are the reasons for "heating" deferred.fulfill? Normalli I am for reducing API-s, but this one seams useful with no additional implementational costs..

domenic commented 11 years ago

My reasoning would be that things you do very uncommonly should have slightly awkard APIs that express exactly what you're trying to do. deferred.resolve(Q.notAPromise(x)) is more explicit than deferred.fulfill(x) and emphasizes that what you want to do is almost always deferred.resolve(x), except maybe sometimes you're dealing with non-promisey thenables and in that case you need to use the explicit Q.notAPromise to mark them as such.