Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Q.fulfill #205

Closed
domenic opened this issue Feb 10, 2013 · 9 comments
Closed

Remove Q.fulfill #205

domenic opened this issue Feb 10, 2013 · 9 comments
Assignees
Milestone

Comments

@domenic
Copy link
Collaborator

domenic commented Feb 10, 2013

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

@kriskowal
Copy link
Owner

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
Copy link
Collaborator Author

domenic commented Feb 28, 2013

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

@kriskowal
Copy link
Owner

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
Copy link
Collaborator Author

domenic commented Feb 28, 2013

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

@kriskowal
Copy link
Owner

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
Copy link
Collaborator Author

domenic commented Feb 28, 2013

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

@kriskowal
Copy link
Owner

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

@rkatic
Copy link
Collaborator

rkatic commented Feb 28, 2013

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
Copy link
Collaborator Author

domenic commented Feb 28, 2013

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants