node.js - should i use `return` in Promise?

Question

function saveToTheDb(value) {  
  return new Promise(function(resolve, reject) {
    db.values.insert(value, function(err, user) { // remember error first ;)
      if (err) {
        return reject(err); // don't forget to return here
      }
      resolve(user);
    })
  }
}

Here is the code which i see from here. i am confused about return keyword.

For resolve(user);, do i need return?

For reject(user);, do i need return?

Answer 1

There is no need to use a return statement inside a new Promise() callback. The Promise constructor is not expecting any sort of return value from the callback.

So, the reason to use a return statement inside that callback is only to control the flow of execution in that function. If you want execution inside your callback to finish and not execute any more code within that callback, you can issue a return; at that point.

For example, you could have written your code like this with no return statement:

function saveToTheDb(value) {  
  return new Promise(function(resolve, reject) {
    db.values.insert(value, function(err, user) { 
      if (err) {
        reject(err);
      } else {
        resolve(user);
      }
    });
  }
}

In this case, you used the if/else clause to make sure the flow of control in your function takes the correct path and no return was needed or used.

---

A common shortcut when promisifying async functions like this is:

function saveToTheDb(value) {  
  return new Promise(function(resolve, reject) {
    db.values.insert(value, function(err, user) { 
      if (err) return reject(err);
      resolve(user);
    });
  }
}

This is not functionally different than the previous code block, but it is less typing and more compact. The return statement in front of reject(err); is only for flow of control reasons to prevent from executing the resolve(user); statement in case of error since the desired flow of control is to call reject(err) and then not execute anything else in the callback.

---

In fact, the return statement in this last block is not actually even needed in this specific case because executing a resolve() after a reject() will not do anything since promises are latched to whichever happens first resolve or reject. But, it is generally considered poor practice to execute unnecessary code so many would argue that it is better to use flow of control structures such as if/else or return to only execute the code that is needed.

So, this would technically work too, but is not considered a best practice because it executes unnecessary code and isn't as clearly structured:

function saveToTheDb(value) {  
  return new Promise(function(resolve, reject) {
    db.values.insert(value, function(err, user) {
      if (err) reject(err);
      resolve(user);
    });
  }
}

---

FYI, what you are doing here is called "promisifying" which makes a regular async function that works with a callback into a function that returns a promise. There are libraries and functions that will "promisify" a function or a whole object of functions (e.g. a whole API) for you in one function call so you don't have to do this manually. For example, I regularly use Bluebird which offers Promise.promisify() for promisifying a single function or Promise.promisifyAll() which will promisify all the methods on an object or prototype. This is very useful. For example, you could get promisified versions of the entire fs module with just this:

var Promise = require('bluebird');
var fs = Promise.promisifyAll(require('fs'));

Then, you can use methods that return a promise such as:

fs.readFileAsync("file.txt").then(function(data) {
    // do something with file.txt data here
});