Feature: Documenting Promises

[pedronasser]

Problem All my projects now have functions that returns promises.

Idea How about some specific promise documentation?

Example

/**
 * @promise Get user from database.
 * @resolve {object} user information
 * @reject {Error} validation error, connection error
 */

Following this spec: http://promises-aplus.github.io/promises-spec/

[hegemonic]

I'm curious to hear from anyone else who has an opinion about the JSDoc syntax for documenting promises, or about what this should look like in the generated documentation.

In #429, @cowwoc suggested using the same syntax that we use for typedefs and callbacks (I've adapted it to refer to "promises" rather than "futures"):

/**
 * Some method.
 *
 * @method
 * @returns {Promise} A promise that returns {@link MyPromise~onResolve} if resolved
 * and {@link MyPromise~onReject} if rejected.
 */

[kylehg]

At Medium, it was common to use a syntax similar to objects and arrays. Although we typically only documented the resolved case (not the error case), I imagine something like this might work:

/**
 * Some method.
 *
 * @returns {Promise.<string, Error>} A promise that returns a string if resolved,
 *     or an Error if rejected.
 */

What do you think?

[phpnode]

I've been using a syntax that's basically identical to @returns

/**
 * @promise {Boolean} true if some condition is true
 */

If it's necessary to document the rejection error:

/**
 * @promise {Boolean} if some condition is true then true, otherwise false.
 * @rejects {Error} if some error occurs.
 */

I think it's pretty unambiguous, it'd be nice if JSDoc supported this. Don't really like the initial proposal of having 3 distinct tags, most of the time we just want one - @promise, which can be used in place of the @returns tag.

[cowwoc]

I'm not against the idea of 2 tags per-se but I dislike the names you chose. "A function @return X" sounds like proper English. "A function @rejects X" makes no sense. The function is not rejecting X. It's returning something that itself may return X on failure. That's not the same thing as the function itself doing so.

[phpnode]

@cowoc I actually agree that @rejects is not ideal, do you have any other suggestions?

[narqo]

@phpnode but how would you document function's parameter, which is a promise? E.g.

/**
 * @param {??} p
 * @returns {??}
 */
function doSomething(p) {
  return p.then(() => { ... });
}

In this case, I really like @kylehg's solution, where Promise is yet another (generic) type

[phpnode]

I'm not sure why we'd need to change anything for @param at all, if it's a promise it would look like:

/**
 * @param {Promise} p a promise.
 */

[narqo]

But it would be much clearer if one would have an ability to specify some details about this Promise

/**
 * @param {Promise.<String>} p my promise
 */
function doSomth(p) {
  p.then((str) => { str.substr(2) });
}

[phpnode]

@narqo how is a promise different to any other object which may encapsulate a value? How do you indicate the contents of a collection object for instance?

[narqo]

According to JSDoc3, you can use @typedef tag for this.

For me, since Promises are going to become a part of DOM (or ECMAScript?) standard, they should be documented the same way we document other embedded non-primitive types, like Array or Object.

[kylehg]

+1 @narqo

[kennknowles]

+1 @narqo

This should simply be generics. Promise is a type that is parameterized by another type. It is decades old, no need to invent things.

[phpnode]

@kennknowles it's a common enough pattern that it warrants its own tag. I think a lot of the points here, especially regarding using @typedef, miss the point - that if you want people to use this, you have to make it easy for them. using a syntax like:

/**
 * @promise {Boolean} if foo then true
 */

is easy, unambiguous and covers 95% of what people need. It's also much nicer to read than:

/**
 * @return {Promise.<Boolean>} if foo then true
 */

Most people will not document the error case, but it could be something like:

/**
 * @promise {String} the name of the thing
 * @fail {Error} If something went wrong.
 */

Passing promises as function arguments is not a common pattern, so, don't optimise for it. In such a case, the promise is just an object like any other.

[kennknowles]

@phpnode Why not both? I'm completely fine with adding special tags to make common cases easy. But right now there's no underlying foundation to build the tag on, so I can't even do it the way you dislike. (It may not be pretty, but it has the benefit of being obviously correct and well understood.)

BTW I actually don't "want people to use this". I am a mere user of jsDoc, just casting my vote for an approach that would actually solve it for me.

[cowwoc]

For what it's worth, I'm in favor of some mechanism which encourages users to specify both the success and failure value. For this reason, I believe @return and other tags meant to denote a single value are inappropriate.

The way I see it, Promises @return and @throws but asynchronously. I leave it up to you to decide how to represent that, but I will point out that being able to specify multiple @throws allows the documentation to scale in case different kind of errors are thrown.

[phpnode]

@kennknowles sure, being able to do it the more verbose way as well would be great, using @promise is just sugar for the same thing. I think we all benefit from better documentation :)

[phpnode]

@cowwoc multiple @fail tags could be supported, just like @throws:

/**
 * Make a request
 *
 * @promise {Object} the result of the request
 * @fail {ConnectionError} if the connection fails
 * @fail {RequestError} if the request is invalid
 * @fail {Error} if something else went wrong
 */

[cowwoc]

@phpnode I'm okay with the concept of using multiple tags but I'm not sure about the naming. Perhaps we should use names that mirror the terminology used in the Promises API: @resolves and @rejects similar to how @returns and @throws mirror the JS constructs return and throw.

[bernhardw]

I'm 100% for using @resolves and @rejects (multiple), as @cowwoc suggested to conform with the standard terminology. Anything else confuses and gives no advantage.

[hegemonic]

Thanks to all of you for contributing your ideas!

I have to say that I agree with @cowwoc's comment earlier in this discussion:

"A function @rejects X" makes no sense. The function is not rejecting X. It's returning something that itself may return X on failure. That's not the same thing as the function itself doing so.

Also, note that the draft ES6 spec uses the terms "fulfilled" and "rejected" to describe potential states of a promise. As the spec says, you can "resolve" a promise by rejecting it, or even by locking it into a "pending" state.

So far, the idea I like best is the suggestion from @kylehg, but with a type union in the generic:

/**
 * Retrieve the user's favorite color.
 *
 * @returns {Promise.<string|Error>} The user's favorite color if fulfilled,
 * or an error if rejected.
 */
User.prototype.getFavoriteColor = function() {
    // ...
};

(You could also write the type expression as {Promise.<(string|Error)>}, with parentheses around the type union.)

I'm not 100% sold on this syntax, but it seems to me that it offers some advantages over a new tag (or two):

1. It emphasizes that a Promise is a value returned by a function, not a flow-control keyword such as return or throw.
2. It's easy to understand after you've seen it once or twice.
3. JSDoc already supports it! (Really! Try it!)
4. It doesn't appear to break Closure Compiler projects.
5. It's concise.

This syntax also has some disadvantages:

1. It may not make sense the first time you see it.
2. You have to use one description for both the "fulfilled" and "rejected" types. (I'm not sure this is a bad thing, but some people might not like it.)
3. At a programmatic level, it's trickier to extract the "fulfilled" and "rejected" types. (I'm not sure whether this matters, but it might matter to someone.)

[cowwoc]

@hegemonic Your syntax does not scale (it does not support Promises that return multiple error conditions). From a readability point of view, I prefer comment 2 for a solution that's not meant to scale. Besides which, a Promise's return value on success and failure is not really a union. They do not come out of the same callback.

I suggest focusing on a solution that scales first, and providing convenience tags for the more basic case later.

For the scalable syntax, I'm thinking of something like this:

/**
 * Retrieves the employee's state.
 *
 * @return {MyPromise} the asynchronous result
 */
function loadEmployee()
{}

/**
 * @typedef {Promise} MyPromise the result of loading an employee's state asynchronously
 * @return {string} the employee's name
 * @throws {TypeError} if the user is not an employee
 * @throws {ResourceNotFound} if the user was not found
 * @throws {ServerNotAvailable} if the server is busy
 */

I'm not sure whether it makes sense to use @return and @throws for a Promise, but I personally like it. It's as if we're saying that a Promise is like a callback that has already been invoked.

[phpnode]

@cowwoc that's really horrible IMHO, it's not DRY it's confusing and way too easy to omit or miss the @typedef. What happens if the function throws synchronously as well?

[cowwoc]

@phpnode It is DRY: returning a Promise and the Promise itself returning a value is not the same thing. It is also possible for multiple functions to return the same Promise.

If the function throws synchronously as well you end up with this:

/**
 * Sets the employee's name.
 *
 * @param {string} name the employee's new name
 * @return {MyPromise} the asynchronous result
 * @throws {TypeError} if arg is not a string
 */
function setName(arg)
{}

There you go. Now the function throws synchronous exceptions on client-side errors and the Promise returns exceptions on server-side errors.

[helmus]

+1 for using @return and @throws for documenting promise resolution / fails. The concept of throwing and returning are the same in promises only asynchronous.

It should always be possible to document the different async throws and so it makes a lot of sense to use the same semantics.

I do feel that it would make sense to have some sort of inline style as well. This might be usefully for asynchronous functions that never trow or only resolve for timing purposes.

Example:

function delay(seconds){
    return new Promise(function (resolve) {
        setTimeout(resolve, seconds * 1000);
    });
}

I'm sure there are more use cases for this and it might be impracticable having to write a @typedef every time.

[hegemonic]

I don't think @returns and @throws are the best solutions here, just as I wouldn't document function callback(err, value) {} with @returns {string} value and @throws {Error} err. The concepts are similar, but the code you write to handle a promise is quite different from the code to handle a synchronous method call.

The scaling concern that @cowwoc raised could be addressed through a hybrid of @kylehg's suggestion and my modified version:

/**
 * Retrieve the user's favorite color.
 *
 * @returns {Promise.<string, (TypeError|MissingColorError)>} The user's favorite color
 * if fulfilled, or an error if rejected.
 */
User.prototype.getFavoriteColor = function() {
    // ...
};

That said, I have another concern about this syntax: Because it abuses the syntax that other languages use for generics, it could be confusing or off-putting to developers who've used generics in other languages.

All of the "use @typedef" comments are hinting at another solution, which is to make @promise a synonym for @typedef {Promise}. This is very similar to our implementation of the @callback tag (see issue #260):

/**
 * A promise for the user's favorite color.
 *
 * @promise FavoriteColorPromise
 * @fulfill {string} The user's favorite color.
 * @reject {TypeError} The user's favorite color is an invalid type.
 * @reject {MissingColorError} The user has not specified a favorite color.
 */

/**
 * Retrieve the user's favorite color.
 *
 * @returns {FavoriteColorPromise} A promise for the user's favorite color.
 */
User.prototype.getFavoriteColor = function() {
    // ...
};

This syntax is extremely clear, and as I said, it's pretty consistent with how @callback works. It also allows you to document every single type for a resolved promise. But it won't please people who want the syntax to be as concise as possible.

[greggman]

So I'm trying to document a promise I just made and found this thread. It seems like more concrete examples would be good. For the example below at least it seems like the documentation for the Promise needs to be separate from the function that returns a Promise.

var Promise = require('Promise');
var child_process = require('child_process');

/**
 * @callback ExecResultCallback
 * @param {string} stdout the contents of stdout from the process
 * @param {string} stderr the contents of stderr from the process
 */

/**
 * @promise ExecPromise
 * @fulfill {ExecResultCallback} 
 * @reject {???}
 */

/**
 * @typedef {Object} ExecOptions
 * @property {string?} cwd Current working directory of the child process
 * @property {Object?} env Environment key-value pairs
 * @property {string?) encoding (Default: 'utf8')
 * @property {number?} timeout (Default: 0)
 * @property {number?} maxBuffer (Default: 200*1024)
 * @property {string?} killSignal (Default: 'SIGTERM')
 */

/**
 * child_process.execFile that uses promises.
 * @function
 * @param {string} cmd command to execute
 * @param {string[]} args arguments to command
 * @param {ExecOptions?} options
 * @returns {ExecPromise} 
 */
var execFile = Promise.denodeify(child_process.execFile);

/**
 * child_process.exec that uses promises.
 * @function
 * @param {string} cmdline command and args to execute
 * @param {ExecOptions?} options
 * @returns {ExecPromise}
 */
var exec = Promise.denodeify(child_process.exec);

[hegemonic]

Unless there are any last-minute objections, I'm going to implement the syntax that I proposed in my last comment, including the new @promise, @fulfill, and @reject tags:

/**
 * A promise for the user's favorite color.
 *
 * @promise FavoriteColorPromise
 * @fulfill {string} The user's favorite color.
 * @reject {TypeError} The user's favorite color is an invalid type.
 * @reject {MissingColorError} The user has not specified a favorite color.
 */

/**
 * Retrieve the user's favorite color.
 *
 * @returns {FavoriteColorPromise} A promise for the user's favorite color.
 */
User.prototype.getFavoriteColor = function() {
    // ...
};

@greggman: We'll have more examples once the syntax is finalized.

[phpnode]

@hegemonic still think this is way too verbose and no one will use it as a consequence. Your proposal is completely impractical for code which makes heavy use of promises. It's also misleading, the example is not returning a FavoriteColorPromise instance. It's returning a Promise instance.

[hegemonic]

@phpnode, I totally understand your concerns about verbosity. You can always write the following instead, as long as you don't care about documenting the error case:

/**
 * Retrieve the user's favorite color.
 *
 * @returns {Promise.<string>} A promise that contains the user's favorite color
 * when fulfilled.
 */
User.prototype.getFavoriteColor = function() {
    // ...
};

I also hear what you're saying about the fact that FavoriteColorPromise is not a real type. But the @typedef and @callback tags follow that same model, so I'm afraid the ship has sailed already.

[cowwoc]

@phpnode, I actually disagree with you. The function does return the aforementioned the promise. The fact that the promise eventually returns another value doesn't change that fact. You have no predicting (promising to the user) what asynchronous value the function will return because someone could chain a followup promise which returns a different value than you expected. The only thing we can and should be documenting is the synchronous result, as @hegemonic is suggesting.

[phpnode]

You have no predicting (promising to the user) what asynchronous value the function will return because someone could chain a followup promise which returns a different value than you expected

@cowwoc do you write a lot of idiomatic promise driven code? I don't think you'd have said this if you do. A chained promise creates a new promise object, it does not mutate the original promise. So the promise returned by a function will always yield the same value.

The fact that a promise is being returned is not particularly interesting when almost all your code returns promises. The interesting point is what that promise resolves to. That's why I still think that this syntax is the most concise option:

/**
 * @promise {Boolean} if something is true then true.
 */

which will be sufficient for probably 95% of the cases out there. If you really need to document the failure mode (vast majority of cases don't even have a failure mode to document), then use @fail

/**
 * @promise {Boolean} if something is true then true.
 * @fail {TypeError} if something went wrong
 * @fail {OtherError} if something else went wrong
 */

It's much easier to reason about code when you can say "this function promises a Boolean", rather than "this function returns a Promise which encapsulates a Boolean". The return value is not interesting.

[cowwoc]

A chained promise creates a new promise object, it does not mutate the original promise. So the promise returned by a function will always yield the same value

@phpnode Sorry, good point.

My only concern with your syntax is whether it scales. What happens if Promise A returns Promise B which returns Promise C? I assume you're saying that we should document the final return value of C and all error conditions returned by A, B and C. Is that correct?

That could work so long as the Promise really does nothing else but fulfill and reject (meaning, you didn't add user-defined properties to it for some reason). Does anyone see a problem with this approach?

[phpnode]

@cowwoc we'd only document the final return value and yes any errors that could be caused directly by the function itself.

/**
 * Load a user.
 * @param   {Number} id   The id of the user to load
 * @promise {User}        The loaded user instance.
 * @fail    {InputError}  If the id is invalid.
 */
function getUser (id) {
  if (typeof id !== 'number') {
    return Promise.reject(new InputError('A user id must be specified'));
  }
  return this.database
  .get(id) // this could reject with any number of failures, we don't document them here
  .then(function (result) {
    return new User(result);
  });
}

[kflynn]

New here but, writing a bunch of Promise-based code, I so very much agree with @phpnode on this one. I'd love to see @promise and @fail.

(I personally would be fine with @rejects, also: I've caught myself saying things like "this function rejects with such-and-such on failure" as a convenient shorthand. But @fail is fine with me.)

[martijndeh]

I don't like @fail. I'm trying to avoid the fail keyword as it seems Kris Kowal's Q proprietary. Fail, as a method, is not supported by angular's $q nor petkaantonov's Bluebird (I think?).

I would opt for @resolve, @reject and @notify to be consistent with promise terminology (the deferred API).

[maciasello]

What about function parameters that can be promises as well? For now only the typedef and generics approaches cover that. I agree that for simple cases simple notations are valuable, but let us not provide this feature partially (for returning promises only).

[maciasello]

+1 for @promise analogous to @callback

[toddbluhm]

I'm sure I am late to the party, but having used promises all over my code I too would love for a simpler and more concise way of writing promise docs.

I think @phpnode is the best I have seen so far after reading through this thread.

Yes I think that the @typedef/@callback style is more flexible, but I am not going to write all of that for every unique promise I have. That is way too much for simple promise returns. For complex promise returns or promises as params, sure I will write verbose, but for that vast majority of my simple promise returns, @phpnode way works best. Just my opinion since this is still open.

[pvdz]

Also late to the party, but why not keep to @param and @returns to stay consistent with current mechanics (they are args and return values, after all). And if need be, use optional @resolves and (one or more) @rejects to clarify certain behavior of a promise if need be, in a @property kind of way.

/**
 * @param {Promise<string, Error>} p
 * @resolves {string} p When it does something right
 * @rejects {TypeError} p When it does something wrong
 * @rejects {SuperError} p When it does something super wrong
 */
function f(p) {}

This also keeps it clear in case a function handles multiple promises (as parameter and return value), as this discussion mainly seems to concern itself with returning a promise, instead of accepting one.

It also seems like a more consistent experience in general.

[cowwoc]

@qfox

1. DRY: you're making me repeat which promise is returning an error or value.
2. How would you specify resolve/reject for return types? (there is no variable name to reference)

[phpnode]

Future versions of ECMAScript are going to introduce an async keyword, like this:

async function myFunc () {
  return true;
}

When invoked, async functions always return promises and capture any exceptions thrown and turn them into promise rejections. Async functions promise to return a value.

With this in mind, how do you document the return value of this function?

In my mind @promise is the only way forward, using @returns here would be incredibly misleading.

[pvdz]

@cowwoc

The clarification is optional and should probably often be omitted. If that's not the DRY you're talking about, please clarify which part you find repeating.

Return types is a different matter. But the same could be said about unclassified @resolves and @rejects (or @promise, @fail, etc). I tend to name my returns if it clarifies things, maybe a similar approach can be taken. Not sure if that breaks any existing syntactic rules of @returns, though.

@phpnode

You said it yourself, it returns promises. So you call that async function and you'll get a promise value back. Ok let me put it differently, how do you annotate a factory function? So a function returning another function? I see this as a very similar thing.

[phpnode]

@qfox when virtually every function in your codebase returns a promise, the fact that a promise is being returned is not interesting. What is interesting is the value being promised.

I really feel that a lot of people in this discussion are not actually using promises much. For example it's incredibly rare to pass promises as parameters to functions, yet almost all of the examples here focus on it.

What is important is that there is a conceptual difference between returning a value and promising a value. I said this upthread, but it's much easier to think of a function which promises a value than a function which returns a promise which encapsulates a future value.

FWIW I'm continuing to use @promise {SomeType} and it works well, it would be nice if jsdoc would support it.

[pvdz]

Just to clarify, these are similar ways in which I would use them, depending on whether certain things need clarification. Using a train catching analogue in an attempt to reduce the obviousness of DRY due to an artificial example.

/**
 * @param {Promise<Train, Error>} p Waits for you to catch the train
 */

/**
 * @param {Promise<Train, Error> p Waits for the train
 * @rejects {LateError} p When you miss the train
 * @rejects {DoorError} p When you get there and the train is there but doors are closed and train leaves as you touch the handle
 * @rejects {LostError} p When you can't find the train station
 */

/**
 * @param {Promise<Train, Error> p Waits for the train
 * @resolves {Train} p The value will contain itinerary, make, model, and applause for catching the train on time
 */

@phpnode your comment seems to project your own personal experience and style onto how others are using it. It feels short sighted to simply ignore promises as arguments. I've encountered them frequently like that. And I'm still curious how you annotate functions returning functions. Do you use @factory or just @returns {Function}?

[phpnode]

@qfox I don't think the doc examples you posted really make sense without any code to back them up, would you mind putting together a couple of functions to show when this is useful?

And I use @returns {Function}. For this kind of stuff I'm looking to provide practical information which helps developers reason about the codebase, so if it's ambiguous, I'll add a comment explaining what's going on. It's impossible and pointless to express all of the different scenarios via special @tags, so I don't even try. But promises are super common and now a core language feature, so they warrant their own shortcut.

[pvdz]

Sigh :)

/**
 * @returns {Promise<Store, Error>} dataStore
 * @rejects {NetworkError} dataStore
 * @rejects {AccessError} dataStore
 * @rejects {TimeoutError} dataStore
 */
function getDataStore() { /* some async stuff to setup a data store from network */ }

/**
 * @param {Promise<Store, Error>} dataStore See getDataStore
 */
function report(dataStore, key) {
  dataStore
    .then(function(d) { /* report the key somehow */ }
    .otherwise(function(e) { /* report the error, there may be bigger fish to fry */ }
}

/**
 * @var {Promise<Store, Error>} 
 */
var store = getDataStore();

// ... lots of sync/async code here, then at some random point like a console.log

report(store, 'importantKey');

You'd pass on the data store without having to know that it loaded and the report would happen as soon as the promise resolves. It's a bit of a contrived example, of course, but that's what you asked for :)

It'd be prettier if we could use sub-bulleted list for the resolve/rejects :) That would also prevent repeating the var name over and over. And solve the anonymous return value case. But it doesn't feel very jsdoc-ish.

@returns {Promise}
- rejects {NetworkError}
- rejects {TimeoutError}

[phpnode]

@qfox ok, I can see how this can be potentially useful in some very limited circumstances, but the example is indeed pretty contrived,

function reportSuccess () {}
function reportError () {}
getDataStore().then(reportSuccess, reportError);

anyway, there is no reason why @promise {Foo} couldn't be syntactic sugar for @returns {Promise<Foo>}, then everyone's happy.

[pvdz]

I'm mainly looking for a better way to annotate promises than just the ugly @returns {Promise} Resolves with something when A. Rejects when B. But I guess there's no consensus about that. Yet.

[pnstickne]

I think the real "issue" is that JSDoc only quasi-supports documenting "inline, one-off types", of any sort. An example of where it is quasi-supported is with @param {} x, @param {v} x.a, or @param {{name: string, age: integer}} - player information etc.

An example of an "inline, one-off type" may look like so and is effectively just a way of applying a "typedef" without creating an explicit type.

/**
* @param {Promise<string, Error>} p - generics are cool too
* @return {{
*   (hint: it's implicitly a @promise)
*   @resolves {string} p When it does something right
*   @rejects {TypeError} p When it does something wrong
*   @rejects {SuperError} p When it does something super wrong
* }} a promise for making toast
*/

And with re-envisioning of the player; note that additional such an expanding syntax also allows the collection of other (useful) type information that was previously lost. Also, I'm making a point - so don't bring up the previously mentioned way of doing this with multiple @param..

/**
* @param {{
*   @property {string} - full name in ML format
*   @property {integer} - age of the player, if they are an adult
* }} - player information
*/

(This could naturally be applied for N levels.)

Unfortunately, to introduce such changes is likely a non-trivial matter - and worse would be the lag from tools that "sorta" understand jsdoc.

[pvdz]

@pnstickne agreed. I tend to solve clarifying important properties of params with @property like so;

/**
 * @param {Object} options
 * @property {string} options.foo Stuff goes here
 * @property {string} options.bar And here
 */

That's why I figured a similar approach would work well for promises as well.