Use Promise library provided in config in Deferred

coopernurse / node-pool

Generic resource pooling for node.js

2.38k stars 259 forks source link

Use Promise library provided in config in Deferred #154

Closed eide closed 7 years ago

eide commented 7 years ago

Deferred always uses native Promises, even when a custom Promise library is supplied in the configuration.

This means the promise returned from acquire() is always native.

sandfox commented 7 years ago

thanks for spotting this! Couple of things/questions:

I think I'd like the deferred to optionally allow passing a Promise impl rather than forcing it to always be passed in. thoughts?
Probably a thing to fixed later and a bit out of scope, but I wonder if the Pool should use some kind of factory for creating the the resourceLoan / resourceRequest.

eide commented 7 years ago

With "optionally passing a Promise impl" do you mean that the "Promise impl" argument should have a default value? Ie: constructor (PromiseImpl = Promise)?

I can do that, but I think having a default value makes it easier to forget to specify it when you need to :)

sandfox commented 7 years ago

Pretty much (although I have irrational dislike for default args in function sigs..) , although I do see you point and it's not as if this is meant to be some general purpose used by the world impl. I'll merge this and then if it's still bothering me in a few months do something about it :-)

sandfox commented 7 years ago

marged and shipped and live as v3.0.1!