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

use Q.js #1699

Closed
aheckmann opened this issue Sep 16, 2013 · 52 comments
Closed

use Q.js #1699

aheckmann opened this issue Sep 16, 2013 · 52 comments

Comments

@aheckmann
Copy link
Collaborator

No description provided.

@refack
Copy link
Contributor

refack commented Oct 5, 2013

How do you see this? Instead of mpromise?
Feel free to assign this to me.

@aheckmann
Copy link
Collaborator Author

yep but v4

On Saturday, October 5, 2013, Refael Ackermann wrote:

How do you see this? Instead of mpromise?
Feel free to assign this to me.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1699#issuecomment-25744700
.

Aaron
@aaronheckmann https://twitter.com/#!/aaronheckmann
soundcloud.com/ajhecky
github.com/aheckmann

@refack
Copy link
Contributor

refack commented Oct 13, 2013

After all I've done for #1732 this seems like the next logical step :)

@refack
Copy link
Contributor

refack commented Oct 13, 2013

I might even find a way to use Q instead of hooks-js

@refack
Copy link
Contributor

refack commented Oct 20, 2013

@aheckmann, I need you guidance.
I need a more comprehensive promise library for #1732 (and aiming for #1446 on the way).
I could:

  • npm install when --save - My favorite
  • npm install q --save
  • Implement into mpromise - Also interesting for me

@pirelenito
Copy link

Hi guys, I love the work you've been doing on adding promise support throughout Mongoose!

I've recently understood and started using Promises and it has been just a way better experience than plain simple callbacks.

Can I ask you what is the motivation behind choosing Q.js? I've been using RSVP and I've loved it for its simplicity. (I am sorry if that is a dumb question)

I would also love to help you out in this transition, I just need some better understanding of the Mongoose codebase.

Cheers,

@aheckmann
Copy link
Collaborator Author

we ideally should use any promise library we want and allow users to specify it as well. As long as it conforms to PromisesA+ I don't really care.

@gabzim
Copy link

gabzim commented Mar 12, 2014

Q is the most starred promise library on GitHub and a rich implementation of the PromisesA+ plus the fact that AngularJS uses Q has people using Q in the frontend. I think the MEAN stack would rock if mongoose used Q as well. Using the same promise library for the frontend and the backend will make the difference between both programming models much shorter. +1 on this issue.

@meaku
Copy link

meaku commented Mar 12, 2014

+1 for when.js

@mgcrea
Copy link
Contributor

mgcrea commented Mar 15, 2014

I would strongly advise to pick bluebird over q.

I've been doing NodeJS for a couple of years, and switching from q to the very well-crafted bluebird has been a really pleasant experience. The API is far superior and the performance is unmatched.

Regarding performance, have a look at this article.

But you said promises are slow!
Yes, I know I wrote that. But I was wrong. A month after I wrote the giant comparison of async patterns, Petka Antonov wrote Bluebird. Its a wicked fast promise library, and here are the charts to prove it.

file time(ms) memory(MB)
callbacks-original.js 316 34.97
callbacks-flattened.js 335 35.10
callbacks-catcher.js 355 30.20
promises-bluebird-generator.js 364 41.89
dst-streamline.js 441 46.91
callbacks-deferred-queue.js 455 38.10
callbacks-generator-suspend.js 466 45.20
promises-bluebird.js 512 57.45
thunks-generator-gens.js 517 40.29
thunks-generator-co.js 707 47.95
promises-compose-bluebird.js 710 73.11
callbacks-generator-genny.js 801 67.67
callbacks-async-waterfall.js 989 89.97
promises-bluebird-spawn.js 1227 66.98
promises-kew.js 1578 105.14
dst-stratifiedjs-compiled.js 2341 148.24
rx.js 2369 266.59
promises-when.js 7950 240.11
promises-q-generator.js 21828 702.93
promises-q.js 28262 712.93

@tsouza
Copy link

tsouza commented Apr 3, 2014

Why not when.js?

@bingdian
Copy link

+1 for when.js

@bernhardw
Copy link

Actually, we ideally should use the native promise implementation and a simple shim for Node versions without support.

According to a comment by tjfontaine from 6 days ago, native promises are in master. So I think they will be available in the next release.

Mongoose doesn't seem to be needing any of the fancy features most promise libraries give you and should be perfectly fine with the native methods resolve, reject, all, then, catch, and race (although that last one may be from an older draft, because the current draft doesn't mention it).

@dalcib
Copy link

dalcib commented May 5, 2014

+1 for native promises

@refack
Copy link
Contributor

refack commented May 7, 2014

I just discovered native promises in node 0.11.13 (behind --es-staging), and I'm playing with making mpromise into a shim,
CC: @bernhardw @dalcib @aheckmann

@bernhardw
Copy link

@refack Greatly appreciated :) I'm not sure what --es-staging is, but for native promises in 0.11.13 you shouldn't need any flag anymore.

@refack
Copy link
Contributor

refack commented May 7, 2014

@bernhardw, thanks for the tip. Very progressive of the V8 team (adding symbols to the global namespace). I think I hear the sound of KLOCs creaking and breaking :)
P.S. https://codereview.chromium.org/165723008

@benjamingr
Copy link
Contributor

Native promises are considerably slower than Bluebird promises. Heck, with newest Node, Bluebird with generators is now faster than callbacks.

@refack
Copy link
Contributor

refack commented May 7, 2014

P.P.S - V8 flags and their meanings

@tony
Copy link

tony commented May 8, 2014

Is https://github.com/domenic/promises-unwrapping an example of native promises spec?

I really like Q and bluebird, but with native promises are around the corner, it may be beneficial to stick to native's conventions but offer a polyfill for older node versions.

Regarding polyfill libraries, what are our options? Is https://github.com/jakearchibald/es6-promise a good candidate? It seems more active than mpromise and doesn't require additional dependencies.

@tony
Copy link

tony commented May 8, 2014

On top of normal es6-promise, rsvp seems to have some cool error bubbling features. https://github.com/tildeio/rsvp.js#error-handling. One of the major pain points with promises (native or not) is tracking down errors. In fact, if I had an adequate way around that, I'd be open to settle for something that wasn't fully native / following barebones es6 promise implementation.

@tony
Copy link

tony commented May 12, 2014

Here is another link I think is a good read: https://github.com/spion/async-compare

http://dailyjs.com/2013/10/04/bluebird/

"One of the tools Petka mentions for verifying the quality of the project was async-compare by Gorgi Kosev. This project aims to compare async patterns based on their complexity, performance, and “debuggability”. The stack traces generated by libraries like Bluebird and Q are captured and curated, so async-compare can be used to measure the distance between the function that created the error and the error in the stack trace. This is useful because debugging asynchronous code is notoriously difficult."

Promises are still something I'm wrapping my brain around, especially the debugging aspects of it. Does the choice of library really matter when it comes to debuggability?

I find my current understanding of errors with promises is vague at best.

@mgcrea
Copy link
Contributor

mgcrea commented May 12, 2014

You might find this recent comment regarding bluebird vs. native from Petka interesting.

Would it be possible to somehow configure the Promise provider used by a mongoose instance (like passing {promise: require('bluebird')} somewhere? Would be the best of both worlds.

@petkaantonov
Copy link

btw in next bluebird version it promisifyAll has been steroid-boosted and will be able to promisify mongoose (and a lot of other modules) in one line with promisifyAll(require("mongoose"))

https://github.com/petkaantonov/bluebird/blob/2.0/API.md#promisification

@vkarpov15 vkarpov15 removed this from the 4.0 milestone Nov 10, 2014
@wclr
Copy link

wclr commented Feb 14, 2015

So are you going to migrate to bluebird?

@benjamingr
Copy link
Contributor

@vkarpov15 the answer is simple: Allow library authors to pass the promise constructor into the library (or a compatible interface) - that way anyone can use their favorite implementation with its quirks and benefits and you can stay uncoupled to any promise implementation.

@vkarpov15
Copy link
Collaborator

That would be great if promise constructors had compatible interfaces. Suppose you have:

var Promise1 = require('bluebird');
var Promise2 = require('q');
var Promise3 = require('mpromise');

For Promise1, you need to pass a function to the constructor, otherwise Bluebird will throw an error.

var p = new Promise1(function(resolve, reject) {});

For Promise2, you need to call .defer() and get the promise property:

var p = Promise2.defer().promise;

And for Promise3 you can simply use the constructor:

var p = new Promise3().

So its not quite as simple as saying mongoose.Promise = require('q');. FWIW, this is why I like mpromise: as far as I've seen, its the library with the least amount of library-specific quirks.

@petkaantonov
Copy link

For Promise1, you need to pass a function to the constructor, otherwise Bluebird will throw an error.

Because that's the standard. Q implements the same standard in Q.Promise which the user should pass as the promise constructor. Mpromise doesn't implement any construction standard at all.

@benjamingr
Copy link
Contributor

@vkarpov15 I'm with petka here, there is a standard and libraries like Q follow it (with Q.Promise) as well as other popular promise libraries (Bluebird, When, RSVP etc) and native promises. Q supports .defer because of legacy reasons (for the same reasons it'd work in bluebird)

mpromise is just not spec compliant. It's the outlier here You can easily create an adapter for it though.

@benjamingr
Copy link
Contributor

var P = require('mpromise');
function PromiseConstructor(resolver){
    var p = new P;
    if(typeof resolver !== "function") throw new TypeError("Promise resolver must get a function");
    try{
    resolver(p.fulfill.bind(p), p.reject.bind(p));
    } catch(e){
        p.reject(e);
    }
    return p; // also, resolving with multiple args is not allowed and is a spec violation...
}

@petkaantonov
Copy link

@benjamingr If resolver is not a function the exception needs to be thrown synchronously

@petkaantonov
Copy link

@whitecolor FWIW, you can promisify mongoose in one-line with promisifyAll(require("mongoose")), the methods added will also perform much faster than if mongoose constructed the promises for you through the promise constructor.

@vkarpov15
Copy link
Collaborator

mpromise does implement a standard. While I do love ES6, its still just a draft of a standard rather than a standard. However, I was wrong about Q's inconsistency, so this could be implemented in theory, so thanks for pointing that out :) It will require a lot of very messy changes because the "promise constructor with executor" syntax is nasty - one of the many reasons why I like mpromise.

@wclr
Copy link

wclr commented Feb 17, 2015

@petkaantonov thanks, will try it

@benjamingr
Copy link
Contributor

@vkarpov15 the A+ standard just specified resolution and then (which, IIRC mpromise doesn't handle gracefully when assimilating a promise for multiple values - generally multiple value promises are a bad idea).

The construction specification is here https://github.com/promises-aplus/constructor-spec , it's older. You don't even have to require it you can ask people to provide a different resolution API and write adapters as long as you let them pass their own implementation.

@petkaantonov
Copy link

mpromise does implement a standard. While I do love ES6, its still just a draft of a standard rather than a standard. However, I was wrong about Q's inconsistency, so this could be implemented in theory, so thanks for pointing that out :) It will require a lot of very messy changes because the "promise constructor with executor" syntax is nasty - one of the many reasons why I like mpromise.

I meant a construction standard, not any standard. And ES6 promises are nailed down.

Awkwardness of revealing constructor pattern should not be a big deal because it's trivial to implement the deferred api on top of it:

function deferredFactoryFor(PromiseConstructor) {
    return function() {
        var ret = {};
        ret.promise = new PromiseConstructor(function(resolve, reject) {
            ret.resolve = resolve;
            ret.reject = reject;
        });
        return ret;
    };
}

var bluebirdDefer = deferredFactoryFor(require("bluebird").Promise);
var QDefer = deferredFactoryFor(require("q").Promise);
var RSVPDefer = deferredFactoryFor(require("rsvp").Promise);
var WhenDefer = deferredFactoryFor(require("when").Promise);
var NativeDefer = deferredFactoryFor(Promise);
var mdefer = ????

@vkarpov15
Copy link
Collaborator

Yeah I agree, its easy enough to add a wrapper around it. Thanks for the insights.

@tamtakoe
Copy link

+1

@joshperry
Copy link

There seems to be a larger issue in the javascript community surrounding promise libraries and libraries that expose promisified interfaces. We've conflated the functionality of providing a future value (specified in the A+ standard), and syntactic sugar for easily working with those future values.

One big problem that this creates is that, as a library author, you basically can't use anything but the naked A+ promise in your code if you're going to allow users of your library to swap out the promise implementation.

It would be nice to take a little time and perhaps think through how these two concerns could be elegantly separated.

@petkaantonov any ideas around this? Promisify has worked well for me, but it seems like there could be a better way...

@benjamingr
Copy link
Contributor

@joshperry

There seems to be a larger issue in the javascript community surrounding promise libraries and libraries that expose promisified interfaces. We've conflated the functionality of providing a future value (specified in the A+ standard), and syntactic sugar for easily working with those future values.

I think you're looking at this the wrong way - you can expose whatever interface you'd like as long as it conforms to promises/A+ it will be easy to consume it to the outside (then only).

There really isn't a huge reason to allow any library to be used - it's enough to use a fast good library like bluebird most of the time - as long as it's not something like mpromise that swallows all errors it's all good.

@joshperry
Copy link

@benjamingr

The reason that most people want a library to support their favorite promise interface is, 99 times out of 100, because they prefer/are used to the syntactical sugar that library adds to the base A+ interface. I have seen performance used as an argument by some users to get their favorite library chosen, but it's usually just a red herring tactic to get what they really want: the interface of their favorite promise library.

This syntactical sugar is tightly bound to the A+ interface of every promises library, which makes it very difficult for a library author to expose a promise-based interface, and support all of their user's favorite promise library sugar.

There are ways around this, but the ones I've seen have been brittle (promisifyAll) or are just so messy it's not worth doing (manually wrapping the promises returned by a library).

A lot of these discussions would go away if there was an elegant convention to separate these concerns, to allow both library authors, and library consumers to use their preferred syntactical sugar. I'm not convinced that having a promise library convert a CPS interface is the best way to accomplish this.

This all reminds me of libraries written in C++ but that expose C bindings because the C++ ABI was never standardized enough to allow other languages to indiscriminately import C++ DLLs.

@benjamingr
Copy link
Contributor

There are ways around this, but the ones I've seen have been brittle (promisifyAll) or are just so messy it's not worth doing (manually wrapping the promises returned by a library).

Why is promisifyAll brittle? I've found it rock solid and working in wrapping every library I want. If there is a case where it isn't working there's always Promise.fromNode.

To be fair - I don't see the huge value in allowing swapping of the promise implementation - I'd just pick the fastest one that's easiest to work with or not at all and do wrapping - see my comments here: #2688 (comment)

@joshperry
Copy link

@benjamingr

Re: promisifyAll; it wasn't until the latest version that it could promisify mongoose properly without going through a bunch of machinations promisifying each schema and its prototype, and even then it didn't handle promisifying the query chaining at all. There are bound to be patterns in the future that promisifyAll will not work with.

You may not see the value because the promise library you use (which is actually the one I prefer as well) is currently the performance leader and has a list of excellent features, but that could change. And then what, everyone change to the then fastest library? And really, the syntactical additions do not a fast promise library make. In fact, it's very possible that native promises will outstrip even Bluebird in speed if the implementers do a good job with the native impl.

Finally, I hope that we can agree that "easiest to work with" is incredibly subjective and doesn't really provide any value in the discussion on what we're trying to accomplish here.

@vkarpov15
Copy link
Collaborator

My 2 cents - if you really care about shaving a couple microseconds of CPU time by using bluebird vs RSVP or something, you're not going to be using promises anyway. Frankly, if that level of optimization matters to you, you probably shouldn't be using node at all. Promises are all about syntactic sugar and writing code that's more durable and easier to maintain. Of course that is a function of which promises library the user has most experience with, which is why I see the value of letting people specify their favorite promises library.

@benjamingr
Copy link
Contributor

My 2 cents - if you really care about shaving a couple microseconds of CPU time by using bluebird vs RSVP or something, you're not going to be using promises anyway.

Why? There is very little if at all overhead. There are actual real world workloads that are impossible to handle with mpromise but are possible (and done with bluebird). If you have 5000 connections to the client and a flow chain of a client's request is 5 thens and 5 queries that's 50000 promises you're going to have at once. For bluebird 50K promises is a walk in the park - for Q or mpromise the process will terminate.

@mgcrea
Copy link
Contributor

mgcrea commented May 24, 2015

As a long time mongoose user, I don't really understand why mongoose 4 is using an exotic (thus probably quite slow) promise implementation over native ones or bluebird (clearly the best pick for the past couple of years)? Why not allow every end user to choose it's library as long as it comply with the A+ spec? something like require('mongoose').usePromise(require('bluebird'))?

@benjamingr
Copy link
Contributor

@mgcrea for one thing you're using free software ;)

I'm sure pull requests are welcome - and as mentioned before using bluebird with Mongoose is already a one-liner

@mgcrea
Copy link
Contributor

mgcrea commented May 24, 2015

@benjamingr, are you referring to promisifyAll(require("mongoose"))?
It's not what I would call a swappable implementation since you have to swap all your methods names, from Model.find().then with mpromise to Model.findAsync().then with bluebird+promisify.

If there is consensus towards removing the tightly coupling between mongoose and mpromise, I would gladly spend some time to craft a potential PR, but for now, I need a better understanding of the reasoning behind the mpromise pick before acting and wasting everybody's time.

@benjamingr
Copy link
Contributor

@mgcrea

I need a better understanding of the reasoning behind the mpromise pick before acting and wasting everybody's time.

People asked for promises support and Mongoose is quite a mature library now over 4 years old- mpromise was chosen because it looked like a good way to add promise support at a time. There was no bluebird when the choice was made and there certainly were no native promises.

@mgcrea
Copy link
Contributor

mgcrea commented May 24, 2015

@benjamingr I though mpromise was picked recently, but it looks like it just only really spun out of mongoose core as a separate library. Thanks for the explanation.

@vkarpov15
Copy link
Collaborator

@mgcrea that's the whole point of #2688. We hope to be able to support arbitrary ES6 promise libraries in the next couple months.

Also, @benjamingr is right, mpromise has been a part of mongoose for quite a long time. We kept it in as a "good enough" solution while we worked on issues we believed to be more pressing. Re-addressing promises is an immediate priority now though.

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

No branches or pull requests