CODEWITHSUNDEEP
javascriptexpresspassport.js
jennifer
jennifer

Reputation: 762

done() vs return done()

I was reading the docs for passport and I noticed that with serialize() and deserialize() done() is called with out being returned.

However when setting up a new strategy using passport.use() in the callback function return done() is used.

Is this something that needs to be understood or just copied from the docs?

http://www.passportjs.org/docs/

From the docs:

var passport = require('passport')
  , LocalStrategy = require('passport-local').Strategy;

passport.use(new LocalStrategy(
  function(username, password, done) {
    User.findOne({ username: username }, function (err, user) {
      if (err) { return done(err); }
      if (!user) {
        return done(null, false, { message: 'Incorrect username.' });
      }
      if (!user.validPassword(password)) {
        return done(null, false, { message: 'Incorrect password.' });
      }
      return done(null, user);
    });
  }
));

Upvotes: 5

Views: 1897

Answers (1)

tex
tex

Reputation: 2766

return done() will cause the function to stop executing immediately. This means that any other lines of code after that line inside the function will be ignored and not evaluated.

done() not preceded by return, however, will not cause the function to stop executing. This means that any other lines of code after that line inside the function will be evaluated.

If you take a look at this passport.use() example (from the Passport docs), you'll see there is reachable code after the first three return done() statements, and you would want the function to exit immediately as soon as done() is called the first time to make sure none of the following instructions are evaluated:

passport.use(new BasicStrategy(
  function(username, password, done) {
    User.findOne({ username: username }, function (err, user) {
      if (err) { return done(err); } 
      if (!user) { return done(null, false); }
      if (!user.validPassword(password)) { return done(null, false); }
      // The following line is the success case. We do not want to execute it
      // if there was an error, a falsy user or a user without a valid 
      // password. If we removed the return keywords from the previous lines
      // of code, the success case would be triggered every time this
      // function was called
      return done(null, user);
    });
  }
));

Here I've added two executable snippets to illustrate the difference between done() and `return done(). The snippets are otherwise identical.

done() without return: 

const done = console.log

const assessThreatLevel = threatLevel => {
  if (threatLevel === 'all good') done('relax :)')
  done('launch the missiles!')
}

assessThreatLevel('all good')

`return done():

const done = console.log

const assessThreatLevel = threatLevel => {
  if (threatLevel === 'all good') return done('relax :)')
  done('launch the missiles!')
}

assessThreatLevel('all good')

As an aside, I have taken to using return done() in most situations for consistency. As far as I'm aware, there's no drawback to using it. It can help you avoid bugs, and the return statement serves as a guarantee and a good visual reminder that the function will exit immediately after that statement is evaluated.

Upvotes: 4

Related Questions