microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.02k stars 12.49k forks source link

JSDoc comment for destructuring param: description text not displayed #24746

Open steph643 opened 6 years ago

steph643 commented 6 years ago

TypeScript Version:
2.9

Search Terms:
JSDoc destructuring param

Code

/**
 * @param {Object} arg
 * @param {number} arg.id - This param description won't show up
 */
function foo({ id }) {}

Expected behavior: In VSCode 1.24.0, when typing foo(, IntelliSense should display the full param description, including its type and text.

Actual behavior: The type is displayed ("number"), but not the text ("This param description won't show up"): image

Related Issues: https://github.com/Microsoft/TypeScript/issues/19645

Additional remark When omitting the {Object} line, the param text shows up correctly: image

mhegazy commented 6 years ago

PRs welcomed.

rbiggs commented 6 years ago

That's because TypeScript treats JSDoc types object and Object as any. You can designate a generic object using Object.<string, any>. So, redoing the above will give you the proper intellisense for id:

/**
 * @param {Object.<string, any>} arg
 * @param {number} arg.id - This param description will show up
 */
function foo({ id }) {}

// Intellisense for id will work
foo({id: 1})
tannerbaum commented 6 years ago

@rbiggs That does indeed work. However, VSCode only displays the description of the first parameter in the destructured object.

screen shot 2018-09-14 at 12 06 04 screen shot 2018-09-14 at 12 09 48

I feel like we are so close here!

rbiggs commented 6 years ago

@THoisington, for that to work, you need to first define a custom object for the options, and then define its properties. Then you assign that custom type as your parameter:

/**
 * Entries Object.
 * @typedef {Object.<string, any>} requiredArguments
 * @property {string} timeSince
 * @property {string} timeUntil
 */
/**
 * 
 * @param {requiredArguments} param
 */
async function getEntries({timeSince, timeUntil}) {}

That'll give you IntelliSense for each property on the options parameter.

tannerbaum commented 6 years ago

This appears to have popped up in several issues, perhaps there could be some consolidation: #11597 and #11859, for example.

@rbiggs , I attempted your solution and found the vscode intellisense didn't display the subkeys or comments at all when using the @typedef and @property combination.

At the risk of making a huge comment, I will document my findings here.


Outcome 1: ❌ Top level details, ❌ subkey descriptions, ❌ current subkey highlighting.

screen shot 2018-09-17 at 16 28 17 screen shot 2018-09-17 at 16 29 27

JSDOC that caused this:

/**
 * @typedef {Object} requiredArguments
 * @property {string} timeSince Beginning date for the range...
 * @property {string} timeUntil End date for the range...
 */
/**
 * @param {requiredArguments}
 * @param {*} options - { filterUserId, filterCustomerId ... }
 * @see https://www.clockodo.com/en/api/entries/
 */
/**
 * @param {Object} requiredArguments
 * @param {string} requiredArguments.timeSince - Beginning date for the range...
 * @param {string} requiredArguments.timeUntil - End date for the range...
 * @param {Object} options { filterUserId, filterCustomerId ... }
 * @see https://www.clockodo.com/en/api/entries/
 */
/**
 * @typedef {Object.<string, any>} RequiredArguments
 * @property {string} RequiredArguments.timeSince - Beginning date for the range...
 * @property {string} RequiredArguments.timeUntil - End date for the range...
 */
/**
 * @param {RequiredArguments} params
 * @param {Object} options { filterUserId, filterCustomerId ... }
 */

Outcome 2: ✅ Top level details, ✅ subkey descriptions, ❌ current subkey highlighting.

screen shot 2018-09-17 at 16 33 29 screen shot 2018-09-17 at 16 33 46

JSDOC that caused this:

/**
 * @typedef {Object.<string, any>} RequiredArguments
 * @param {string} timeSince - Beginning date for the range...
 * @param {string} timeUntil - End date for the range...
 */
/**
 * @param {RequiredArguments} params
 * @param {Object} options { filterUserId, filterCustomerId ... }
 */
/**
 * @typedef {Object} RequiredArguments
 * @param {string} RequiredArguments.timeSince - Beginning date for the range...
 * @param {string} RequiredArguments.timeUntil - End date for the range...
 */
/**
 * @param {RequiredArguments} paramsss
 * @param {Object} options { filterUserId, filterCustomerId ... }
 */

I could move this comment to a different issue if it helps.

sandersn commented 5 years ago

11859 is just for Typescript, but is otherwise the same. Let's use this issue to track bugs in the Javascript side.

lookuh commented 4 years ago

Interestingly, both the typedef and the integrated properties syntax work if you declare the argument you're destructuring to be a @property instead of the @param that it is.

jnfrati commented 3 years ago

I'm having the same issue, I'm currently using javascript, and the solution provided by @lookuh partially implemented works in an awkward way.

When used the JSDocs comments in this way, it for sure won't show the destructured params comments:

/**
 * @param {object} params
 * @param {stirng} params.id Some usefull id
 * @param {number} params.randomNumber Some randome number
 */

Captura de Pantalla 2021-01-13 a la(s) 11 33 41

But when at least one of the param is defined as property, it starts to show the other params comments correctly, but it does not give type to them:

Captura de Pantalla 2021-01-13 a la(s) 11 41 03

Captura de Pantalla 2021-01-13 a la(s) 11 42 56

And a full typedef with @property solution isn't showing the destructured params:

/**
 * @typedef MyParams
 * @property {string} id Some usefull id
 * @property {number} randomNumber Some random number
 * @property {Date} someDate Just a date
 */

/**
 * @function useCustomHook
 * @param {MyParams} params
 */

Captura de Pantalla 2021-01-13 a la(s) 11 49 24

Hope this comment helps in any way.

Juanpam commented 3 years ago

Just another workaround. I'm adding all the params as regular parameters instead and that way I get intellisense to display the properties in the argument as well as the description. It is also hinted that the function expects an object rather than multiple parameters:

image

/**
 * This function syncs the player configuration available in the server with the one sent by the player.
 * @param {object} argument
 * @param {string} playerID - ID of the player which configuration is going to be synced.
 * @param {Object} playerStatus - Object representing the status of the player.
 * @param {boolean} isTest - Indicate if this action is a test or not.
 * @param {Object} playerUpdates - Object representing the updates sent by the player.
 * @returns Resolves if the configs have been synced succesfully. Rejects otherwise.
 */

Only drawback is the additional parameter that is shown as well (argument param in this case).

J3m5 commented 3 years ago

@Juanpam The problem here is that we lose the type definition of the arguments at the top. We can't have both at the same time.

Juanpam commented 3 years ago

@J3m5 I agree! Just my workaround for this issue. Still hoping this gets reviewed soon.

kolchurinvv commented 2 years ago

having the same issue with in vscode in javascript using a named parameter

custom error class jsdoc
ShadowLNC commented 2 years ago

I seem to have the same issue but I am not specifying my types in JSDoc because I'm explicitly specifying them on the function, e.g.:

/**
 * Description.
 *
 * @param arg0 arguments.
 * @param arg0.a first.
 * @param arg0.b second.
 * @param arg0.c third.
 */
public async myFunc({a, b, c}: SomeInterface) {}

Apologies if this is not intended to be supported, the JSDoc docs only show examples where the types are in JSDoc (for object destructuring), but I know that VSCode does display the tooltip for regular parameters even when I don't put types in JSDoc.

PoulBak commented 2 years ago

Is JSDoc dead? This is not solved after more than 4 years.

joesawa commented 1 year ago

Problem still persists into 2023.

dagadbm commented 1 year ago

I have found the simplest cleanest solution for this mess.

/**
 * Main entrypoint and recommended function to use for route navigation.
 * @param {string} location - The path or url to navigate to, serves as a fallback url when goBack is true
 * @param {object} [config] - Additional configuration to alter the navigation
 * @property {object} [config]
 * @param {boolean} [config.replace] - If true, will use replace instead of push. This bypasses the browser history mechanism.
 * @param {boolean} [config.newTab] - If true, will open the url in a new tab, using window.open bypassing router
   * navigation.
 * @param {function} [config.callback] - If a function, will be called after navigation has ended
 * @param {boolean} [config.goBack] - If true, will use the browser history mechanism to go back. If going back leaves
 * the page, e.g. triggers beforeunload event, we warn the user, if he wants to stay in the page we redirect to the
 * provided url, which serves as a fallback.
 */
image
kolchurinvv commented 1 year ago

@dagadbm At least there's some work-around. It still doesn't seem to cut it for me, though. take a look:

export class ExtendedPromiseError extends Error {
  /**
   * `val: "default"` assigns `this.val = val(){return {}}`\
   * you can also assign any value to this prop
   *
   * @param {String} message - required by the parent `Error` class
   * @param {object} options - optional
   * @param {string} [options.cause] - specific reason
   * @param {string|any}  [options.val] - for cases when handler needs to return a value
   * @param {string} [options.once] - if attaching a listener is undesirable
   */
  constructor(message, options) {
    super(message)
    this.message = message
    this.name = "Extended Promise Error"
    this.cause = options?.cause
    if (options?.val) {
      if (options.val === "default") {
        this.val = () => {
          return {}
        }
      } else {
        this.val = options.val
      }
    }
  }
}
Screenshot 2023-01-30 at 13 55 40

is there a jsconfig.js / tsconfig.ts setting i'm missing/misusing?

dagadbm commented 1 year ago

You are missing the empty line I added with the @property part.

kolchurinvv commented 1 year ago

that was dumb of me 🤣 - worked like a charm! 💯

joesawa commented 1 year ago

I have found the simplest cleanest solution for this mess.

/**
 * Main entrypoint and recommended function to use for route navigation.
 * @param {string} location - The path or url to navigate to, serves as a fallback url when goBack is true
 * @param {object} [config] - Additional configuration to alter the navigation
 * @property {object} [config]
 * @param {boolean} [config.replace] - If true, will use replace instead of push. This bypasses the browser history mechanism.
 * @param {boolean} [config.newTab] - If true, will open the url in a new tab, using window.open bypassing router
   * navigation.
 * @param {function} [config.callback] - If a function, will be called after navigation has ended
 * @param {boolean} [config.goBack] - If true, will use the browser history mechanism to go back. If going back leaves
 * the page, e.g. triggers beforeunload event, we warn the user, if he wants to stay in the page we redirect to the
 * provided url, which serves as a fallback.
 */
image

Sadly this leaves out type information.

dagadbm commented 1 year ago

I didn't notice that I was just focused on getting it to show the documentation

On Thu, Feb 2, 2023, 16:21 Joseph Sawa @.***> wrote:

I have found the simplest cleanest solution for this mess.

/**

  • Main entrypoint and recommended function to use for route navigation.
  • @param {string} location - The path or url to navigate to, serves as a fallback url when goBack is true
  • @param {object} [config] - Additional configuration to alter the navigation
  • @property {object} [config]
  • @param {boolean} [config.replace] - If true, will use replace instead of push. This bypasses the browser history mechanism.
  • @param {boolean} [config.newTab] - If true, will open the url in a new tab, using window.open bypassing router
    • navigation.
  • @param {function} [config.callback] - If a function, will be called after navigation has ended
  • @param {boolean} [config.goBack] - If true, will use the browser history mechanism to go back. If going back leaves
  • the page, e.g. triggers beforeunload event, we warn the user, if he wants to stay in the page we redirect to the
  • provided url, which serves as a fallback. */

[image: image] https://user-images.githubusercontent.com/16737095/215473857-4a260050-6bf6-4168-a7c6-88bf31a0bbf1.png

Sadly this leaves out type information.

— Reply to this email directly, view it on GitHub https://github.com/microsoft/TypeScript/issues/24746#issuecomment-1414010348, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD7WGR47F3SSIKFUTWTCRS3WVPNKJANCNFSM4FDYEENQ . You are receiving this because you were mentioned.Message ID: @.***>

woodjs commented 1 year ago

My decision

/**
 * Middleware для загрузки файлов
 * @param {{
 *  allowedExtensions: string[];
 *  compress?: boolean;
 *  directory?: string;
 * }} options - Data Object
 * @param directory - Папка, в которую будут загружаться файлы (по умолчанию: uploads)
 * @param allowedExtensions - Расширения файлов, которые разрешены для загрузки (по умолчанию: ['jpg', 'jpeg', 'png', 'gif'])
 * @param compress - Сжимать ли файлы (по умолчанию: false)
 * @returns {function} - Middleware для загрузки файлов
 */
dmrickey commented 1 year ago

This is still an issue. The official jsdoc gives this example

/**
 * Assign the project to an employee.
 * @param {Object} employee - The employee who is responsible for the project.
 * @param {string} employee.name - The name of the employee.
 * @param {string} employee.department - The employee's department.
 */
Project.prototype.assign = function({ name, department }) {
    // ...
};

Found here https://jsdoc.app/tags-param.html#parameters-with-properties subheader - Documenting a destructuring parameter

I'd expect the tooltip to work if the jsdoc is correct per its own documentation

jpc-ae commented 1 year ago

I'm unsure if this could be related to the issue, but I noticed that semicolons are being added instead of commas to object parameters. This happens regardless of using the dot-notation or object definition syntax.

image

That is, both of the following result in the above tooltip:

/**
   * Space object stores snippet groupings in buckets.
   * @param {{name: string, synced: boolean, data: DataBucket}} params
   */
/**
   * Space object stores snippet groupings in buckets.
   * @param {Object} params
   * @param {string} params.name
   * @param {boolean} params.synced
   * @param {DataBucket} params.data
   */

I wasn't able to find where this semicolon is being added in the jsdoc parser, but could someone please fix that at least?

Genroa commented 1 year ago

Bumping up this. It's a major issue. Destructured parameters have been around since ES6, and JSDoc has an official documentation on how to document them properly. They're used everywhere, why was it never properly supported in VSCode's JSDoc tooltips?