Tsdoc: Documenting promise return types

Created on 18 Sep 2018  Â·  4Comments  Â·  Source: microsoft/tsdoc

JsDoc has been discussing how to document Promise return types for some time.

Does tsdoc have any plans to support @resolves and @rejects attributes on methods which return Promise<T>?

general discussion

Most helpful comment

This seems pretty good to me:

/**
 * Send a request.
 * @param req - the `Request` object to be processed.
 * @returns a `Response` object containing the data.
 */
export async function sendRequest(req: Request): Promise<Response>;

The fact that it is an asynchronous function and thus returns a promise seems fairly obvious to someone reading the code or API docs. The documentation tool could easily recognize this from the type signature, and put a little async icon on the web site.

I guess I'm unclear what's the benefit of documenting async functions differently from regular functions.

Whereas I do understand why JSDoc might want this, since there's no type information:

export function sendRequest(req) {
  return Promise.resolve(new Response());
}

All 4 comments

Hmm.... Is that useful in TypeScript? The @resolves information seems to be already part of the type signature. For example, this function resolves to a string or a number:

export function doSomething(): Promise<string | number> {
  return Promise.resolve(123);
}

The @rejects information seems like it wouldn't be very interesting, since in my personal opinion 😊, it should always be an object that implements the Error contract:

export function doSomething(): Promise<string | number> {
  // This makes everyone's catch blocks much more complex,
  // with zero practical benefit:
  // return Promise.reject('Something went wrong'); // BAD CODE

  return Promise.reject(new Error('Something went wrong')); // GOOD CODE
}

Beyond that, I suppose it might be useful to specify the possible kinds of Error objects. For example, in the Java language there is a throws keyword:

public static void checkPassword(String password) 
  throws ArgumentNullException, BadPasswordException {

  if (password == null)
    throw new ArgumentNullException("The password cannot be null");

  if (password.length() < 5)
    throw new BadPasswordException("The password is too short");
}

This idea is not specific to JavaScript promises, however. It would apply to synchronous functions as well.

In my personal opinion again 😊, Java's throws keyword turned out to be fairly cumbersome in practice. Most other languages chose not to adopt it. I can think of a couple possible reasons:

  • Most functions have unlimited possible errors: The big insight of exception handling is that most functions never need to catch exceptions. Instead, they just pass them along to their caller. But since Java's throws is a transitive constraint, those functions end up declaring the union of all possible exceptions for every library function that they call, which quickly defeats the purpose of writing throws clauses.

  • People avoid "unexceptional exceptions": If your program throws lots of exceptions during normal execution, this can hurt performance (because call stack unwinding is expensive). Also, it ruins the ability to debug using the "break on exceptions" feature. So for commonplace tests, people tend to choose to return a value rather than throwing an error. They only use exceptions for unexpected "exceptional" situations (e.g. the user gave a bad input, an expected file couldn't be found on disk, etc). This means that in practice, catch blocks usually don't care what kind of exception they are catching. Thus, listing every possible exception often turns out to be not very useful to people who call your API.

That said, maybe the goal isn't to enumerate every possible exception like Java does. Instead, maybe we're documenting a API and need a way discuss some specific exceptions that are interesting. In that case, we could probably reuse JSDoc's @throws tag. Although it was meant for synchronous exceptions, that shouldn't cause any ambiguity, because an async function should never throw a synchronous error. I won't call this a personal opinion, because there's a fairly compelling proof:

export function doSomething(): Promise<string | number> {
  // throw new Error('Something went wrong');  // BAD CODE
  return Promise.reject(new Error('Something went wrong')); // GOOD CODE
}

// Why it's bad:
export function doAllThings(): Promise<void> {
  // If doSomething() were to throw a synchronous exception, then doSomethingElse()
  // would become a dangerous unterminated promise chain:
  return Promise.all([doSomethingElse(), doSomething()]);
}

So, I probably could have used better language here--the idea is to be able to document the meaning of the resolved value much like the <returns> doc element in C#--enough type information is generally given by the TypeScript return type.

I feel this is complicated slightly by the advent of async functions. i.e.

/**
 * @returns a response containing the something-or-other.. insert meaning here
 */
async function sendRequest(req: Request): Promise<Response>;

This seems pretty good to me:

/**
 * Send a request.
 * @param req - the `Request` object to be processed.
 * @returns a `Response` object containing the data.
 */
export async function sendRequest(req: Request): Promise<Response>;

The fact that it is an asynchronous function and thus returns a promise seems fairly obvious to someone reading the code or API docs. The documentation tool could easily recognize this from the type signature, and put a little async icon on the web site.

I guess I'm unclear what's the benefit of documenting async functions differently from regular functions.

Whereas I do understand why JSDoc might want this, since there's no type information:

export function sendRequest(req) {
  return Promise.resolve(new Response());
}

Well put-- this issue mostly came up for me as part of an effort to understand the potential/purpose of @resolves in doc comments for some TypeScript code I maintain. Probably the way forward will be to delete the @resolves and put any meaningful information that was in the @resolves into a @returns.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

aciccarello picture aciccarello  Â·  12Comments

octogonz picture octogonz  Â·  7Comments

stacey-gammon picture stacey-gammon  Â·  12Comments

iansan5653 picture iansan5653  Â·  8Comments

darrenmart picture darrenmart  Â·  13Comments