11

So I have a Posts collection

{
  id: String,
  comments: [String], # id of Comments
  links: [String], #id of Links
}

Comments: { id: String, comment: String, }

Links: { id: String, link: String, }

Find a post with comments and links belong to it by id:

Posts.findOne({id: id}, function(post) {
  Comments.find({id: post.id}, function(comments) {
    Links.find({id: post.id}, function(links) {
      res.json({post: post, comments: comment, links: links})
    })
  })
})

How to use Promise(http://mongoosejs.com/docs/promises.html) to avoid callback hell?

var query = Posts.findOne({id: id});
var promise = query.exec();

promise.then(function (post) {
  var query1 = Comments.find({id: post.id});
  var promise1 = query1.exec();
  promise1.then(function(comments) {
    var query2 = Links.find({id: post.id});
    var promise2 = query2.exec();
    promise2.then(function(links) {
      res.json({post: post, comments: comment, links: links})
    })
  })
});

Seems no good......

Sato
  • 8,192
  • 17
  • 60
  • 115

6 Answers6

4

Try whit this:

function getPost(id) {
  return Post
    .findOne({id: id})
    .then( post => {
      return post;
    });
}

using Q module

function getCommentsAndLinks(post) {
  return Q.all([
    Comment.find({id: post.id}),
    Links.find({id: post.id})
  ])
  .done( results => {
    let comments = results[0];
    let links = results[1];
    return [post, comments, links];
  })
  .catch( err => {
  // handle err
  })

on controller

getPost(postId)
.then(getCommentsAndLinks)
.then( results => {
  let post = results[0];
  let comments = results[1];
  let links = results[2];
  // more code here
})
.catch( err => {
// handle err
}) 

but i suggest you not save string of IDS, save the instance of object, so you can use populate to get all data of comments and links, something like this:

Post
.findOne({id: id})
.populate('comments')
.populate('links')
.then( post => {
    // here have the post with data of comments and links
});
DJeanCar
  • 1,463
  • 1
  • 11
  • 13
  • @Sato - The first version here serializes `getComments()` and `getLinks()` which is not necessary. Running them in parallel (as in my answer) will likely perform better. Also, make sure you're understanding how to do error handling in either your non-promise or in a promise version. – jfriend00 Mar 10 '16 at 01:11
  • @jfriend00 you are right, i edited my answer using [Q](https://www.npmjs.com/package/q) module, also you can use `Promise.all()` if you prefer. – DJeanCar Mar 10 '16 at 01:37
3

You are nesting the callbacks. You don't need to do this. If you return a promise from .then then any .then you chain to it will be resolved when that promise gets resolved:

promise.then(post => Comments.find({id: post.id})
  .then(comments => Links.find({id: post.id})
  .then(links => {});

The comments query does not depend on links so you can actually do both queries at once:

promise.then(post => {
  return Promise.all([
     post,
     Comments.find({id: post.id}),
     Links.find({id: post.id}),
  ]);
}).then(data => res.json({
  post: data[0],
  comments: data[1],
  links: data[2],
});

If you use a library like bluebird you can also use something like the spread operator to make the names more transparent.


I would also look into using co for generator-based control flow as I think this is even clearer:

co(function* () {
  const post = yield Posts.findOne({id});
  const [comments, links] = yield [
    Comments.find({id: post.id}),
    Links.find({id: post.id}),
  ];

  res.json({post, comments, links});
});
Explosion Pills
  • 188,624
  • 52
  • 326
  • 405
  • 1
    Clever way to get the `post` variable through to the next `.then()` handler by passing it into `Promise.all()`. – jfriend00 Mar 10 '16 at 08:56
2

You can do it using promises like this:

Posts.findOne({id: id}).exec().then(function(post) {
    let p1 = Comments.find({id: post.id}).exec();
    let p2 = Links.find({id: post.id}).exec();
    return Promise.all([p1, p2]).then(function(results) {
        res.json({post: post, comments: results[0], links: results[1]});
    });
}).catch(function(err) {
    // error here
});

This sets up two operations Comments.find().exec() and Links.find().exec() that both depend upon the post variable, but are independent of each other so they can run in parallel. Then, it uses Promise.all() to know when both are done and then the JSON can be output.

Here's the step by step description:

  1. Run Posts.findOne().exec().
  2. When it's done, start both Comments.find().exec() and Links.find().exec() in parallel.
  3. Use Promise.all() to know when both of those are done.
  4. When both of those are done, then output the JSON.

This could be done with less nesting, but because you're using prior results in subsequent requests or in the final JSON, it's easier to nest it a bit.

You can see various options for sharing prior results while chaining promise requests in this other answer How to chain and share prior results.


FYI, where this promise implementation really shines compare to what you show in your question is for error handling. Your non-promise code shows no error handling, but the promise version will propagate all errors up to the .catch() handler for you.

Community
  • 1
  • 1
jfriend00
  • 683,504
  • 96
  • 985
  • 979
1

The advantage of using promises is you can chain them, so your code can be reduced to:

let post, comments;
Posts.findOne({id: id}).exec().then(_post => {
    post = _post;
    return Comments.find({id: post.id}).exec();
  }).then(_comments => {
    comments = _comments;
    return Links.find({id: post.id}).exec();
  }).then(links => res.json({post, comment, links}))
  .catch(error => res.error(error.message));

you would notice that I needed only a single catch block.

mido
  • 24,198
  • 15
  • 92
  • 117
1

Here's a somewhat shorter version

Posts.findOne({id: id}).then(function (post) {
  var query1 = Comments.find({id: post.id});
  var query2 = Links.find({id: post.id});

  Promise.all(query1.exec(), query2.exec()).then(function(data) {
     res.json({ post: post, comments: data[0], links: data[1] });
  });
});
gabesoft
  • 1,228
  • 9
  • 6
-3

In my opinion, you can't avoid callback hell. It's the nature of async programing. You should take advantage of async programing, not trying to make it looks like synchronous.

You must use callback to create a promise, just to achieve the "then" syntax. The "then" syntax looks better, but not really provide anything useful than callback, why bother. The only useful feature of promise is Promise.all, which you can use to wait for all your promises finish.

Try using rxjs for handling async problems. You still have to use callback to create a rxjs observable. But rxjs provides lots of functionality to help you take advantage of async programing, not avoid it.

Chef
  • 633
  • 5
  • 17
  • the responses above show that it is possible and desirable avoiding callback hell with promises. see answer from @Explosion Pills – pungggi Apr 19 '17 at 14:47