Redundant JSDoc @return from an anonymous callback in JavaScript

Question

I am using JSCS with Google preset style rules to check my code and I have a method in a DAO class defined like so:

/**
 * Inserts a new user into database.
 *
 * @param {User} user User to insert.
 * @return {User} Last inserted user.  // Redundant return statement
 * @throws Error if query fails.
 * @since 1.0
 */
add(user) {
  this.pool.getConnection((err, conn) => {
    conn.query('INSERT INTO users SET ?', user, (err, rows) => {
      if (err) {
        throw err;
      }
      conn.release();
      return this.getById(rows.insertId);
    });
  });
}

JSCS marks JSDoc @return tag as redundant because it can't find a return statement inside add(user) function scope, but it actually resides inside the anonymous callback (err, rows) => { ... }.

How can I correctly document that return statement? Is my approach wrong or bad in some way?

`add` indeed doesn't return anything. How does the `conn.query` method use the return value of its callback? In other words: why are you returning anything at all inside the `query` callback? — apsillers, Mar 24 '16 at 17:26
@apsillers The value returned from `add(user)` goes to server controller in order to manage authentication. — Enrico Ceron, Mar 24 '16 at 17:30
`add` does not return a value, and its return value cannot be based upon the result of an asynchronous operation like `query`. It is currently not possible to tell you how to correctly document your code, because your code doesn't work the way you intended. (Right now, the answer to your question is "You must remove the `@return` doc statement, because your function *does not return anything*.") See [How do I return the response from an asynchronous call?](http://stackoverflow.com/q/14220321/710446) for how to restructure your code to accept a callback instead of a returning a value. — apsillers, Mar 24 '16 at 17:34
@Emissary That resolves JSDoc warning but `conn.query(...)` (and so `this.pool.getConnection(...)`) do not return a `User`. — Enrico Ceron, Mar 24 '16 at 17:39
Yes, that has indeed been your problem all along -- neither `conn.query` nor `this.pool.getConnection` nor `add` ever returned a `User`, and they can't: `return`ing is a synchronous operation that must happen before any asynchronous operations resolve. See the link above. — apsillers, Mar 24 '16 at 17:41
@apsillers Solved with promises and no return. JS asynchronous behavior is now a lot more clear :) — Enrico Ceron, Mar 24 '16 at 18:31

score 0 · Accepted Answer · edited May 23 '17 at 12:15

add does not return anything, so JSDoc is correct when it tells you that your @return tag is not appropriate. If you refactored your code so that add accepted a callback which is passed the result (as described in How do I return the response from an asynchronous call?), you would end up with

add(user, resultCallback) {
  this.pool.getConnection((err, conn) => {
    conn.query('INSERT INTO users SET ?', user, (err, rows) => {
      if (err) {
        throw err;
      }
      conn.release();
      resultCallback(this.getById(rows.insertId));
    });
  });
}

Call this with add(user, result => { ... }) instead of result = add(user).

How to document this? See How to document callbacks using JSDoc? It would look like:

/**
 * Inserts a new user into database.
 *
 * @param {User} user User to insert.
 * @param {userResultCallback} resultCallback callback with last inserted user
 * @throws Error if query fails.
 * @since 1.0
*/
add(user, resultCallback) {
  //...
}

/**
 * Callback used to get a single User value.
 * @callback userResultCallback
 * @param {User} user Result of the callback
 */

The @callback at the bottom is stand-alone. It defines a type of callback function, which in this case is a callback that accepts a User as its only argument.

Redundant JSDoc @return from an anonymous callback in JavaScript

1 Answers1