cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

Initial prototype for AbortablePromise #415

Open briancavalier opened 9 years ago

briancavalier commented 9 years ago

DO NOT MERGE: This is a prototype of an AbortablePromise, for discussion and iteration to prove the idea.

This adds a new AbortablePromise subtype that has an abort() method which can send a signal from a consumer back toward the root of the promise graph to the producer indicating that the consumer wishes to abort the task that is computing the promise's value. This can be used to stop long-running or resource-consuming tasks when a consumer decides it no longer wants the result. For example: XHR request.

Initial prototype

The initial implementation isn't ideal but is headed in the right direction. The abort functionality should be provided at the handler level, and a thin wrapper for it provided at the AbortablePromise.prototype level. The initial prototype does that, but by stashing an additional property on the existing handler. Ideally, we would have a new type of handler, eg AbortableHandler, that provided the functionality.

Open questions

This prototype doesn't provide AbortablePromise.resolve or AbortablePromise.reject. I believe that it will need to. The reason is for API consistency: APIs that return AbortablePromise may need to return resolved and/or rejected promises, and returning Promise.resolve/reject would return a promise without an abort() method, which would break callers of those APIs.

Promise.all and Promise.race may also need abortable analogs that return an AbortablePromise. They would also need to handle abortable thenables in their input arrays, aborting them when the returned promise is aborted. That may be a slippery slope into needing to detect abort in many more places (when.map, when.filter, when.reduce, etc etc etc).

cc @mjackson

briancavalier commented 9 years ago

Simple test program here: https://gist.github.com/briancavalier/f606b6af9b35855c092f

mjackson commented 9 years ago

This looks great @briancavalier ! Thank you for taking this on.

This prototype doesn't provide AbortablePromise.resolve or AbortablePromise.reject. I believe that it will need to. The reason is for API consistency

It seems like if AbortablePromise does everything that Promise does we'll end up with a fairly complete parallel Promise implementation, just with an abort method on the prototype.

If closing over things as I did in my initial hack isn't an option, and I agree that it may not be due to performance reasons, then it may make more sense to evolve Promise instead of creating something new. Yes, it would break things. But that's why we have major version numbers ;)

briancavalier commented 9 years ago

It seems like if AbortablePromise does everything that Promise does we'll end up with a fairly complete parallel Promise implementation, just with an abort method on the prototype.

Yeah, and that'd be a bummer. The real issue is all the other useful operations around promises which aren't currently abort-aware. If abort is not a first-class citizen, then literally every API that wants to take advantage of abort would either have to fail-fast when passed non-abortable promises (not really an option!), or branch based on the presence/absence of abort. That's a slippery slope, since adding the next new one-off feature would mean having to add another branch. Which is why ...

it may make more sense to evolve Promise instead of creating something new

... I'm starting to think that's actually the better option, too :) Then all promises (for now, all when.js promises) will have an abort method. For Promise, it would basically be a noop. That'd avoid lots of sniffing around for abort methods, and avoid creating a complete, parallel AbortablePromise implementation

mjackson commented 9 years ago

For Promise, it would basically be a noop.

For resolved promises, yes. But maybe unresolved promises should reject?

briancavalier commented 9 years ago

But maybe unresolved promises should reject?

I think vanilla Promise rejecting would grant too much authority to consumers by default: any consumer could force any promise to reject, potentially affecting other (unsuspecting) consumers in the same promise graph. What's nice about AbortablePromise being a separate concept is that the producer can choose to grant that authority or not--in fact, it forces the producer to make an effort to grant it by choosing AbortablePromise over Promise.

I'd def like to have a go at another prototype where all promises have .abort(), Promise makes it a noop, and AbortablePromise implements the abort functionality. Not sure when I can get to that, tho ... might be next week, or might be sometime after Dec 28.

Please do feel free to hack on this branch if you have ideas :)