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

Things to discuss about jsdoc and document generation. #22062

Open
sainthkh opened this issue May 4, 2020 · 2 comments
Open

Things to discuss about jsdoc and document generation. #22062

sainthkh opened this issue May 4, 2020 · 2 comments
Labels
[Package] Docgen /packages/docgen [Type] Discussion For issues that are high-level and not yet ready to implement.

Comments

@sainthkh
Copy link
Contributor

sainthkh commented May 4, 2020

Related #21238, #17271.

1. Should we allow JSDoc type notation?

JSDoc and TypeScript type notations are different. They can create confusion. For example:

  • [string] means an optional string argument in JSDoc, while it means a tuple with a single string item.
  • string? means a nullable string in JSDoc. But it mean an optional item in a tuple in TypeScript. const x: string? is simply an error in TypeScript.
  • string= doesn't exist in TypeScript.

I guess most type-loving developers expect TypeScript type notation. It makes me think if providing type in JSDoc notation is right.

2. About Object

TypeScript treats capital Object as any. If you want it to check if it's an object, we need to use small object.

And it's the first don't of the do's and don'ts list.

In TypeScript, object is the {} thing. Object means this global Object class. That's why it is not recommended to use Object.

But the problem is that it requires huge changes.

3. = and ?

TypeScript vs. JSDoc is going on.

If we decide to prefer TypeScript notation, we need to change:

  • string=, [string] to string | undefined
  • ?number, number? to number | nullable

4. Documenting default values.

Currently, default values are not documented. comment-parser correctly parses default values. So, all we need to decide is how to show them.

One way is to add it after description like below:

-   _status_ `string`: Notice status. (Default: `info`)

5. Should we allow spaces as indentation in code examples?

In some code examples, spaces are used as indentations:

// From packages/data/src/components/with-dispatch/index.js

/**
 * @example
 * ```jsx
 * function Button( { onClick, children } ) {
 *     return <button type="button" onClick={ onClick }>{ children }</button>;
 * }
 * ```
 */

It's against WordPress coding convention. Should we allow this?

If not, we need an eslint rule to prevent this.

6. Should we allow *\t in JSDoc comment?

Some JSDoc comment starts with *\t like:

// edit-post/src/components/sidebar/plugin-document-setting-panel/index.js

/**
 * @example
 * <caption>ESNext</caption>
 * ```jsx
 * const MyDocumentSettingTest = () => (
 * 		<PluginDocumentSettingPanel className="my-document-setting-plugin" title="My Panel">
 *			<p>My Document Setting Panel</p>
 *		</PluginDocumentSettingPanel>
 *	);
 * ```
 */

There is no coding convention about this. Maybe we need one.

7. We might need our own eslint plugin for checking type validity.

As I have written in #21238, we can parse TypeScript types with babel.

eslint-plugin-jsdoc uses jsdoctypeparser. It cannot parse many TypeScript types.

We can solve this problem by using @babel/core TypeScript parser.

8. Do we need wrapping () for union types?

Current type string generator always generate () when union types are used (e.g. (null|undefined)). Do we need this ()?

9. How to handle keyof type?

_representation_ `keyof FORMATTING_METHODS`: Type of representation (display, raw, ariaLabel).

It's a bit meaningless if we don't provide the list of values. I think there're 2 ways to handle this:

  • list the values: 'A' | 'B' | 'C'
  • document type separately: type FORMATTING_METHODS: 'A' | 'B' | 'C';

10. WordPress coding convention and type notation.

WordPress coding convention uses spaces a lot, but docgen-generated types don't.

Should we change:

  • (timestamp:number)=>void to ( timestamp: number ) => void?
  • null|undefined to null | undefined?
@sainthkh sainthkh added the [Package] Docgen /packages/docgen label May 4, 2020
@talldan talldan added the [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. label May 4, 2020
@aduth
Copy link
Member

aduth commented May 15, 2020

I guess most type-loving developers expect TypeScript type notation. It makes me think if providing type in JSDoc notation is right.

At least as it concerns documentation, I think we can also consider another option, which is to be more verbose in using human-readable labels to avoid any ambiguity. I think it'd be totally fine to use the word "(Optional)" somewhere in the description, than to worry over which of [string], string?, or string= is going to be best understood as being optional.

@youknowriad youknowriad added [Type] Discussion For issues that are high-level and not yet ready to implement. and removed [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. labels Jun 4, 2020
@skorasaurus skorasaurus added the [Type] Developer Documentation Documentation for developers label Jan 14, 2021
@ryanwelcher
Copy link
Contributor

Is there anything actionable around the documentation? This feels like a discussion so I'll remove the docs label for now

@ryanwelcher ryanwelcher removed the [Type] Developer Documentation Documentation for developers label Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Docgen /packages/docgen [Type] Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

No branches or pull requests

6 participants