I am using ExpressJS and needed to run and pass ESLint rules on my code. There is a rule "consistent-return", which is thrown for code like this:

function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()
}//getUsers()

Now, this code gives 'consistent-return' error. There seem to be 2 possible changes, which remove this error, I'm not sure which of these is correct.

function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()

  return false; // adding return false passes the ESLint error
}//getUsers()

OR

 function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  // adding return in below line passes the ESLint error.
  return collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()
}//getUsers()

Please advise which is the right way to go about it. Thanks.

I am using ExpressJS and needed to run and pass ESLint rules on my code. There is a rule "consistent-return", which is thrown for code like this:

function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()
}//getUsers()

Now, this code gives 'consistent-return' error. There seem to be 2 possible changes, which remove this error, I'm not sure which of these is correct.

function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()

  return false; // adding return false passes the ESLint error
}//getUsers()

OR

 function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  // adding return in below line passes the ESLint error.
  return collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()
}//getUsers()

Please advise which is the right way to go about it. Thanks.

• javascript
• node.js
• express
• asynchronous
• eslint

Share Improve this question Follow edited Jan 5, 2017 at 13:05 Mohit Bhardwaj asked Jan 5, 2017 at 12:21 Mohit BhardwajMohit Bhardwaj 10.1k77 gold badges4040 silver badges6565 bronze badges

Add a ment  | 

2 Answers 2

Sorted by: Reset to default 8

The rule here points to a real consistency problem: sometimes you seem to return a value, and sometimes you don't. And it only works because the returned value is ignored. The only point of your return statement is to branch out of executing the rest of the function.

Your code here is misleading and it hurts the readability.

Replace

if( err ){
  return next();
}

with

if( err ){
  next();
  return;
}

The whole not confusing code:

function getUsers( req, res, next ){
    if( req.userIds.length === 0 ){
        next();
        return
    }
    collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
        if( !err ){
            req.users = doc;
        }
        next();
    });//find()
}//getUsers()

Let us analyze your code a bit, with some details stripped out:

function getUsers( req, res, next ){

  if( req.userIds.length === 0 ){
    return next();
  }

  collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    // lotsa code here
  });

}

Here, inside the function getUsers(), there can two code-paths: either an error condition (when req.userIds is empty) or not.

Depending on whether the error condition is satisfied, the function getUsers() is either returning something (next()) or nothing (actually returns undefined implicitly). Note that collection.find() is executed in the second case, which itself may return something within it, but getUsers() won't know about it.

Now, as the ESLint docs state, a function can either return something or nothing (implicit undefined), but not both (which is inconsistent behaviour); otherwise a consistent-return error is thrown.

So, to resolve the error, both Denys' code and your own second possible change will work. In Denys' code, both code paths return nothing (implicit undefined) while in yours, they both return something.