microsoft / tsdoc

A doc comment standard for TypeScript
https://tsdoc.org/
MIT License
4.71k stars 131 forks source link

RFC: Support for dot syntax on @param #19

Open darrenmart opened 6 years ago

darrenmart commented 6 years ago

I'm currently working with d.ts files where methods taking object parameters are documented with the following approach:

/**
* Method description.
* @param options addItemOptions - e.g. { id: 1234, title: "someTitle" }
*             id: The unique identifier.
*             title: The title to assign.
**/

It's challenging to parse and auto-generate documentation for these parameters given this approach. Ideally there'd be support for dot syntax e.g.:

@param options.id - The unique identifier.
@param options.title - The title to assign.
octogonz commented 6 years ago

For APIs intended for third parties, generally we recommend to give a name to the interface, for example:

/** Options for {@link ItemCollection.add} */
interface IAddItemOptions {
  /** the identifier */
  id: string;
  /** a title, not to exceed 40 characters */
  title: string;
}

class ItemCollection {
  /**
   *  Adds an item to the collection
   * @param options - information about the item to be added
   */
  public add(options: IAddItemOptions): void {
  }
}

Naming the interface improves the structure of the documentation. It also helps developers when they need to work with the object independently, for example:

const options: IAddItemOptions = this.getOptions();
collection1.add(options);
collection2.add(options);

That said, I believe TypeDoc supports the @param options.id notation. If people find it useful, we could include it in the TSDoc standard.

seanpoulter commented 6 years ago

What's the long term plan for TSDoc? Will we be able to use it to annotate vanilla JavaScript like the current JSDoc support? In those cases the it's crucial to be able to declare an interface in the comments or document @param properties.

octogonz commented 6 years ago

@seanpoulter I moved your question into a separate issue, since it's a separate topic.

Gerrit0 commented 6 years ago

An additional point to consider when reviewing the dot syntax is the values might not be valid identifiers. When describing a tuple type, it would be great to be able to be able to use a.0 and a.1.

DovydasNavickas commented 6 years ago

I wouldn't put this functionality in the initial version of TSDoc, because we could go forever like that and not release anything as it's been quite some time already. I'd rather have a working version using non-exported interfaces to document function parameters rather than postpone release by whatever amount of time with more functionality.

@pgonzal, if you add me to admins of the project (or whatever role I could be), I could structure things with releases and add initial versions to the issues. If we get arguments saying that something is super important and should be done from the get go, we can always change priorities. But it would be clear to everyone what are we aiming at.

dschnare commented 5 years ago

What's more of a pain to document is destructured arguments:

/**
 * Call home.
 * @remarks
 * This texts Mom a message.
 * @param message - The message to send to Mom
 * @param ? - The number of seconds to delay calling home
 * @public
 */
function callHome (message: string, { delay = 0 }: { delay?: number } = {}) {
  // TODO: send a text to Mom!
  throw new Error('Unimplemented')
}
octogonz commented 5 years ago

@dschnare do people commonly do this in a real world public API?

This practice seems to be entangling the internal implementation details of callHome() with its public API signature. It makes things more concise for the person who's writing the code, but at the expense of hindering our ability to help other people read and understand the code. One of my personal soapboxes is: Reading > Writing. (Writing code is fun and easy. But enabling others to understand and extend a code base is hard work, and a more important goal.)

From a documentation standpoint, destructuring has some downsides:

For an external-facing API, my own team would normally want to convert it to something like this:


/** Options for the `callHome()` API */
export interface ICallHomeOptions { 
  /** 
   * The number of seconds to delay calling home
   * @defaultValue 0 
   */
  delay?: number;
};

/**
 * Call home.
 * 
 * @remarks
 * This texts Mom a message.  If you want to delay the message,
 * consider using {@link ICallHomeOptions.delay}.
 * @param message - The message to send to Mom
 * @param options - Additional options
 * @public
 */
function callHome (message: string, options?: ICallHomeOptions = {}) {
  // TODO: send a text to Mom!
  throw new Error('Unimplemented')
}

So now a caller can do this:

// Using this:
function getStandardOptions(scenario: Scenario): ICallHomeOptions {
  . . .
}

callHome(message, this.getStandardOptions(Scenario.Intranet));

And someone can open a GitHub issue complaining that ICallHomeOptions.delay is broken, and we know what they're talking about.

seanpoulter commented 5 years ago

@dschnare do people commonly do this in a real world public API?

I have struggled with this use case trying to add type information to a vanilla JS for some time. I wouldn't go so far as to call it the "public API" but this is common practice when consuming props in a React component. I've found the first argument destructured ~450 times in ~300 files in an e-commerce website I work on. If you have access to any other React repos, try searching for const [^ ]+ = \(\{.

octogonz commented 5 years ago

Fair point. But how would you propose for TSDoc to handle that?

aciccarello commented 5 years ago

I agree with @pgonzal's comments. IMO it would be better to define an interface. Instead of destructuring in the function arguments, I would also recommend destructuring within the function body. I'm not sure if you were counting this in your react repo but I wanted to note that alternative which I find makes the function signature easier to understand.

It may be common not to create an interface for function arguments, however I don't think TSDoc should be complicated because of this pattern when there is a clear alternative.

/** Options for the `callHome()` API */
export interface ICallHomeOptions { 
  /** 
   * The number of seconds to delay calling home
   * @defaultValue 0 
   */
  delay?: number;
};

/**
 * Call home.
 * 
 * @remarks
 * This texts Mom a message.  If you want to delay the message,
 * consider using {@link ICallHomeOptions.delay}.
 * @param message - The message to send to Mom
 * @param options - Additional options
 * @public
 */
function callHome (message: string, options?: ICallHomeOptions = {}) {
  const { delay = 0 } = options;

  throw new Error('Unimplemented')
}
seanpoulter commented 5 years ago

I think you've nailed it for the public API. 👌

I've been brainstorming variants but haven't converged on anything I like. I've shared the variants I've thought of below. If anything, you've won me over that adding the interface is the way to go. It's readable and reduces the complexity to parse things.


Brainstorming ... 💭

I've mashed this into a JSDoc comment in the past which doesn't work as you add members or descriptions:

/** @param {name: string, age: number} ? - A person */
const toExample = ({ name: n, age: a }) => `${n} - ${a}`;


Consider the case when you refactor a function that has too many arguments to use named arguments with default values. The dot notation is great for this case. You can see the repetition of * @param args. adds to the clutter -- the interface is looking better.

const bad = (a, b, c, d, e, ...rest) => {};
// This is called as: bad('foo', null, null, FORM.FOO_ID, 1)

/**
 * @param args
 * @param args.a - Placeholder
 * @param args.b - Accepted values
 * @param args.c - Validation functions
 * @param args.d - Field ID
 * @param args.e - Tab index
 */
const better = ({ a, b = null, c = null, d = null, e, ...rest }) => {}; 


It would be nice to remove the args. and have an anonymous argument. Naming things is hard. The ??? placeholder is really awkward and it's clear that this would look better expanded -- like the interface.

/**
 * @param message
 * @param ???.delay - The seconds ... etc
 */
const callHome = (message: string, { delay = 0 } = {}) => {};


If we got a little crazy, we could drop the parent object(s) from the annotation and only document the values after they're destructured and renamed and let the type inference pick up the slack:

/**
 * @param' n - First name
 * @param' a - Age
 */
const toExample = ({ name: (n: string), age: (a: int) }) => `${n} - ${a}`;
rbuckton commented 4 years ago

I would also like to see this implemented, as sometimes it doesn't make sense to introduce an interface for optional named parameters.

For reference, here is the documentation for the same feature in JSDoc: https://jsdoc.app/tags-param.html#parameters-with-properties

octogonz commented 4 years ago

Seems reasonable. :+1:

NickHeiner commented 3 years ago

I understand @octogonz' argument from 2018. In most cases, it's probably best to define an explicit arguments type.

However, I would still like ergonomic support for anonymous object argument types in TSDoc.

  1. I'm migrating a legacy codebase to TS. I can give @octogonz' recommendation for writing new code and refactoring, but I'd like a lower barrier to entry for TS for the legacy code.
  2. Sometimes in JS, I see an object used essentially as "named arguments to a function":
declare function f({a, b}: {a: string; b: boolean});

type FArgs = {a: string; b: boolean};
declare function f({a, b}: FArgs);

Arguably, creating FArgs is a bit redundant; why couldn't we just do Parameters?

Even if @octogonz approach is right 90% of the time, I like the idea of providing the option for anonymous object function args for the 10% where it is actually more readable, and trusting devs to make the right call.

  1. With regards to @octogonz' specific documentation concerns:

There's no name that we can use to discuss the second parameter of this function.

To me, in the example I listed above, someone could say "parameter a passed to f" and I'd understand exactly what they meant.

If a caller wants to construct and reuse that parameter, they can't easily declare its type

They can, with Parameters<typeof f>.

If we later want to deprecate the delay option, or write examples for it, there's no place to put that

I think the TSDoc for f would be a perfectly fine place for examples.

octogonz commented 3 years ago

This seems fine to me, particularly since JSDoc has the same solution:

/**
 * @param args
 * @param args.a - Placeholder
 * @param args.b - Accepted values
 * @param args.c - Validation functions
 * @param args.d - Field ID
 * @param args.e - Tab index
 */
const better = ({ a, b = null, c = null, d = null, e, ...rest }) => {}; 

I think we could update the TSDoc parser to support this pretty easily.

👉 Let's make a PR to support that at least, since it probably covers 95% of use cases.

Documenting more elaborate structures like this example from JSDoc seems more sketchy:

/**
 * Assign the project to a list of employees.
 * @param {Object[]} employees - The employees who are responsible for the project.
 * @param {string} employees[].name - The name of an employee.
 * @param {string} employees[].department - The employee's department.
 */
Project.prototype.assign = function(employees) {

It implies a complete grammar for referring to nested members (analagous to the declaration reference syntax that we use for {@link} tags -- but different because it traverses runtime objects rather than types).

That seems like overengineering. The frankly simplest approach would be to put the doc comments inline, something like this:

export function f(a: {
  /**
   * the docs for `b`
   */
  b: {
    /**
     * the docs for `c`
     */
    c: number;
  }[];
}): void {}

When I tried this, the TypeScript compiler faithfully emitted these comments in the .d.ts files, and VS Code IntelliSense seems to recognize it. So we would simply want to formalize it a bit:

NickHeiner commented 3 years ago

I would be perfectly happy to put the doc comments inline. That also has the benefit of saving one from repeating the param name.

I ended up on this issue because @microsoft/api-extractor does not recognize this inline style of doc comment. But since you said TS is emitting those commits and VSC Intellisense recognizes it, then maybe I need to follow up with @microsoft/api-extractor.

aogriffiths commented 3 years ago

has anyone started work on this? I would find it very useful!

NickHeiner commented 3 years ago

I agree that both of the simpler approaches would be very valuable and supporting the more complicated grammar is excessive.

On Fri, Mar 5, 2021 at 05:00 Adam Griffiths notifications@github.com wrote:

has anyone started work on this? I would find it very useful!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/microsoft/tsdoc/issues/19#issuecomment-791312502, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGKTA26ST22BRQDB3FNJLDTCCTUHANCNFSM4FB2FV4Q .

tukusejssirs commented 2 years ago

I’d like to see this implemented as well.

As for documenting the interfaces, okay, I could do that, but what about classes? I mostly use DTOs as classes. I can’t use @param, as a class does not use parameters, and there I can’t use `@property'.

ADTC commented 9 months ago

@dschnare I thought it may be better to name the options block and then destructure it within the function code block:

/**
 * Call home.
 * @remarks
 * This texts Mom a message.
 * @param message - The message to send to Mom
 * @param options - An object containing optional arguments.
 * @param options.delay - The number of seconds to delay calling home
 * @public
 */
function callHome (message: string, options: { delay?: number } = {}) {
  const { delay = 0 } = options
  // TODO: send a text to Mom!
  throw new Error('Unimplemented')
}

Technically this is equivalent, but as far as I can tell, it doesn't really improve things as far as Intellisense is concerned. It avoids having to create a separate interface though.

pick68 commented 8 months ago

Has any work started on this? This will be very useful, while I could use an interface to solve the problem that's not always practical.

Gerrit0 commented 8 months ago

Depending on the documentation generator you use, it may already be supported. TypeDoc supports it.

pick68 commented 8 months ago

My question was prompted by having an issue in Visual Studio Code, when I have dot syntax in a param ie. @param options.key - some comment, VS shows a problem with it, which appears to be coming from @microsoft/tsdoc@0.14.2 which is part of the eslint-plugin-tsdoc, so it appears to be a tsDoc problem. The documentation generated by typeDoc is working.

magnusriga commented 3 weeks ago
/** Options for the `callHome()` API */
export interface ICallHomeOptions { 
  /** 
   * The number of seconds to delay calling home
   * @defaultValue 0 
   */
  delay?: number;
};

/**
 * Call home.
 * 
 * @remarks
 * This texts Mom a message.  If you want to delay the message,
 * consider using {@link ICallHomeOptions.delay}.
 * @param message - The message to send to Mom
 * @param options - Additional options
 * @public
 */
function callHome (message: string, options?: ICallHomeOptions = {}) {
  // TODO: send a text to Mom!
  throw new Error('Unimplemented')
}

@octogonz Would you not use an interface alongside destructuring? That seems common these days, especially in React applications. That leaves us back at square one though, as we now have no way of using @param.

Example:

/** Options for the `callHome()` API */
export interface ICallHomeOptions { 
  /** 
   * The number of seconds to delay calling home
   * @defaultValue 0 
   */
  delay?: number;
};

/**
 * Call home.
 * 
 * @remarks
 * This texts Mom a message.  If you want to delay the message,
 * consider using {@link ICallHomeOptions.delay}.
 * @param message - The message to send to Mom
 * @param ??? - Nothing to put here?
 * @public
 */
function callHome (message: string, { delay = 0 }: ICallHomeOptions = {}) {
  // TODO: send a text to Mom!
  throw new Error('Unimplemented')
}

That is how I usually write components, i.e. with named arguments instead of a generic options.

How would you describe that destructured parameter with TSDoc?

PS: Documentation is helpful to our team internally, not just in public-facing APIs.