Use mongoose PromiseProvider to enable ES2015 Promise compatibility

[gavinaiken]

When using mongoose-fill and a promise provider other than the current default of mpromise in mongoose (as per http://mongoosejs.com/docs/promises.html#plugging-in-your-own-promises-library ), you receive an error "Can't use ES6 promise with mpromise style constructor" when trying to fill a query.

This change modifies mongoose-fill to use the new mongoose PromiseProvider and ES6 style promises so mongoose-fill is compatible with using native ES6 node promises.

[wclr]

from which version does mongoose provides mongoose.PromiseProvider? Won't this make it non-compatible with some prev versions?

[gavinaiken]

Yes, but it doesn't bump up the requirement much further than you already had it - PromiseProvider is available from 4.1.0 https://github.com/Automattic/mongoose/commit/03e874eb and you already had it requiring 4.0.5 or above. I changed that to 4.1.2 (version I tested with) but 4.1.0 in the package.json would probably be ok.

[gavinaiken]

Just tested with 4.1.0 and it works, so I could amend the PR to drop the dependency requirement down to that?

[wclr]

But I believe it should support version before 4.1, and mongoose version dep in the package is in devDependencies, could you add the support both variants? If PromiseProvider is available - use it if not, use mpromise.

And it would be nice to have a way to test both.

[gavinaiken]

Oh yes, good point about the dep being in devDependencies. Maybe it should be a peer dependency instead?

As to supporting mpromise - it has quite a different API than standard promises, so we would have to copy over quite a few chunks of mongoose's promise wrapper to do so, or have a very large if/then conditional chunk just doing your code or the block I changed depending on the presence of PromiseProvider. Neither sounds ideal to me?

Could bump the version to say 2.0 and say that for those requiring support for mongoose < 4.1.0 they should use 1.x and otherwise use 2.x?

[wclr]

Maybe it should be a peer dependency instead?

I should probably in peer be but not instead, but also )

can we use just native node Promise ?

[gavinaiken]

Yes, using native promises would work. But that would limit you to node 0.12 or above. Another possibility is to use the bluebird library to provide the promises - one more dependency, but works across many versions. But both of these solutions have a problem, which is that it changes the type of promise returned by a mongoose query, depending on whether you use .fill() in the query chain or not. The advantage to using the mongoose promise provider is that the user then still gets whatever type of promise mongoose is returning, whether or not they use .fill. So by default (in current versions of mongoose) they would get an mpromise, in mongoose 5.x they would get a native promise etc, and if mongoose's promise is overridden (eg with mongoose.Promise = global.Promise;) then they get what ever the override is.

So I'd argue that using promise provider is better than supporting older versions of mongoose - but it's your package! :)

[wclr]

have added it in 1.3.0 https://github.com/whitecolor/mongoose-fill/blob/91383ca27cff1ed199cecae591ff830902b8955b/index.js#L170 check it.

[wclr]

It is actually supports now only node v > 4 as I used some of ES6 code there, all this stuff need to be rewritten)

[gavinaiken]

Ah, nice solution. However you have a bug here: https://github.com/whitecolor/mongoose-fill/blob/91383ca27cff1ed199cecae591ff830902b8955b/index.js#L167 - should be var promise, onResolve, resolve, reject or you still get Error: Can't use ES6 promise with mpromise style constructor when using native promises in mongoose.

[wclr]

Yes I've seen this, that I should not assign it there, but are you sure that it doesn't work?

I just then don't get what code at all causes your error?

[gavinaiken]

Yes, just tested it and got the Error: Can't use ES6 promise with mpromise style constructor error, because in my project I have mongoose's promise overridden as described above, so when you do new mongoose.Promise(); it throws that error. (Only if you are using mongoose >= 4.1.0 and have a line like mongoose.Promise = global.Promise;)

[gavinaiken]

Here's the full stack btw:

Error: Can't use ES6 promise with mpromise style constructor
      at new ES6Promise (/Users/gavin/repos/node-core/mongo/node_modules/mongoose/lib/ES6Promise.js:19:9)
      at Query.mongoose.Query.exec (/Users/gavin/repos/node-core/mongo/node_modules/mongoose-fill/index.js:167:17)
      at Context.<anonymous> (/Users/gavin/repos/node-core/mongo/test/event-detail.js:175:18)

I can make a PR to fix line 167 if you want? Meanwhile I will close this PR since there is no need for it once you fix the bug there. Thanks by the way!

[wclr]

Ok, I fixed it.

[gavinaiken]

Works great now, thanks!