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]

Unlocked by request. If you add a comment, please keep it on-topic.

[ExE-Boss]

In VS Code and TypeScript, it’s possible to use @return {Promise<T>}, where T is the type of the resolved value.

[hegemonic]

@ExE-Boss That’s also the recommended syntax in JSDoc.

[brettz9]

I see it is used in an example, but might it be documented on the types page?

[xmedeko]

What about to automatically turn @async functions to Promises?

/**
 * @async
 * @return {string}
 */
function asyncFoo() { return Promise.resolve('test') }

And the generated doc would become @return Promise<string> ? Similarly for ES2017 async keyword.

[brettz9]

It seems one convoluted way to work around the lack of support in plain jsdoc for specifying rejectors is to use @interface itself have @class on it as well, such that, for example, one could avoid subclassing Promise yet indicate the constructor follows the signature of the constructor, e.g.:

/**
 * @callback SpecialResolver
 * @param {string|number|whatever} resolutionValue
 */

/**
 * @callback SpecialRejector
 * @param {TypeError|SyntaxError} rejectionValue
 */

/**
 * @callback SpecialExecutor
 * @param {SpecialResolver} resolve
 * @param {SpecialRejector} reject
 */

/**
 * @interface
 * @class SpecialPromise
 * @implements Promise
 * @param {SpecialExecutor} executor
 */

A standard tag for @rejects or an ability to pass in as a template (generic?) like Promise<MyResolver,MyRejector> would be far more succinct of course, but the above seems to work now in plain jsdoc.

[pinksynth]

I agree with @brettz9's recommendation of Promise<MyResolver,MyRejector>. The work done for Promise support is helpful, but it is effectively incomplete without rejection handling.

[brettz9]

FWIW, it seems TypeScript has such a proposal offered there too: microsoft/TypeScript#39680

[jimmywarting]

when a function is async then i wish i didn't have to type Promise<type>
i would just like to be able to write

/**
 * @return {string} some desc
 */
async function() {
  return globalThis.xyz
}

and the IDE would automatically translate it to Promise<string>

[kb-ig]

@jimmywarting I dont think that's really an option since promises dont have to be awaited. And a promise which is not awaited returns a promise.

E.g.

const asyncFn = async () => {
  return await Promise.resolve('Hello');
};

const notAwaited = asyncFn(); // returns Promsie<string>
const awaited = await asyncFn(); // returns string

[thw0rted]

Jimmy's point is that an async function always returns a Promise, so JSDoc could "infer" that oops you actually must have meant Promise<string>. That's technically correct but I'm not sure it's behavior I'd actually want. For example, older tooling would continue to say that the function returns string (not Promise<string>) -- changing the current behavior seems like a great way to wind up with confusing/conflicting output from different tools. I would absolutely support having any tools generate an error when it detects the mismatch, though. That's how Typescript works today. If you say an async function returns anything other than a Promise, the return type annotation is marked as an error (which, let's be honest, it is!).

[jimmywarting]

Jimmy's point is that an async function always returns a Promise, so JSDoc could "infer" that oops you actually

That was a bit what i was getting at... it sometimes feels too verbose sometimes to write Promise<string>

in my terminology: A async function is just a function that when called happens to wrap everything in a promise with the returned value invoked, no matter what you return.
the return value in this 👇 function isn't really a promise, it's actually a string that is being returned

/** @return {string} */
async function foo () {
  return "abc" // The return value is actually a `string` not a promise
}

/** @return {Promise<string>} */
async function foo () {
  // here we do actually return a promise, therefor you should
  // specify in the `@return` that you are actually returning a promise promise
  return Promise.resolve("abc")
}

I kind of look at both normal function and async function as two different kinds of functions that does different things
if you where to just slam in async in the beginning of a sync function then the return value is (in my opinion) still the same.
and the only thing that has been changed is the type of function you are using

(() => {}).constructor // ƒ Function() { [native code] }
(async () => {}).constructor // ƒ AsyncFunction() { [native code] }
(async () => {}).constructor === (() => {}).constructor // false

[thw0rted]

The thing that you pass to return is a string but the thing that the caller gets is, 100% of the time, a Promise. That return value may then get passed to the await keyword but it's immaterial. Try it yourself: foo().constructor === Promise; // true

ETA: I think we might be talking past the point here. The audience for JSDoc is the caller, first and foremost. It can be helpful for the implementer, I guess, but you're documenting it for whoever is going to call the function, and that's why the actual output (always a Promise) is most important.

[jimmywarting]

yea, true... in the end you will always end up with a returned Promise<string> in the end when it's called. But ppl can also make distinct decision based on what type of function something is based on the constructor.

if (foo.constructor.name === 'AsyncFunction') {
  throw new Error("Your function need to be sync, cuz fs.readSync() don't expect a promise")
} else {
  return foo()
}

Therefore i also think it might be important to distinguish normal function from async functions as well.

kindof like:

/**
 * @returns {AsyncFunction}
 */
globalThis.xyz = function xyz () {
    return async () => {};
};

/**
 * @returns {Function}
 */
globalThis.xyz = function xyz () {
    return () => {};
};

if i write this kind of code then i would expect to see a error at the end of this snippet in my IDE

const foo = async () => {}
const AsyncFunction = foo.constructor

/** @param {AsyncFunction} fn */
function xyz(fn) {
    fn().then(console.log)
    //   ^-- the IDE should know that it returns a promise
}
// should roughly be equivalent to this typescript `async function xyz(fn: () => Promise<unknown>)`

// expect-error
xyz(() => {})

[dball]

I don't believe anyone's observed this yet, but the proposed workaround syntax, @return {Promise<T>} cannot distinguish between a function that rejects with an Error T and one that resolves with an Error T. Sure, this is something of an edge case, but it demonstrates the incompleteness of the jsdoc syntax.

More pertinently, perhaps — it does not catch the eye of the casual reader the way a specific, correct tag would, which is, imho, the entire purpose of unchecked type claims.

[thw0rted]

The problem of capturing reject-type is, who provides your Promise type definition? I don't have data to back this up, but I strongly suspect a large percentage of folks watching this issue are using JSDoc via TypeScript and/or VSCode integration. If your Promise type is coming from somewhere else, maybe it supports a second generic argument (as in #509 (comment) and following comments), but TypeScript does not. See microsoft/TypeScript#13219 (comment) , which probably means that microsoft/TypeScript#39680 is not happening any time soon. If your Promise type doesn't support strong types for rejection, the only way forward for JSDoc would be adding something like @rejects.