microsoft / tsdoc

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

RFC: Documenting the default value for a parameter #151

Open octogonz opened 5 years ago

octogonz commented 5 years ago

In issue https://github.com/Microsoft/tsdoc/issues/27#issuecomment-473178150 , @sindresorhus asked:

How about documenting default function parameters? What will that look like?

I've opened a new issue since I think it's a separate topic from the @defaultValue tag.

octogonz commented 5 years ago

My initial instinct would be to say that TSDoc should not be involved with parameter defaults, because the information is already clearly expressed in the source code.

For example:

chart.ts

/**
 * Add a series to the chart
 * @param formatString - the label to show for the new series
 * @param options - additional options
 */
export function addSeries(formatString: string = "The ${title} series",
  options: IOptions = { dataSource: "default" }): void {
   // ...
}

But there's a hitch. The TypeScript compiler does not propagate this information into the generated .d.ts files. For example, you might get output like this:

chart.d.ts

/**
 * Add a series to the chart
 * @param formatString - the label to show for the new series
 * @param options - additional options
 */
export declare function addSeries(formatString?: string, options?: IOptions): void;

The .d.ts files are often the only file you ship to consumers of your library. So it does seem that the doc comments need to express this somehow.

I went and looked at JSDoc for comparison. Their @param tag spuports a special notation for this. It uses [] to indicate an optional parameter (which is redundant in TypeScript, but is maybe a useful grouping mechanism). And it uses = to indicate the default value. If we follow that approach, we'd get something like this:

/**
 * Add a series to the chart
 * @param [formatString=The ${title} series] - the label to show for the new series
 * @param [options={ dataSource: "default" }] - additional options
 */
export declare function addSeries(formatString?: string, options?: IOptions): void;

This could work, however TSDoc seeks to be a rigorous and syntactically complete language. I have a number of design questions about JSDoc's notation:

aciccarello commented 5 years ago

Is this something that should be changed in .d.ts files? I'm not sure either way but if TypeScript already can communicate it, why would we want to move that into the doc comment?

octogonz commented 5 years ago

Is this something that should be changed in .d.ts files? I'm not sure either way but if TypeScript already can communicate it, why would we want to move that into the doc comment?

@aciccarello It certainly seems so. But we can also ask a similar question about why decorators are not represented in the .d.ts files (since libraries like Angular use them extensively to describe API contracts). My guess is that in both cases, these expressions can incorporate arbitrarily complex executable code, so there's a slippery slope regarding to what extent it can/should be represented in the type declaration.

A more pathological example:

function square(x: number): number {
    return x * x;
}

function add(
    x: number,
    y: number,
    z = [x, square(x)].reduce((x, y) => x + y)
): number {
    return x*z;
}

@DanielRosenwasser @RyanCavanaugh who may have a more informed opinion than me. :-)

octogonz commented 5 years ago

On the other hand, the above discussion shows that if we push the problem onto TSDoc, it doesn't seem to get much easier. Well... unless we take the escape hatch (no pun intended) of saying that TSDoc's default values are not parsed as TypeScript code at all. They are merely blobs of documentation text. If we go that route, then the problem transforms into a simpler problem of character escaping: How do you delimit a string containing a lot of symbol characters without a ton of backslash escapes everywhere?

I suppose we could simply rely on Markdown:

/**
 * Add a series to the chart
 * @param formatString = `"The ${title} series"` - the label to show for the new series
 * @param options = `{ dataSource: "default" }` - additional options
 */
export function addSeries(formatString: string = "The ${title} series",
  options: IOptions = { dataSource: "default" }): void {
   // ...
}

This feels reasonable at first, although I can raise a bunch of design questions for it as well:

If people like this idea, it is easier I think. These problems are the kind of thing TSDoc deals with every day heheh.

transitive-bullshit commented 5 years ago

Instead of adding functionality to tsdoc to handle default function parameters, I'd much rather just infer the default value from the function definition itself.

But there's a hitch. The TypeScript compiler does not propagate this information into the generated .d.ts files. For example, you might get output like this:

Couldn't we have tsdoc only infer this type of information if run on the original source files? I feel like this is the main use case for tsdoc as opposed to running on third-party .d.ts files.

octogonz commented 5 years ago

Couldn't we have tsdoc only infer this type of information if run on the original source files? I feel like this is the main use case for tsdoc as opposed to running on third-party .d.ts files.

I was mainly thinking about it from the perspective of API Extractor, one of the biggest implementers of TSDoc. API Extractor analyzes the output of the TypeScript compiler (.d.ts files), rather than the original source files (.ts files). This was an intentional design decision, but other documentation tools are free to analyze the .ts files however they like. TSDoc is mainly a standard for how doc comments are parsed, so it doesn't say a lot about how TypeScript signatures get analyzed.

If your documentation tool reads the .ts files, then it's perfectly acceptable to extract the parameter default values from the source code. But then you wouldn't need any help from TSDoc, so this RFC wouldn't be relevant.

transitive-bullshit commented 5 years ago

If your documentation tool reads the .ts files, then it's perfectly acceptable to extract the parameter default values from the source code. But then you wouldn't need any help from TSDoc, so this RFC wouldn't be relevant.

Got it -- thanks!

In that case, I'd prefer @aciccarello's proposal to push this information into the .d.ts files which seems like the KISS solution here.

JakeShirley commented 2 years ago

Any movement on this? I am new to Typescript and I found it quite odd that I couldn't put the default values in my .d.ts files. I'd really like for the users of my code to know the default values for these parameters (as we only ship .d.ts files) but there doesn't seem to be a way to do that.

braebo commented 2 years ago

I strongly agree with @JakeShirley and come across this often. Regarding @octogonz's comment:

My initial instinct would be to say that TSDoc should not be involved with parameter defaults, because the information is already clearly expressed in the source code.

I use TSDoc so that I don't have to read soure code to know what a function does.

For example, I really think the hover-info for a function like this should include the important fact that the default is 0.5. Or I should at least have a formal way to add that information, no? This added information is completely invisible (and seemingly useless) in my editor:

Screenshot 2022-07-19 at 11 59 35 AM

Sorry if I'm just missing something here 😅

octogonz commented 2 years ago

Sorry if I'm just missing something here 😅

What I meant is that typically the default value is already visible in the function signature (chance = 0.5 in your example screenshot). And by "source code" I really meant the output the .d.ts file that is shipped with an NPM package. If the VS Code tooltip does not include that information, it's not really TSDoc's problem. Instead of copying+pasting this information in two places, it would be better to improve the VS Code language service to display this additional data in the tooltip.

That said, your particular example includes additional information ((50%)) which cannot be inferred from the = 0.5 declaration. That would be a reasonable case for supporting @defaultValue, I think.

bad4iz commented 1 year ago

Colleagues, the problem with default values has been going on for a very long time, in my opinion, for 4 years. You couldn't have solved this problem already. Because of this, I can't switch from jsdoc to tsdoc

JakeShirley commented 1 year ago

@octogonz Our situation is a bit different. We use .d.ts files to expose interfaces for our native-based modules which run in our game so there is no backing js/ts source to gleam the additional data from, so I'd love a way to put it into the comment.

Not sure of the general mentality for TSDoc, but I do realize that this creates two "sources of truth" for the default value (implementation vs the comment).