alkemics/CancelablePromise

pop() instead of shift()?

Closed this issue · 4 comments

It seems to me that removing from the last promise is more logical, what do you think?

cancel() {
  options.isCanceled = true;
  for (const callbacks of [options.onCancelList, options.finallyList]) {
    if (callbacks) {
      while (callbacks.length) {
        const onCancel = callbacks.pop();
        if (typeof onCancel === 'function') {
          onCancel();
        }
      }
    }
  }
},

Perhaps also every finally should be called after its corresponding onCancel.

removing from the last promise

What do you mean? You cancel from the promise which calls the cancel method. A cancelable promise does not have access to other chained promises, they only share some internals such as the isCanceled boolean or the onCancelList array. Then I used shift to keep the order in which you created your promise callbacks, for example consider 3 promises p1, p2, and p3 which each declare onCancel and finally callbacks in this order:

1. p1 onCancel
2. p1 finally
3. p2 onCancel
4. p2 finally
5. p3 onCancel
6. p3 finally

So if you cancel one of these promises, it will execute onCancel and finally callbacks in this order:

1. p1 onCancel
2. p2 onCancel
3. p3 onCancel
4. p1 finally
5. p2 finally
6. p3 finally

@alk-sdavid It seems to me that the correct order after cancel() is:

1. p3 onCancel
2. p3 finally
3. p2 onCancel
4. p2 finally
5. p1 onCancel
6. p1 finally

this will change in the next release, there is indeed no good reasons to split onCancel and finally callbacks, I'll merge these lists and so the callbacks declarations will be respected