Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft B #5

Open
novemberborn opened this issue Dec 29, 2012 · 6 comments
Open

Draft B #5

novemberborn opened this issue Dec 29, 2012 · 6 comments

Comments

@novemberborn
Copy link

See https://gist.github.com/4407774 for the spec. Work-in-progress implementation at https://github.com/novemberborn/legendary/tree/cancelation-draft-b.

  • Simplified cancel signature
  • Removed need for propagateCancel
  • Resolvers don't emit canceled events
@novemberborn novemberborn mentioned this issue Dec 29, 2012
@ForbesLindesay
Copy link
Member

If options is not null or of type object its properties, except for name, are copied onto the OperationCanceled error.

should read:

If options is of type object its properties, except for name, are copied onto the OperationCanceled error.

@ForbesLindesay
Copy link
Member

Resolvers may directly cancel themselves if the only derived and pending promise is canceled (directly or indirectly).

should read:

Resolvers may directly cancel themselves if all derived promises are cancelled (directly or indirectly).

@ForbesLindesay
Copy link
Member

Good points

  • Removing need for .propagateCancel method
  • Simplified cancel signature
  • Let cancellation handling be implementation specific (until resolvers are spec'd at least)

Bad points

  • Directly cancelling promises higher up the chain is too simplistic (see Interrupts #3)
  • We don't have a chain that goes resolver -> promise, we have a chain that goes (resolver -> promise) -> (resolver -> promise) -> (resolver -> promise). We need to make the spec follow that through well.

Solution

One solution to the first point is to require an onCancelled method, leading to the signature:

promise.then(onFulfilled, onRejected, onProgress, onCancelled)

onCancelled would then be called whenever a promise is cancelled either directly or indirectly. If the promise was cancelled directly, it is called instead of the onRejected handler if it's present.

When you wire up one promise, to another (resolver, promise) pair you then need to do something like:

promiseA.then(resolverB.fulfill, resolverB.reject. resolverB.progress);

promiseB.then(null, null, null, promiseA.cancel);

which is hideous, we need to sort this.

@ForbesLindesay
Copy link
Member

In short, I don't think removing .propagateCancel like this actually works.

@novemberborn
Copy link
Author

The trick I'm using for propagating cancelation is that with:

var aToB = promiseA.then(resolverB.fulfill, resolverB.reject, resolverB.progress);

When resolverB is canceled, it can propagate by canceling aToB. The implementation can then decide how to propagate that cancelation internally.

@ForbesLindesay
Copy link
Member

OK, that's fine, since the resolver gets cancelled for direct and indirect cancellation, that should work, along the lines of:

var aToB = promiseA.then(resolverB.fulfill, resolverB.reject, resolverB.progress);
resolverB.onCancel(aToB.cancel());

Since B has already been directly cancelled, it doesn't matter if we call resolverB.reject an extra time. The internal workings of then must still be considered properly:

function then(cb, eb) {
  let {promiseB, resolverB} = defer();
  resolverB.onCancel(resolverA.cancel); //note here that we **indirectly** cancel ourselves
  //Normal then method stuff goes here
  return promiseB;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants