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

Background #1

Open
ForbesLindesay opened this issue Dec 16, 2012 · 18 comments
Open

Background #1

ForbesLindesay opened this issue Dec 16, 2012 · 18 comments

Comments

@ForbesLindesay
Copy link
Member

Some of the promises libraries now implement progress. The key thing that has yet to be decided upon is how we should deal with exceptions thrown in progress handdlers.

It has been broadly agreed upon that progress should be a single value (akin to fulfillment and rejection). We have also agreed that the third argument to then should be treated as the progress handler and should also controll propagation of progress.

I'd apreciate it if someone could fill in what the currently in use rules are regarding progress propagation.

@domenic
Copy link
Member

domenic commented Dec 16, 2012

Progress propagation: given

var promise2 = promise1.then(undefined, undefined, onProgress1);
var promise3 = promise2.then(undefined, undefined, onProgress2);
promise3.then(undefined, undefined, onProgress3);
  • The return value of onProgress1 is passed to the progress handler for onProgress2.
  • If onProgress2 is not a function, then the return value of onProgress1 is passed to onProgress3.

Other issues: does progress get replayed? What happens if you return a promise as your progress value? How should cases like this be handled?


Another piece of background: many people believe progress should not belong to promises at all. E.g. promises should just be event emitters and fire off progress events, or allow them to be dispatched. This introduces two difficulties, in my mind:

  1. It fails to separate the "notifier" role from the "listener" role. I.e. it should be the resolver's responsibility to trigger progress events, while it's the promise's responsibility to provide a hook for listening to them. Most event emitter implementations (e.g. EventEmitter in Node or EventTarget in the DOM) conflate the two.
  2. It requires manually propagating progress, negating much of the use of promises and leading to ugly code.

If neither point is addressed, it turns code like this (which currently works in Q)

function getUser() {
   return doJsonXhr("/user").then(function (res) {
      return res.user;
   });
}

into this code:

function getUser() {
  var p = doJsonXhr("/user");

  var derived = p.then(function (res) {
    return res.user;
  });

  p.on("progress", function (x) {
    p.emit("progress", x);
  });

  return derived;
}

If only point 1 is addressed, but point 2 is not, the code is much worse than even that.

@ForbesLindesay
Copy link
Member Author

How do you stop progress propagation in your first example? You could deliberately address 2 and not 1 by having a simple method to do straight forward direct propagation within then by passing true as the third argument. You'd then let people do the more complex event emitter stuff when they needed to handle more complex stuff like fast then slow.

@domenic
Copy link
Member

domenic commented Dec 16, 2012

To stop progress propagation, add a progress handler that returns undefined.

@domenic
Copy link
Member

domenic commented Dec 16, 2012

Hmm, actually, that won't stop propagation, it'll just propagate undefined instead of anything useful. Good point.

@ForbesLindesay
Copy link
Member Author

I'm in favour of also making undefined not propagate. I think it would be nice to have a value that succinctly says "don't propagate"

@domenic
Copy link
Member

domenic commented Dec 16, 2012

Reminds me of StopIteration from ES6. Maybe throwing a distinguished error would make more sense there?

@ForbesLindesay
Copy link
Member Author

Yeh, throwing an exception would work, but it may be difficult to distinguish the right exception. An instanceof check would prevent using them interoperably

@briancavalier
Copy link
Member

I can't see any reason not to propagate undefined, especially since it propagates like any other value for fulfill/reject. I do see the value in having a StopIteration like value, but I also see the problems around interop there since it's not something that the language can provide right now ... unless we were to actually use StopIteration, which would require a polyfill, and just feels wrong.

Throwing seems reasonable, but raises the question of what to do with the thrown value. It seems like that may be a candidate for a promise debugger hook similar to the stuff we're discussing in unhandled-rejections-spec land

@ForbesLindesay
Copy link
Member Author

How about instead of using instanceof we add a .code property to the error with a value of 'StopProgressPropagation'. By just checking for that would allow all libraries to use other libraries' exceptions interchangeably. Then it could be upgraded to an instanceof check if it gains language support.

@domenic
Copy link
Member

domenic commented Dec 17, 2012

@ForbesLindesay or .name, to match existing errors.

@ForbesLindesay
Copy link
Member Author

I'm not at all fussed what we call the property, I thought node.js used .code a lot elsewhere? I'm happy to use .name if people prefer.

@briancavalier
Copy link
Member

afaik, .name is what builtins like TypeError use. If we go with a particular exception name, then an implementation will assume that whatever thing it catches is indeed an error, compare it's .name to the "official" name, and if it matches, silently stop progress propagation? That seems reasonable.

Would it also be reasonable to return one of these "stop propagation" objects instead of throwing it?

What about other exceptions? Should we assume that we'll be able to rely on some debug hook? Crash the app with a next-tick throw?

@domenic
Copy link
Member

domenic commented Dec 20, 2012

Let's not forget we can open more than one GitHub issue for the separate issues involved ;)

@juandopazo
Copy link

I haven't been exposed to that many situations involving promises as you have, so my thoughts are probably missing something.

I can think of "progress" in two terms: progress steps taken during the life of a promise and progress through a promise chain. I'll exemplify:

request(url).then(function (html) {
  // HTTP sends chunks which correspond to a progress in the HTTP request 
});

// there are three steps here that constitute progress
async1().async2().async3().then(function (result) {
});

I don't think progress in the sense of the first example has a place in a promises implementation. In the world of asynchronously calculated values we've seen three major approaches:

  • Callback passing style
  • Promises
  • Streams

The interesting part is that complexity increases with the type of approach chosen. A callback only requires understanding of scopes and asynchronicity. Promises require understanding of asynchronicity and control flow using objects. Streams require understanding the previous knowledge and the granularity of events, pausing, resuming, etc.

I think that intermediate states belong to Streams and that Promises should remain boolean, with only a fulfilled or rejected state. This will be easier to communicate, teach and maintain. As you're already showing, more granular interaction with promises starts to delve in the realms of event emitters which in a way defeat the purpose of promises: a lightweight object representing the state of an asynchronous transaction. I also think more complexity will be unhelpful when trying to get native providers (DOM, WinJS, Node, etc) to adhere to a standard.

In the case of the second example, I haven't run across an implementation that captures the progress of the promise chain. Does it need any special considerations or is it doable with just then?

@ForbesLindesay
Copy link
Member Author

I disagree. Streams have complexity mostly because of their desire to have a super low memory footprint and be really efficient. They were never meant to simplify things in the way promises are. I often use helpers which do things like: stream a file from a web-server to the local file system. That has a boolean granularity, so I want to return a promise for it for simplicity, but I might still want to display progress to the user. I'd describe that as UX (user experience) progress.

In the chaining progress option, it could be for two reasons. Their might be something I can do with the partial results, or I might simply want to display what stage of the operation I'm at.

I've not actually had occasion to use the first (UX) type of progress, but I can see it's value. It is especially valuable in client-side JavaScript where you might do a file upload (boolean granularity with UX progress). component/upload is a good example of a library that uses this kind of progress (it's not a real promise, but it's trying to be a promise).

I do have an occasion to use the second type of progress, where I am able to do stuff with partial results. It definitely needs special consideration, because I have one method which makes total sense and does a series of about 20 things one after another. It only ever makes sense to call in that way, but sometimes I'll want to display the intermediary results, so I need progress.

@juandopazo
Copy link

Should I understand the lack of other answers besides @ForbesLindesay's as consensus?

Personally I have to say I'm not very interested. Promises as defined in A+ offer a lot of value with a very small code footprint. That allows us to incorporate it into libraries like jQuery or YUI without increasing the KB weight by as much as a fully featured event system. Also, like I said before, the smaller it is, the easier it is to teach, learn and evangelize.

@domenic
Copy link
Member

domenic commented Dec 29, 2012

I am somewhere in between. Ideally I think a promise for a stream is the way to go, e.g.

sendRequest.then(function (response) {
   response.on("progress", function () {});
   response.readToEnd().then(function (allData) {
   });
});

I agree that progress is not that compelling, but, on the other hand, people do use it in the real world---mostly for UI programming on the client side. And having an interoperable way to deal with it seems prudent, given its presence in most implementations these days.

@sebarina
Copy link

does promisekit support progress now?

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

5 participants