Skip to content

Instantly share code, notes, and snippets.

@trevnorris
Last active March 27, 2016 14:32
Show Gist options
  • Save trevnorris/d9c77a68e5af4e642bb3 to your computer and use it in GitHub Desktop.
Save trevnorris/d9c77a68e5af4e642bb3 to your computer and use it in GitHub Desktop.
Reasons why it's not good to use Promises for logical flow control
From Bradley Meck (https://twitter.com/bradleymeck)
1. when using promises for notification you need to avoid recursively calling
.then , it is easy to make nested closures if a fn in .then produces another fn
2. you are mixing logic, .then vs .catch as a means to branch is not quite the
same as try/catch (and they don't have a finally mechanism)
3. you can use logic inside of a .then handler to route creation of a new
promise which is fine, but if that promise is returned you get into major
problems if you have a branch. example follows if I have:
1. task_a (soft fail semantics [Promise])
2. task_b (fail fast semantics [Promise.all])
2.1. task_b_a
2.2. task_b_b
3. task_c (race semantics [Promise.race])
3.1 task_c_a
3.2 task_c_b
if task_a.then returns task_b, you are now bound to task b's logic for routing.
we are ok for the first level as we are just changing to fast fail semantics;
however, just need to be careful not to cleanup anything that may be in use by
task_b_* that may be in use even though we failed fast. the lack of finally
here causes issues and extra overhead that can cause problems
next we get into race semantics when task_b chains to task_c, when routing the
chain via a race we want to return on the first result (either error or
success), this means we need to be careful about how we handler dropped errors
(if they are not the first result). most situations will want to avoid using
race unless you are doing timeouts.
in general, you start mixing semantics without a good language construct to
help have all the error/resource handling be unified
you *can* code around it, but that is not what I see in the wild
another way to think of this: a promise as a data type, for reacting to an
eventual value. when using them as notification like .ready(), something may
never be ready (neither fail nor success) and we don't have a good way to
notify that it won't be ready
if you use any sort of cache / make non-weak bindings to the promise (putting
it in a Map for example) :(
Additional information from a conversation in IRC:
<bradleymeck> they are a good data struct for eventual values
<bradleymeck> in an ideal (pure) world they aren't a problem since the side
effects are less likely to happen and you won't need resource cleanup semantics
<bradleymeck> though I may just have been in the rabbit hole of leaking
resources for too long T_T
<ljharb> bradleymeck: so #1 i think applies no matter what you're using
promises for - ie, not nesting thens
<bradleymeck> ljharb: correct but recursive break conditions are a common use
of them
<bradleymeck> i don't see nesting to an arbitrary depth as being terrible
<bradleymeck> same as nesting fns
<ljharb> bradleymeck: if someone made an `if/then` promise subclass would that
satisfy #2?
<bradleymeck> ljharb: no, because it automatically swallows errors, warns on
not having a catch handler, and still won't have a true finally mechanism
<ljharb> well, wouldn't a "finally" just be a .then(cleanup, cleanup) on the
promise (not chained)?
<bradleymeck> ljharb: not exactly, because if my promise opens a fd but does
not want to give the fd to the user... when does it close the fd?
<bradleymeck> you start getting to .dispose() style cleanup but that assumes no
errors happen on your way to dispose()
<bradleymeck> also cleanup is only for the next in the chain not the final
propagation
<bradleymeck> in example above*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment