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

Verify that type aliases and interfaces, as well as members for these, have documentation #223

Open
mcmire opened this issue Jun 24, 2022 · 6 comments

Comments

@mcmire
Copy link
Contributor

mcmire commented Jun 24, 2022

Currently if we have an interface like:

interface PackageMetadata {
  readonly dirName: string;
  readonly manifest: PackageManifest | MonorepoPackageManifest;
  readonly name: string;
  readonly dirPath: string;
}

or a type alias like:

type PackageMetadata = {
  readonly dirName: string;
  readonly manifest: PackageManifest | MonorepoPackageManifest;
  readonly name: string;
  readonly dirPath: string;
}

we are not required to supply documentation for these. This seems inconsistent with our rules. I think we should try to provide documentation for types as much as possible so that we can always look them up in our editors (and such documentation is useful when we publish tsdocs eventually).

We should be able to achieve this by configuring the require-jsdoc rule like so: gajus/eslint-plugin-jsdoc#647

@mcmire
Copy link
Contributor Author

mcmire commented Oct 12, 2022

While we are doing this, we should enforce the correct way to document types for TypeScript files. Although the JSDoc documentation suggests using @property, this tag is actually not supported by TypeDoc. Instead, they prefer you document each property on its own. An example of this can be seen here: https://github.com/TypeStrong/typedoc/blob/2e8af773a2e8f37af547e66bed07c01bf30cbb91/example/src/types.ts#L17

@Mrtenz
Copy link
Member

Mrtenz commented Nov 12, 2022

Although the JSDoc documentation suggests using @property, this tag is actually not supported by TypeDoc. Instead, they prefer you document each property on its own.

How would we do this for inferred types, e.g., from superstruct? I've been doing this in a few places:

/**
 * @property bar - Baz.
 */
type Foo = Infer<typeof FooStruct>;

@mcmire
Copy link
Contributor Author

mcmire commented Nov 14, 2022

@Mrtenz Hmm... That's a good point. @property would make more sense in that context.

I said that TypeDoc doesn't support @property, but that doesn't mean it doesn't render it. For context here is the difference between documenting properties in the docstring and documenting them individually. Given this code:

/**
 * ContactEntry representation
 *
 * @property address - Hex address of a recipient account
 * @property name - Nickname associated with this address
 * @property importTime - Data time when an account as created/imported
 */
export interface ContactEntry {
  /** Hello I am an address */
  address: string;
  name: string;
  importTime?: number;
}

TypeDoc produces:

Screenshot 2022-11-14 at 9 30 56 AM

As you can see, the box at the top includes the @property lines, and the docstrings for the properties themselves are listed in the Properties section.

And of course here's what VSCode shows when you hover over ContactEntry first and address second:

Screenshot 2022-11-14 at 9 39 58 AM

Screenshot 2022-11-14 at 9 40 07 AM

Maybe using @property is not horrible?

@mcmire
Copy link
Contributor Author

mcmire commented Feb 8, 2023

Revisiting this. It appears that TypeDoc and VSCode more or less do what we want here, but require some tweaks. So I think in order to truly support this we would want to:

  • Update the VSCode JavaScript/TypeScript integration (not sure whether it's an extension or the language server) to treat individually documented properties and properties documented via @property as the same and include them in the output when hovering over a type definition
  • Update TypeDoc to treat individually documented properties and properties documented via @property as the same and always include documentation for properties in the Properties list
  • Submit a new feature to the TypeScript ESLint repo that checks that type definitions and their properties are documented (provide a config option to check that properties are either individually documented or documented via @property but not both; or to just check that they are present in either).

@Mrtenz
Copy link
Member

Mrtenz commented Feb 8, 2023

This seems to be the case for IntelliJ IDEA as well. Using @property works, but when using inline comments it doesn't show up. Ideally we support @property in TypeDoc at a minimum, and the other two points would be a "nice to have".

We could start by opening issues in those projects?

@mcmire
Copy link
Contributor Author

mcmire commented Feb 8, 2023

@Mrtenz Yup, absolutely. I'll start by doing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants