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

Derive parameter name of desctructured parameter from JSDoc comment #1703

Closed
Gudahtt opened this issue Sep 20, 2021 · 3 comments · Fixed by #1704
Closed

Derive parameter name of desctructured parameter from JSDoc comment #1703

Gudahtt opened this issue Sep 20, 2021 · 3 comments · Fixed by #1704
Labels
enhancement Improved functionality

Comments

@Gudahtt
Copy link
Contributor

Gudahtt commented Sep 20, 2021

Search Terms

__namedParameters, destructured parameter, @param

Problem

TypeDoc sets the name of any destructured parameters to "__namedParameters", even when a name is given in the JSDoc comment. This also results in the description of this parameter (and any child properties) in the JSDoc comment being ignored.

This relates to the "@param dot syntax" that is currently under consideration in TSDoc. TypeDoc already supports the dot syntax for parameters with names (i.e. when they are not being destructured), but it doesn't work for the more common scenario where the parameter is destructured inline.

For example, with this code snippet, all descriptions are rendered correctly:

/**
 * Concatenate strings 'a' and 'b'
 *
 * @param options - The options.
 * @param options.a - The 'a' string.
 * @param options.b - The 'b' string.
 */
function concatAB(options: { a: string, b: string }) {
  const { a, b } = options;
  return a + b;
}

But in this example, the @param tags end up being ignored because TypeDoc doesn't understand that the options param is the descructured parameter:

/**
 * Concatenate strings 'a' and 'b'
 *
 * @param options - The options.
 * @param options.a - The 'a' string.
 * @param options.b - The 'b' string.
 */
function concatAB({ a: string, b: string }) {
  return a + b;
}

Suggested Solution

We could set the name of the parameter to the name given in the JSDoc comment. This would also ensure the descriptions are correctly matched up. This relies upon the assumption that the JSDoc @param tags are present for all parameters, and that they are correctly ordered, so we can add a check to ensure this replacement only occurs if this appears to be the case (or maybe we can make this configurable?).

I am unaware of what other similar tools do in this circumstance.

I'd be happy to do this in a plugin instead, but I'm not sure that it would be possible. This relies upon the parsed comment, so it can't be triggered by the EVENT_CREATE_SIGNATURE, EVENT_CREATE_DECLARATION, or EVENT_CREATE_TYPE_PARAMETER event, because how would we be certain the comment was resolved by then? I think it would need to be done during the "resolve" phase, which is when the information in the parsed comment gets incorporated into the signature at the moment. So we could do this in a plugin that gets triggered by the beginning of the resolve phase, but that would be challenging without duplicating the functionality of addCommentToSignatures, or somehow guaranteeing that the plugin gets run before that function.

@Gudahtt Gudahtt added the enhancement Improved functionality label Sep 20, 2021
Gudahtt added a commit to Gudahtt/typedoc that referenced this issue Sep 20, 2021
The name of a destructured parameter can now be inferred from a JSDoc
comment. The name is only inferred if the number of top-level `@param`
tags in the JSDoc comment exactly matches the number of parameters.

Closes TypeStrong#1703
@Gudahtt
Copy link
Contributor Author

Gudahtt commented Sep 20, 2021

I've created a draft PR to better explain how this could be done (#1704). It doesn't have tests yet, but it does pass all existing tests. If you think this is worth pursuing further, I can update the PR to include better test coverage. I would also be happy to help with documentation updates.

Gudahtt added a commit to MetaMask/eth-sig-util that referenced this issue Sep 20, 2021
The parameter descriptions were missing for any destructured
parameters. Apparently TypeDoc doesn't support showing descriptions for
destructured parameters (see here for more details: [1]).

I have forked TypeDoc to fix this problem, and published this fork as
`@gudahtt/typedoc@0.23.0`. We can use this fork until this has been
resolved upstream.

[1]: TypeStrong/typedoc#1703
Gudahtt added a commit to MetaMask/eth-sig-util that referenced this issue Sep 20, 2021
The parameter descriptions were missing for any destructured
parameters. Apparently TypeDoc doesn't support showing descriptions for
destructured parameters (see here for more details: [1]).

I have forked TypeDoc to fix this problem, and published this fork as
`@gudahtt/typedoc@0.23.0`. We can use this fork until this has been
resolved upstream.

[1]: TypeStrong/typedoc#1703
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Sep 21, 2021

I like this idea - it matches how VSCode recognizes comments, and is fairly intuitive to use.

@Gudahtt
Copy link
Contributor Author

Gudahtt commented Sep 21, 2021

Great! I've just updated the PR with tests and marked it as ready for review.

Gudahtt added a commit to MetaMask/core that referenced this issue Apr 19, 2022
TypeDoc has been updated to the latest version. This version supports
later TypeScript versions, allowing us to update `tsc` in a later PR.
It also includes improvements to document generation, most importantly
in the case where parameters are destructured [1].

Versions v0.21.0 [2] and v0.22.0 [3] both included breaking changes,
but none of them affect us.

This new version included new console warnings that alerted me to a
pre-existing problem that was introduced with v0.20.0, which is that
many types are missing from our documentation. This is because TypeDoc
will only include types that are exported from the package, and many of
our internal types aren't exported from the package itself (even when
they are exported from the module they're declared in). The plugin
`typedoc-plugin-missing-exports` was added to address this. This plugin
ensures that any types referenced in the docs are included in the docs,
even if they aren't exported.

[1]: TypeStrong/typedoc#1703
[2]: https://github.com/TypeStrong/typedoc/releases/tag/v0.21.0
[3]: https://github.com/TypeStrong/typedoc/releases/tag/v0.22.0
Gudahtt added a commit to MetaMask/core that referenced this issue Apr 19, 2022
TypeDoc has been updated to the latest version. This version supports
later TypeScript versions, allowing us to update `tsc` in a later PR.
It also includes improvements to document generation, most importantly
in the case where parameters are destructured [1].

Versions v0.21.0 [2] and v0.22.0 [3] both included breaking changes,
but none of them affect us.

This new version included new console warnings that alerted me to a
pre-existing problem that was introduced with v0.20.0, which is that
many types are missing from our documentation. This is because TypeDoc
will only include types that are exported from the package, and many of
our internal types aren't exported from the package itself (even when
they are exported from the module they're declared in). The plugin
`typedoc-plugin-missing-exports` was added to address this. This plugin
ensures that any types referenced in the docs are included in the docs,
even if they aren't exported.

[1]: TypeStrong/typedoc#1703
[2]: https://github.com/TypeStrong/typedoc/releases/tag/v0.21.0
[3]: https://github.com/TypeStrong/typedoc/releases/tag/v0.22.0
MajorLift pushed a commit to MetaMask/core that referenced this issue Oct 11, 2023
TypeDoc has been updated to the latest version. This version supports
later TypeScript versions, allowing us to update `tsc` in a later PR.
It also includes improvements to document generation, most importantly
in the case where parameters are destructured [1].

Versions v0.21.0 [2] and v0.22.0 [3] both included breaking changes,
but none of them affect us.

This new version included new console warnings that alerted me to a
pre-existing problem that was introduced with v0.20.0, which is that
many types are missing from our documentation. This is because TypeDoc
will only include types that are exported from the package, and many of
our internal types aren't exported from the package itself (even when
they are exported from the module they're declared in). The plugin
`typedoc-plugin-missing-exports` was added to address this. This plugin
ensures that any types referenced in the docs are included in the docs,
even if they aren't exported.

[1]: TypeStrong/typedoc#1703
[2]: https://github.com/TypeStrong/typedoc/releases/tag/v0.21.0
[3]: https://github.com/TypeStrong/typedoc/releases/tag/v0.22.0
MajorLift pushed a commit to MetaMask/core that referenced this issue Oct 11, 2023
TypeDoc has been updated to the latest version. This version supports
later TypeScript versions, allowing us to update `tsc` in a later PR.
It also includes improvements to document generation, most importantly
in the case where parameters are destructured [1].

Versions v0.21.0 [2] and v0.22.0 [3] both included breaking changes,
but none of them affect us.

This new version included new console warnings that alerted me to a
pre-existing problem that was introduced with v0.20.0, which is that
many types are missing from our documentation. This is because TypeDoc
will only include types that are exported from the package, and many of
our internal types aren't exported from the package itself (even when
they are exported from the module they're declared in). The plugin
`typedoc-plugin-missing-exports` was added to address this. This plugin
ensures that any types referenced in the docs are included in the docs,
even if they aren't exported.

[1]: TypeStrong/typedoc#1703
[2]: https://github.com/TypeStrong/typedoc/releases/tag/v0.21.0
[3]: https://github.com/TypeStrong/typedoc/releases/tag/v0.22.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improved functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants