Add .complete() and .then() methods to Promise and find-chain.

[ptnplanet]

In response to #316: I've just added the .complete() method to the Promise API. This enables traditional callbacks with err, result arguments to be used with the .success().fail().complete() syntax.

[ptnplanet]

When writing tests for the .fail() method, how do I force an error? I tried creating one with .find({ notExistingField : '' }), but that does not work with mongodb. I had to change my previous tests for the travis build to pass.

[dresende]

Perhaps with a hook that forces failing. I'll look into this and add the test.

[dresende]

I'm not sure if .complete shouldn't be .done. I don't know much about Promises, don't use them but maybe we should stick with the usual names (that's why I used .fail instead of .error).

[ptnplanet]

Yes, you're right. jQuery's deferred has a .done, .fail and .then method. That is probably the way to go.

[ptnplanet]

In my latest commit, I've left the tests for .fail commented out because I wasn't able to get a hook to throw an error in the find chain.

[servatj]

how can i cancel notificacions in github ?

2013/8/27 Philipp [email protected]

When writing tests for the .fail() method, how do I force an error? I
tried creating one with .find({ notExistingField : '' }), but that does
not work with mongodb. I had to change my previous tests for the travis
build to pass.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/317#issuecomment-23331892
.

by yuse

[ptnplanet]

Set your notification status to "Not watching" on the top right to not receive notifications for the whole repo and/or click "Unwatch thread" on the bottom left of this page.

[dresende]

@ptnplanet don't worry about the hooks, I was talking about maybe afterLoad model hook but I can add the test after merge.

[dresende]

@dxg @spartan563 opinions about this 😄 ?

[notheotherben]

Read an interesting article a while ago on "Node's biggest design mistake", essentially saying that they should have gone with a promise architecture from the start. Very interesting read, and they made a good argument for the design methodology.

I feel that at some point it will be worth writing up a formal API spec, from which we derive all future API surfaces. At the moment it seems like the project has a bunch of awesome ideas and new features appearing, but no structured place for them to be added. Ideally, I'd love to discuss a clean API rewrite of the module - formalizing many of the current design decisions, and giving us something to fall back on when looking at the best way to implement a new feature. It should also make it somewhat easier for people to use the library, since at the moment there are a bunch of undocumented features that get added without proper supporting documentation.

Anyway, long story short - this seems like a good addition to the module. 😄

[ptnplanet]

This is the article you are probably referring to. The thing I especially like about JavaScript is the ability to write both functional and object-orientated imperative programs.

There is still one major issue with this mockup implementation of a promise. It isn't really a promise yet, because it is time dependent and thus more imperative again. And this violates the promise pattern. With promises, the following should be possible:

var promise = Model.find(/* ... */).run();
/* Do something time consuming ... */
promise.then(/* ... */);

So instead of merging this pull request and adding pseudo-promise only to the find chain, how about adding a more thought-through system on lower levels. Backwards-compatibility isn't really an issue - the following would still be possible:

var promise = Model.find(/* ... */);
promise.run().then(/* ... */);

// Support 'traditional' callbacks
Model.find(/* ... */, function (err, results) { /* ... */ }).done(/* .... */);

// And even stuff like this:
Model.create(/* ... */).done(/* ... */).fail(/* ... */).always(/* ... */);

Thoughts?

[notheotherben]

Yeah, that's the one. When I've got some free time again I'd love to take a look at implementing this - it looks like a great feature to have (if done right). Only issue I can see is that it's gonna make me start wishing that other Node modules used promises more often as well 😄

[ptnplanet]

Have a look at the Promises/A and A+ specification proposals. I would love to implement this over the next week or two. Let me know, if there is room for any contribution.

There is a node module called Q, that enables converting callbacks to promises.

[dxg]

Sounds good to me.

[dresende]

@ptnplanet I want to rewrite the Promise.js file with a Promise "class" based on some spec. I was looking at the html spec but I'll look into that too.

[ptnplanet]

Instead of writing it yourself - it has been done quite a few times.

[dresende]

I know but I can't seem to find a striped one (minimalistic don't fulfill your link or have extra things and I liked the A+ spec). I just created a basic Promise. I'll probably make a small module with it. It was good to better understand how they work, they're not as easy to implement as I imagined (full spec).

[dresende]

Here: https://github.com/dresende/node-fulfill

[calmdev]

To keep this conversation going...

I am doing what @ptnplanet has suggested with Q's nbind method to convert all the node style methods on the models into promises when they are loaded. In addition to node-orm2 this code below is using async, underscore and Q. I need to finish removing the async stuff in favor of promises, but hopefully this illustrates the idea. All you really need is Q.

Feels a bit hackish, but it does certainly work for me thus far. I just started learning about promises this week, so if this is problematic or can be improved on please let me know....

/* Model create with promise...
You can use: catch, progress, finally, done, then, fail, etc. 
Anything Q has to offer. Simple example to illustrate:*/
db.models.MyModel.create({...})
.then(function(record) {
    console.log('record created', record);
})
.fail(function(error) {
    console.log('something went wrong', error);
});

// Connect and load models.
orm.connect('...', function(err, db) {
    load(db, next);
});

// Load models from individual files.
function load(db, next) {
    var modelFileLoader = function (file) { 
        return function (cb) { 
            db.load(file, function() {
                // Convert model methods into Q promises.
                var model = _.last(_.keys(db.models));
                var methods = _.functions(db.models[model]);
                _.each(methods, function(method) {
                    db.models[model][method] = Q.nbind(db.models[model][method], db.models[model]);
                });
                cb();
            }); 
        }; 
    };
    async.parallel([
        modelFileLoader('MyModel'),
        modelFileLoader('MyOtherModel')
    ], function (error) {
        if(error) throw error;
        db.sync(next);
    });
};

[tb01923]

calmdev, that is a nice little implementation. I have played around with it a bit, and run this after my models are loaded and after I form relationships. I replace the methods with one with a 'q' in front of it - for example findByParent becomes qFindByParent. It allows me to have this chaining syntax:

object.find({key: value}).find({key: value}).qFind({key: value}).then()

Unfortunately the underscore functions method is not picking up all of the methods. Because of the hasOne relationship ORM creates for me the getParent method, but underscore function doesn't seem to find it. Just an FYI. I will see if i can get to the bottom of it.

[calmdev]

@tb01923 fwi, I've switched to sequelizeJS which has built-in support for promises.