Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Documenting the default value for a parameter #151

Open
octogonz opened this issue Mar 15, 2019 · 12 comments
Open

RFC: Documenting the default value for a parameter #151

octogonz opened this issue Mar 15, 2019 · 12 comments
Labels
request for comments A proposed addition to the TSDoc spec

Comments

@octogonz
Copy link
Collaborator

In issue #27 (comment) , @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
Copy link
Collaborator Author

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:

  • Why aren't string literals enclosed in quotes? For example, if the "The ${title} series" string happened to be the text string "{ dataSource: "default" }" then without quotes we can't distinguish it from the object notation used by the options parameter. Or what the title was "] -"? Without quotes, there's no way to distinguish those characters from @param delimiters.

  • If strings are quoted, obviously we need to support \ escape characters, otherwise you can't have a " character as your default value.

  • Which quoting styles are allowed? In TypeScript we have ', ", and `

  • What other kinds of expressions are allowed for the default value? For example, a TypeScript parameter default value can include operators (e.g. "hello" + " world") and even function calls (e.g. f("hello").trim()). That seems a little excessive for documentation, and it would require TSDoc parsers to include a full JavaScript subgrammar, which is costly. So we'd need to restrict it down somehow.

  • Can the value expression span multiple lines? If so, then we need to design the rules carefully to ensure that if someone types an invalid expression, the parser won't consume an arbitrary amount of text. Otherwise the syntax highlighter can weirdly highlight too much stuff, or parsing may be inefficient.

@octogonz octogonz added the request for comments A proposed addition to the TSDoc spec label Mar 15, 2019
@octogonz octogonz changed the title Documenting the default value for a parameter RFC: Documenting the default value for a parameter Mar 15, 2019
@aciccarello
Copy link

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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:

  • What if the default value expression spans multiple lines?
  • How do we syntax-highlight it as TypeScript?
  • What if the default value expression includes the dreaded ` character that Markdown provides no way to escape? (Fortunately TSDoc already has to solve this one elsewhere.)

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
Copy link

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
Copy link
Collaborator Author

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
Copy link

transitive-bullshit commented Sep 13, 2019

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
Copy link

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
Copy link

braebo commented Jul 19, 2022

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
Copy link
Collaborator Author

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
Copy link

bad4iz commented Jan 5, 2023

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
Copy link

@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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request for comments A proposed addition to the TSDoc spec
Projects
None yet
Development

No branches or pull requests

6 participants