always-return should ignore the last then

[nkovacs]

Taking the example from the linked article and extending it a bit:

getUserByName('nolan').then(function (user) {
  if (user.isLoggedOut()) {
    throw new Error('user logged out!'); // throwing a synchronous error!
  }
  if (inMemoryCache[user.id]) {
    return inMemoryCache[user.id];       // returning a synchronous value!
  }
  return getUserAccountById(user.id);    // returning a promise!
}).then(function finalThen(userAccount) {
  console.log(userAccount);
}).catch(function (err) {
  console.error(err);
});

There's no need to return anything in finalThen. Would it be possible to ignore these cases (in the last then, when the promise isn't returned or used in any way)?

The following would become valid:

myPromise.then(function(val) {});

The following would remain invalid:

myPromise.then(function(val) { /* missing return here */ }).then(function(val2) { /* no error here */ });
var p = myPromise.then(function(val) {});
return myPromise.then(function(val) {});

[xjamundx]

I kind of need to think about this. At some point you do want to do something with the data, so sure, but in general it's nice to always return something.

I don't know @nolanlawson do you have any thoughts on this?

[friday]

I agree. I can't use eslint with promise/always-return enabled now.

• False positives makes people ignore or turn off warnings (the moral of "The Boy Who Cried Wolf")
• Your then/catch-block might not even have anything to return. console.log() for example doesn't return anything.
• Your code should have functional rather than cosmetic effects. In general eslint tries to detect and eliminate redundancy, because code that doesn't really do anything indicates bloat and/or errors. See for example: no-useless-return
• You could mistake this for intentional later on, if you need to add to the code. Let's say your last then() ends with return X(), and you need to add something after running X(), you would fool yourself and change the code to something like this:

const x = X();
doSomethingWith(x)
return x;

If anything, I think a rule to ensure that you don't return in the last call (like "no-useless-return" but for promises) would be fitting.

I respect that this might be a challenging change, or that you may lack the time or interest however.
Thanks for your great work so far! :)

[macklinu]

This is an interesting idea and one that would be nice to explore in the promise/always-return rule. Not sure how we'd detect this off-hand, but I'm thinking context.getScope() would be necessary in order to track references to variables.

PRs are definitely welcome!

[dbkaplun]

This would be great! Thanks for considering!

[github-actions]

🎉 This issue has been resolved in version 6.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀