promises-aplus / progress-spec

Discussion and drafts of a possible promise progress signalling spec
6 stars 1 forks source link

Draft A #5

Open ForbesLindesay opened 11 years ago

ForbesLindesay commented 11 years ago

This has lots of holes in it, I'm putting it here though, because I'm hoping that we'll start to see where the holes are and fill some of them in a little.

Progress/A+

This proposal specifies how progress is triggered and propagated in a PromisesA+ promise library.

Requirements

The then method

The then method is the same as that specified in the promises-spec except that it also takes a third argument. We'll call that third argument the onProgress.

The onProgress

  1. If the onProgress is not a function
    1. It is ignored
  2. If the onProgress is a function
    1. When progress is emitted, the onProgress is called with the ProgressValue as the first argument.
    2. If the onProgress throws an exception with a .name property equal to 'StopProgressPropagation' then the error is silenced and progress is not propagated. In all other cases, the result of the function is used as the progress value to propagate.
    3. onProgress is never called once a promise has already been fulfilled or rejected.

      The progress method

The resolver has a .progress(value) method. This triggers all the onProgress methods. It returns a promise which is fulfilled with undefined once all progress methods are complete or is rejected with the first (non-StopProgressPropagation) exception thrown by the handlers, if any.

novemberborn commented 11 years ago

What is supposed to happen when progress is propagated, and a propagated handler throws? Is the promise returned by the first resolver dependent on the results of all propagations, or only of the handlers it directly knows about?

novemberborn commented 11 years ago

This draft doesn't mention anything about emitting progress after the resolver has been fulfilled/rejected. AFAIK current implementations stop emitting progress at that point.

novemberborn commented 11 years ago

WIP implementation at https://github.com/novemberborn/legendary/compare/progress. See examples.

domenic commented 11 years ago

Some minor nits:

This spec doesn't include any of our discussion on propagation.

ForbesLindesay commented 11 years ago

I'm inclined to say it should depend on all the propagated progress, not just that first one, or we have to find another way to display those errors.

ForbesLindesay commented 11 years ago

@domenic I know this is pretty seriously lacking, I just thought a draft (even if it is a bit vague/unfinished) might help the discussion move forwards, I'll try and improve it when I get time.

novemberborn commented 11 years ago

I've now implemented this draft in Legendary 0.2.0, including tests. I did reword the draft a bit, and it's hard to collaborate on that unless we can create a wiki page for it. Anyway, I'm including my version below.


Progress/A+

This proposal specifies how progress is triggered and propagated in a PromisesA+ promise library.

Requirements

The then method

The then method is the same as that specified in the promises-spec except that it also takes a third argument. We'll call that third argument the onProgress.

  1. onProgress is an optional argument:
    1. If onProgress is not a function, it must be ignored
  2. If onProgress is a function:
    1. It must be called after progress is emitted, with the progress value as its first argument.
    2. Unless the onProgress callback throws an exception with a .name property equal to 'StopProgressPropagation', the result of the function is used as the progress value to propagate.
    3. If the onProgress callback throws an exception with a .name property equal to 'StopProgressPropagation', then the error is silenced.
    4. onProgress is never called once a promise has already been fulfilled or rejected.

The progress method

  1. The resolver has a .progress(value) method.
  2. This triggers all onProgress callbacks.
  3. It returns a promise which is fulfilled with undefined once all progress callbacks are complete,
  4. or is rejected with the first (non-StopProgressPropagation) exception thrown by the callbacks, if any.
ForbesLindesay commented 11 years ago

I think we need to make it clear that onProgress callbacks can be asynchronous too. This means that the result could include rejected promises, as well as thrown exceptions.

novemberborn commented 11 years ago

I think we need to make it clear that onProgress callbacks can be asynchronous too. This means that the result could include rejected promises, as well as thrown exceptions.

You mean when they're propagated? I suppose that makes sense. It'd also mean that propagation waits until the returned promise fulfills.

novemberborn commented 11 years ago

If onProgress callbacks can return promises, and the progress is only propagated when those promises resolve, then what happens if the progress value itself is a promise? If that exact same value is returned by the callback I think it should be propagated as-is.

ForbesLindesay commented 11 years ago

I wasn't really thinking we'd support a promise as the progress value, if we are then we need to think about how that's done. Is promises as progress values something anyone's using in the wild? @domenic ?

novemberborn commented 11 years ago

I'd be fine with not supporting that, but then we'd have to decide whether it's silently ignored or if it's an error.

ForbesLindesay commented 11 years ago

Neither, I'd say it get's fully resolved. i.e. you can call resolver.progress(promise) but it's equivalent to calling:

promise.then(resolver.progress);
novemberborn commented 11 years ago

Works for me.

novemberborn commented 11 years ago

Implemented those changes in Legendary 0.3.0, with an updated spec at https://gist.github.com/4400252.

domenic commented 11 years ago

Looks pretty nice to me, although of course what exactly "propagation" means is underspecified.

I think what's most curious now is how to spec the various scenarios you can get by combining weird stuff like propagation and notification with a promise. E.g. this simple fast/slow case, but you can think of much more, especially now that you've added more tools.

I think the way to do that is to see how it behaves in Legendary (and, in the simpler cases not making use of the newer tools, in other libraries like Q, When, and jQuery). Then ask, is that how it should behave? Open an issue at this point to keep us informed. Then fix it until it behaves like you think it should. Once all those cases are behaving nicely, see if there's a set of words that describes exactly what behavior was settled on.

novemberborn commented 11 years ago

I think what's most curious now is how to spec the various scenarios you can get by combining weird stuff like propagation and notification with a promise. E.g. this simple fast/slow case, but you can think of much more, especially now that you've added more tools.

What do you have in mind as regards to specifying those scenarios? I've added an example of the fast/slow case and it behaves as expected: Progress is received from "fast" until it fulfills, then from "slow". Perhaps the spec could do with a section that explains how those scenarios would unfold?

I think the way to do that is to see how it behaves in Legendary (and, in the simpler cases not making use of the newer tools, in other libraries like Q, When, and jQuery). Then ask, is that how it should behave? Open an issue at this point to keep us informed. Then fix it until it behaves like you think it should. Once all those cases are behaving nicely, see if there's a set of words that describes exactly what behavior was settled on.

At this stage Legendary is just play, we'll eventually start using it at State but that won't be for a while. Ideally it'll also be the basis for promises in Dojo 2.0, but again that's some time away. My use cases have always been rather limited and this implementation supports them just fine.

I've opened #6 to collect use cases.