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

Promise wrapper #176

Closed
gergoerdosi opened this issue Jan 9, 2016 · 9 comments
Closed

Promise wrapper #176

gergoerdosi opened this issue Jan 9, 2016 · 9 comments
Assignees
Labels
feature New functionality or improvement

Comments

@gergoerdosi
Copy link
Contributor

What do you think about moving the promise wrapper (https://github.com/hapijs/hapi/blob/master/lib/promises.js) from hapi to hoek? Other modules could then easily reuse this code. For example: hapijs/glue#54.

@devinivy
Copy link
Member

devinivy commented Jan 9, 2016

I like that idea.

@csrl
Copy link

csrl commented Jan 9, 2016

The idea is good, but the specific implementation expects the callback to always have a single parameter which is determined to be either the result or an error depending on whether it is an Error instance or not. Most callback handlers have the form of => (err, result), so that should be taken into consideration.

@hueniverse
Copy link
Contributor

I'm open to it if we keep it as simple.

@nlf
Copy link
Member

nlf commented Apr 25, 2016

i'll get this in the next release, i think i know how i want to do it though. the main question i have is do we anticipate people only wrapping things in promises that accept two parameters to the callback?

what about things like wreck.get() which pass 3 parameters to the callback? should the promise resolve with an array containing the last two params?

@catalint
Copy link
Contributor

I've just now seen this discussion ... ignore pr #187 until we settle on this

@catalint
Copy link
Contributor

catalint commented Apr 26, 2016

propsed something that could work for multiple params, you can take a look at #188

it's quite general, maybe we should by default expect a callback to look like this callback(err,value) instead of the current callback(err|value) ?

@catalint catalint mentioned this issue Apr 26, 2016
@catalint
Copy link
Contributor

hi guys, could I help more to get this to the finish line?

@nlf nlf self-assigned this Aug 26, 2016
@nlf
Copy link
Member

nlf commented Aug 26, 2016

closing this, IMO the use cases here are too varied to get it right with one method

@nlf nlf closed this as completed Aug 26, 2016
@Marsup Marsup added feature New functionality or improvement and removed request labels Sep 21, 2019
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

7 participants