digitalbazaar / eslint-config-digitalbazaar

BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

Convention for documenting async functions #26

Open mattcollier opened 4 years ago

mattcollier commented 4 years ago

Example code:

/**
 * cool documentation here
 *
 * @returns {object} The data returned with an `access_token`.
 */
async function getAuthToken(

@aljones15 says:

JsDoc automatically marks aync functions async when creating documents so the promise is redundant here.

This is a very cool JsDoc feature, but since we have very few modules with actually generated JsDocs, I am personally much more likely to read the docs directly in the code.

Does anyone else prefer the explicit/redundant syntax?

 * @returns {Promise<object>} The data returned with an `access_token`.
aljones15 commented 4 years ago

I think the biggest, and somewhat alarming, problem with this issue is that anyone would trust JsDoc above their code base. 2 lines below that @returns statement is the keyword async. I guess my point is a developer should not rely on JsDoc statements as much as reading existing code. I always view JsDoc as a clarification and mnemonic. A year from now when I reread this code the JsDoc statement will give me an example or a use case that will help jog my memory so I recall what exactly it was I was thinking when I wrote this snippet. Many of my PayPal related comments are so future developers know what the PayPal api looked like when the module was written.

aljones15 commented 4 years ago

@mattcollier this seems kind of like a no brainer, but:

https://eslint.org/docs/rules/require-await

we can add a rule require-await: "error"

and eslint will warn if an async function is not awaited. actually it only throws if an async function does not await anything at all. So mistakes could still get through. If more than one function needs to be awaited in an async function.