promises-aplus / constructor-spec

Discussion and drafts of a possible spec for creating and resolving promises
10 stars 4 forks source link

Single- versus multi-param factory #19

Closed domenic closed 11 years ago

domenic commented 11 years ago

This is somewhat of a rehash of #7, but it's in the context of the latest strawman (#18).

Basically, the choices for parameters to factory are:

  1. resolve + reject
  2. resolver + resolver.reject
  3. an object with resolve and reject properties

And the reason for choosing 2 or 3 over 1 is future extensibility.

domenic commented 11 years ago

I argue for 1, despite extensibility concerns.

briancavalier commented 11 years ago

I like 1 as well. Both resolve and reject can act as extension points as needed.

juandopazo commented 11 years ago

In YUI we're going for 1. It's really easy to teach and understand because it hides the fact that there's another object around there.

I like reject.cancel!

novemberborn commented 11 years ago

Let's do 1.

ForbesLindesay commented 11 years ago

I like 1. Your suggestion for cancellation misses the point slightly, you're shouldn't be doing cancellation from the resolver/factory, you should be handling it and canceling the underlying operation.

function get(url) {
  return new Promise(function (resolve, reject, progress, onCancelled) {
    var xhr = getXHRObject();

    onCancelled(function () {
      xhr.abort();
    });

    xhr.onProgress = progress;

    //other xhr stuff
  });
}

Personally, the way I see it, we're going to add progress to then, we may as well add progress and cancellation to this, that's only 4 arguments. I can't see any reason we'd ever need a fith, so 4 seems fine.

domenic commented 11 years ago

Seems like we have consensus on 1, so, closing.

@ForbesLindesay: I don't think adding progress to then is a foregone conclusion, given how progress is kind of a hack. We'll see. And reject.cancel() is meant to reject the promise with a cancellation reason; I think the API for cancelling the underlying operation would look more like new Promise(resolverFunc, onCancelled) as per https://github.com/promises-aplus/cancellation-spec/issues/7 or similar.

ForbesLindesay commented 11 years ago

I'd be fine with that too. The point I was making is that I don't think cancellation needs to be possible for anyone who's not got access to the promise, so they can just do promise.cancel(). Anyway, those are separate issues for discussion elsewhere.