29
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?

BlackMamba
  • 10,054
  • 7
  • 44
  • 67
  • no you don't need to return, you just have to call the callback resolve/reject function. – Tuan Anh Tran Jan 08 '16 at 03:09
  • 5
    It has `return reject(err)` to halt the function at that point. The alternative would be `if (err) { reject(err); } else { resolve(user); }` – Phil Jan 08 '16 at 03:11
  • 2
    @Phil - although, in this example it doesn't really matter as a Promise can only be fulfilled/rejected once, so if reject were called, resolve would be effectively ignored – Jaromanda X Jan 08 '16 at 03:14
  • @Phil Could you give some detail explantion? For other callbackm , use `retrun` is must. – BlackMamba Jan 08 '16 at 03:15
  • For heaven's sake - don't write this code manually - you can use a library (like bluebird) that'll generate a wrapper for a callback API in one go for you. – Benjamin Gruenbaum Jan 08 '16 at 03:18
  • See also [Do I need to return after early resolve/reject?](https://stackoverflow.com/questions/32536049/do-i-need-to-return-after-early-resolve-reject) and maybe [Should I return the result of deferred.resolve/reject?](http://stackoverflow.com/q/32053334/1048572) – Bergi May 16 '16 at 20:20

3 Answers3

54

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
});
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Shortly return means will terminating the code execution. – Faris Rayhan Mar 10 '18 at 00:08
  • 1
    @FarisRayhan - Yes that is what the second paragraph of my answer says. `return` is for controlling flow of code (exiting callback). – jfriend00 Mar 10 '18 at 00:33
  • And sometimes even just: new Promise((resolve, reject) => db.values.insert(value, (err, user) => err ? reject(err) : resolve(user))); – Killroy Oct 12 '18 at 08:38
3

Generally, in NodeJS, you shouldn't use the promise constructor very much.

The promise constructor is for converting APIs that don't return promises to promises. You should consider using a library that provides promisification (even if you use native promises all-around) since it provides a safe alternative that does not have subtle errors with error-handling logic.

Automatic promisification is also considerably faster.

That said, the answer to your question is "Yes".

It is perfectly safe to do so, there is nothing special about promise constructors - they are just plain JavaScript. Domenic discusses the design of the promise constructor in his blog.

It is perfectly safe (just like any other function) to return early - it is actually quite common in regular asynchronous functions.

(Also, in your example code you should just use Promise.resolve, but I assume it was that simple only because it is an example).

Copied this answer from duplicate

Community
  • 1
  • 1
Benjamin Gruenbaum
  • 270,886
  • 87
  • 504
  • 504
  • Could you provide a quick example of a "subtle error with error-handling logic" in home-grown promisification? –  Jun 20 '16 at 13:58
  • @torazaburo sure, you need to deal with the case where the function you call `throw`s synchronously for instance, the case where the callback is called with multiple parameters and deal with synchronous throws of proxies when you call `resolve` in the asynchronous context. Lots of fun. – Benjamin Gruenbaum Jun 20 '16 at 13:59
-1

As @JaromandaX said in this case the return statement does not make any diference. From the docs:

In all cases where a promise is resolved (i.e. either fulfilled or rejected), the resolution is permanent and cannot be reset. Attempting to call resolve, reject, or notify if promise is already resolved will be a no-op.

  • incorrect - once a promise is rejected/fulfilled any further attempt to do so is ignored – Jaromanda X Jan 08 '16 at 03:15
  • @JaromandaX I Edited my answer with your explanation, thank you. – Vinicius Zaramella Jan 08 '16 at 03:22
  • It is not 100% correct to say "does not make any difference". It does not make any difference in the behavior of the program, but omitting it results in an unneeded call to `resolve`, which presumably has some minor performance implication, and possibly less importantly, it makes the program read slightly less semantically, because it seems as if it might be resolving the promise, although of course for the reason you mentioned in actuality it is not. –  Jan 08 '16 at 03:31
  • It also makes the program more vulnerable to bugs, because when some developer unwittingly adds another statement before the `resolve`, imagining it will be executed only in the success case, it will actually get executed even after the `reject`. Therefore, I would say the `return` is definitely recommended. –  Jan 08 '16 at 04:32