Skip to content

feat(inheritDoc): Add support for copying item’s documentation by copying it from another API item #1468

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

Merged
merged 1 commit into from
Jan 23, 2021

Conversation

Dergash
Copy link
Contributor

@Dergash Dergash commented Jan 14, 2021

Hello! Thanks for your work, I really appreciate it.

Recently our team started to use Typedoc and we've come to the conclusion that we really do miss the support for @inheritDoc, which I believe is not currently implemented (see open issue #304) when we talk about inheriting from unrelated entities which is allowed in TSDoc, so I hope I can do something to make this feature a bit close.

I've prepared a really quick PoC and a demo

/**
  * Original description
  *
  * @param arg1 Numeric argument 1
  * @param arg2 Numeric argument 2
  * @returns Stringified sum of arg1 and arg2
  */
export function sourceFunction(arg1: number, arg2: number): string {
    return `${arg1 + arg2}`
}

/**
 * @inheritDoc sourceFunction
 * @param argOne Numeric argument One
 * @param argTwo Numeric argument Two
 * @param argThree Numeric argument Three
 * @deprecated TODO: Remove
 */
export function targetFunction(argOne: number, argTwo: number, argThree: number): string {
    return `${argOne + argTwo + argThree}`
}

Demo for this snippet is here: https://dergash.github.io/typedoc/modules.html#targetfunction

When preparing this PoC I've come to some questions which I hope we can discuss before going further.

So this draft is majorly based around ImplementsPlugin which I believe contains initial but undocumented support for TSDoc/inheritDoc.
My understanding is that when some class implements an interface, methods from that interface can inherit some parts of documentation (shortText, text, @returns and any tags which TypeDocs leaves without processing), to enable this feature inheritDoc must be specified:

export interface SomeInterface {
  /**
   * Some method
   *
   * @param someArg1 Some argument
   * @returns Some results
   */
  someMethod(someArg1: number): string
}

export class SomeInterfaceImplementation implements SomeInterface {
  /**
   * @inheritDoc
   */
  someMethod(someArg2: number) {
    return String(someArg2)
  }
}

Apparently it works. Docs will not be inherited without the tag.

So thinking about the differences between this implementation and the one expected in #304 / TSDoc:

Shoud current behavior of ImplementsPlugin be documented?

Missing curly brackets

Apparently TSDoc treats inhericDoc as an inline tag, hence they wrap it in curly bracers:

// https://tsdoc.org/pages/tags/inheritdoc/
/**
* {@inheritDoc example-library#Serializer.writeFile}
* @deprecated Use {@link example-library#Serializer.writeFile} instead.
*/
public save(): void {
    . . .
}

ImplementsPlugin (and InheritDocPlugin from my PoC) are expecting no curly braces; the only tags moment stored in MarkedLinksPlugin and probably MarkedPlugin

What gets copied

According to TSDoc page following things are copied:

  • summary section
  • @remarks block
  • @params blocks
  • @typeParam blocks
  • @returns block

summary and @remarks confused me a bit; after reading both TSDoc and Typedoc docs I believe TSDoc summary equals to Typedoc shortText, but I might be wrong: Typedoc emphasize on shortText "being the first paragraph of text" and the text is the rest of the comment, while from TSDoc description and API Exractor Overview I've got the impression that summary is all text until the first block tag.
So it probably works like this: Typedoc takes first paragraph into shortText and the rest of the text to text, when TSDoc tooks short text to summary and rest of the text shoul be in remarks tag.

So at the moment either summary and @remarks are already copied as per TSDoc, or they don't and can't be copied due to inconsistency in comment structure.

@params and @typeParams are not copied, but according to the TSDoc the should be.

@returns is copied.

Custom tags which are not processed by Typedoc are copied.

Should this be implemented both for inheritDoc and implements plugin?

How exactly copying should be implemented?

At the moment inherited values override matching values on the target entity. Which seems consistent with JSDoc where @inheritDoc leads to ignoring all other tags. From TSDoc site and github I didn't quite get if it's override or merge. But if it is indeed override then @inheritDoc won't work for #304 where there is a need to inherit a comment by concating it with existing comment.

If this is the case, will you consider support for @copyDoc?

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 16, 2021

Thanks for the PR!

Your analysis is right - TypeDoc's behavior doesn't match TSDoc's, which is somewhat expected since it predates the TSDoc specification by several years. The goal is for TypeDoc to move towards aligning with the TSDoc standard, so any improvements in that area are welcomed!

Missing curly brackets

I never really understood why inheritdoc was an inline tag... For now, the POC you provided is consistent with other TypeDoc tags, so I like it as is. TypeDoc doesn't really have support for inline tags currently ({@link} is rather hackily supported), I've been putting off adding support until I have time to spend some real time getting significantly closer to TSDoc.

summary and @remarks

This is another difference between TSDoc & TypeDoc - TSDoc's summary would be ${comment.shortText}\n\n${comment.text}. This behavior does need to be revisited as it can cause issues (#494), so I'd say we already copy summary, but not @remarks since TypeDoc doesn't have special handling for it.

Should this be implemented both for inheritDoc and implements plugin?

I think - yes. The described behavior makes sense to me.

override or merge

Tossing this into the TSDoc playground: https://tsdoc.org/play

/**
 * summary
 * @remarks X
 * {@inheritdoc x}
 */

Shows that TSDoc is even stricter - not an override, but an error if summary or remarks is specified (but not for @param, @returns, @typeParam). This is kind of weird; I would have expected it to copy if not present in the inheriting comment... For parity with TSDoc, we probably shouldn't extend this behavior, and instead use @copyDoc for providing merge support.

@Dergash Dergash force-pushed the feat/inheritDoc branch 7 times, most recently from 0893237 to 5982b25 Compare January 20, 2021 12:21
@Dergash
Copy link
Contributor Author

Dergash commented Jan 20, 2021

Extended demo here
Demo source code here

Some details on implementation:

  1. copyComment is extracted from ImplementsPlugin to utils/reflections

  2. copyComment is modified to conform with TSDoc standard of replacing @param block instead of merging; to avoid breaking existing behaviour of @inheritDoc the current implementation is moved to legacyCopyImplementation function and is called when @inheritDoc is used without parameter.

  3. copyFrom now only copies tags specified in TSDoc: @remarks and @returns, and leaves other tags not used by Typedoc without change

  4. InheritDocPlugin introduced to traverse through reflections with @inheritDoc and call copyComment. There is a white spot there concerning matching methods signatures: InheritDocPlugin relies on Type.isTypeListEqual to check templated arguments and it works for entities with established inheritance relationship (extends, implements). When referencing unrelated entity there is no obvious way to know that source<T>(arg: T) matches target<T>(arg: T), so there is a fallback to first available which probably not very correct.

If it works I can move to @copyDocs as per initial issue #304

@Dergash Dergash marked this pull request as ready for review January 20, 2021 13:32
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 21, 2021

At first glance, this looks great! I will need to look at it in more depth this weekend.

Copy link
Collaborator

@Gerrit0 Gerrit0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Thanks :)

@Gerrit0 Gerrit0 merged commit 84db49a into TypeStrong:master Jan 23, 2021
@Dergash Dergash deleted the feat/inheritDoc branch January 23, 2021 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants