-1

I want to filter and route promise based on its result, so I use code like this that uses unresolved promise to omit executing of a branch:

const resultP = getSomething()
  .then((result) => result === 'good' ? true : false )
    
const goodStuffP = resultP
  // procceds if r is true, otherwise returns unresolved promise (so that branch is omitted)
  .then((r) => r ? r : new Promise(() =>{})) 
  .then(...do good stuff...)

const badStuffP = resultP
 // procceds if r is false, otherwise returns unresolved promise
.then((r) => !r ? r : new Promise(() =>{}))
.then(...do bad stuff...)

I wonder if something wrong with this approach, that thing that doesn't look ok to me is an unresolved promise.

So is using unresolved promise this way is a valid use in terms of possible tech issues (like leaks)? The question is not about good/bad programming style.

WHITECOLOR
  • 24,996
  • 37
  • 121
  • 181

2 Answers2

0

No, don't use unresolved promises for control flow. Just use if/else inside that then callback:

const resultP = getSomething().then(result => {
  if (result === 'good') {
    // ...do good stuff, return a value
  } else {
    // ...do bad stuff, return a value
  }
});
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Why, this is the obvious option, I'm asking about why my option is not valid if so. – WHITECOLOR Oct 25 '20 at 22:42
  • 1
    @WHITECOLOR Because it is bad coding not to use the obvious, simple and working solution. The approach with unresolved promises has too many non-obvious edge cases (like bugs when it comes to dealing with rejections). Not sure what you mean by "valid", but there is simply no good reason to use it even if you fix the bugs. – Bergi Oct 25 '20 at 22:45
  • I mean can it cause some tech issues like a leak or something. – WHITECOLOR Oct 25 '20 at 22:46
  • @WHITECOLOR Yes, it has those (I briefly mentioned one - hint: try rejecting `getSomething()`), but what makes the solution so horrible is that you cannot easily notice them. – Bergi Oct 25 '20 at 22:47
  • What is the problem with rejecting? It will be handle in catch if needed, in both branches. – WHITECOLOR Oct 25 '20 at 22:59
  • Found this issue btw https://github.com/nodejs/promises-debugging/issues/16 it states that not resolving a promise is actually a valid case. – WHITECOLOR Oct 25 '20 at 23:11
  • @Whitecolor: yes, the problem is that you now need to do it in both branches. Which works subtly different than the normal straight-forward solution. – Bergi Oct 25 '20 at 23:15
  • 1
    And yes, not resolving promises is valid (see [here](https://stackoverflow.com/q/34289551/1048572) or [there](https://stackoverflow.com/q/38286358/1048572)) and [is expected not to leak memory](https://stackoverflow.com/q/20068467/1048572) (although [some implementations don't live up to that](https://bugs.chromium.org/p/v8/issues/detail?id=9858)). However, it is not a valid solution for the `if`/`else` branching problem. – Bergi Oct 25 '20 at 23:20
  • Sometimes you want to avoid clumsy if/else (in general), the same for try/catch constructs. – WHITECOLOR Oct 25 '20 at 23:22
  • 1
    @Whitecolor: "clumsy" is subjective, and in my opinion a never-resolving promise is a much more clumsy solution. But it seems we disagree there. Or maybe you've haven't presented your actual use case well. – Bergi Oct 25 '20 at 23:24
  • Yeah, this clumsiness can be concealed if needed. This issue is to get FP oriented style. – WHITECOLOR Oct 25 '20 at 23:34
0

Actually, the answer seems to be like this: using not ever resolved promises in the code is an absolutely valid case. It is like not calling the callback, which is not should be forced. Just catch and handle errors correctly.

Nodejs repo issue discussing not ever resolved promises in the code: https://github.com/nodejs/promises-debugging/issues/16

WHITECOLOR
  • 24,996
  • 37
  • 121
  • 181