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

docgen: Add TypeScript support #29189

Open
wants to merge 9 commits into
base: master
from

Conversation

@sarayourfriend
Copy link
Contributor

@sarayourfriend sarayourfriend commented Feb 21, 2021

Description

Adds TS support to docgen. This does not require adding any new parsers, the currently used parsers are sufficient (🎉).

All of the new logic exists inside of get-type-annotation. This basically consists of a huge switch function over all of the possible types of type annotation nodes. The module then opinionatedly formats the type annotation based on the AST and returns the new type annotation. If a type annotation already exists (for example, in the case of JSDoc) it just uses that instead.

This does require explicit typing of all exported function returns, however, which is why I added the return type annotation to react-i18n. docgen isn't running any actual TypeScript engine so there's no way to infer the return type like TS normally would be able to do. However, I don't see this as a shortcoming but as a feature: it forces us to explicitly document the inputs and outputs of our exported functions (ESLint should enforce this anyway, once #27143 is merged.

Most of this code is currently unused. Testing it is going to be a chore, so if someone wants to jump in and help me with that, I'd love the assistance (but of course is not necessary). I'll slowly add some unit tests over the next week or so. If anyone has suggestions for how/what exactly is worth testing in this new area, please let me know.

Note: I did remove a nonsense param annotation in the first commit. Like I said, it was a nonsense annotation so I think it's safe to remove it.

How has this been tested?

Run npm run docs:build. Ensure that there are no changes except to react-i18n.

Types of changes

Adds support for TypeScript type annotations without affecting existing JSDoc support.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • [N/A] My code follows the accessibility standards.
  • [N/A] I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • [N/A] I've updated all React Native files affected by any refactorings/renamings in this PR.
@github-actions
Copy link

@github-actions github-actions bot commented Feb 21, 2021

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @sarayourfriend! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@github-actions
Copy link

@github-actions github-actions bot commented Feb 21, 2021

Size Change: +692 B (0%)

Total Size: 1.38 MB

Filename Size Change
build/api-fetch/index.js 3.4 kB -1 B (0%)
build/block-editor/index.js 124 kB +9 B (0%)
build/block-library/blocks/button/style-rtl.css 460 B -5 B (-1%)
build/block-library/blocks/button/style.css 460 B -4 B (-1%)
build/block-library/index.js 145 kB -35 B (0%)
build/block-library/style-rtl.css 8.81 kB -4 B (0%)
build/block-library/style.css 8.82 kB -2 B (0%)
build/blocks/index.js 48.3 kB +29 B (0%)
build/components/index.js 272 kB -4 B (0%)
build/core-data/index.js 16.7 kB -1 B (0%)
build/data/index.js 8.86 kB -1 B (0%)
build/edit-navigation/index.js 11 kB -2 B (0%)
build/edit-post/index.js 307 kB +2 B (0%)
build/edit-site/index.js 26.2 kB +578 B (+2%)
build/edit-site/style-rtl.css 4.41 kB +32 B (+1%)
build/edit-site/style.css 4.41 kB +31 B (+1%)
build/edit-widgets/index.js 20 kB -59 B (0%)
build/editor/index.js 42 kB +92 B (0%)
build/keyboard-shortcuts/index.js 2.54 kB +1 B (0%)
build/list-reusable-blocks/index.js 3.14 kB -1 B (0%)
build/media-utils/index.js 5.35 kB -2 B (0%)
build/plugins/index.js 2.55 kB -1 B (0%)
build/reusable-blocks/index.js 3.77 kB +8 B (0%)
build/reusable-blocks/style-rtl.css 225 B +17 B (+8%) 🔍
build/reusable-blocks/style.css 225 B +17 B (+8%) 🔍
build/rich-text/index.js 13.4 kB -2 B (0%)
build/server-side-render/index.js 2.76 kB -1 B (0%)
build/viewport/index.js 1.85 kB +1 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.78 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 665 B 0 B
build/block-directory/index.js 9.1 kB 0 B
build/block-directory/style-rtl.css 1.01 kB 0 B
build/block-directory/style.css 1.01 kB 0 B
build/block-editor/style-rtl.css 12.1 kB 0 B
build/block-editor/style.css 12.1 kB 0 B
build/block-library/blocks/archives/editor-rtl.css 61 B 0 B
build/block-library/blocks/archives/editor.css 60 B 0 B
build/block-library/blocks/audio/editor-rtl.css 58 B 0 B
build/block-library/blocks/audio/editor.css 58 B 0 B
build/block-library/blocks/audio/style-rtl.css 103 B 0 B
build/block-library/blocks/audio/style.css 103 B 0 B
build/block-library/blocks/block/editor-rtl.css 161 B 0 B
build/block-library/blocks/block/editor.css 161 B 0 B
build/block-library/blocks/button/editor-rtl.css 475 B 0 B
build/block-library/blocks/button/editor.css 474 B 0 B
build/block-library/blocks/buttons/editor-rtl.css 233 B 0 B
build/block-library/blocks/buttons/editor.css 233 B 0 B
build/block-library/blocks/buttons/style-rtl.css 303 B 0 B
build/block-library/blocks/buttons/style.css 303 B 0 B
build/block-library/blocks/calendar/style-rtl.css 208 B 0 B
build/block-library/blocks/calendar/style.css 208 B 0 B
build/block-library/blocks/categories/editor-rtl.css 84 B 0 B
build/block-library/blocks/categories/editor.css 83 B 0 B
build/block-library/blocks/categories/style-rtl.css 79 B 0 B
build/block-library/blocks/categories/style.css 79 B 0 B
build/block-library/blocks/code/style-rtl.css 90 B 0 B
build/block-library/blocks/code/style.css 90 B 0 B
build/block-library/blocks/columns/editor-rtl.css 190 B 0 B
build/block-library/blocks/columns/editor.css 190 B 0 B
build/block-library/blocks/columns/style-rtl.css 421 B 0 B
build/block-library/blocks/columns/style.css 421 B 0 B
build/block-library/blocks/cover/editor-rtl.css 390 B 0 B
build/block-library/blocks/cover/editor.css 389 B 0 B
build/block-library/blocks/cover/style-rtl.css 1.25 kB 0 B
build/block-library/blocks/cover/style.css 1.25 kB 0 B
build/block-library/blocks/embed/editor-rtl.css 486 B 0 B
build/block-library/blocks/embed/editor.css 486 B 0 B
build/block-library/blocks/embed/style-rtl.css 396 B 0 B
build/block-library/blocks/embed/style.css 395 B 0 B
build/block-library/blocks/file/editor-rtl.css 199 B 0 B
build/block-library/blocks/file/editor.css 198 B 0 B
build/block-library/blocks/file/style-rtl.css 248 B 0 B
build/block-library/blocks/file/style.css 248 B 0 B
build/block-library/blocks/freeform/editor-rtl.css 2.45 kB 0 B
build/block-library/blocks/freeform/editor.css 2.45 kB 0 B
build/block-library/blocks/gallery/editor-rtl.css 689 B 0 B
build/block-library/blocks/gallery/editor.css 690 B 0 B
build/block-library/blocks/gallery/style-rtl.css 1.07 kB 0 B
build/block-library/blocks/gallery/style.css 1.06 kB 0 B
build/block-library/blocks/group/editor-rtl.css 318 B 0 B
build/block-library/blocks/group/editor.css 317 B 0 B
build/block-library/blocks/group/style-rtl.css 57 B 0 B
build/block-library/blocks/group/style.css 57 B 0 B
build/block-library/blocks/heading/editor-rtl.css 129 B 0 B
build/block-library/blocks/heading/editor.css 129 B 0 B
build/block-library/blocks/heading/style-rtl.css 76 B 0 B
build/block-library/blocks/heading/style.css 76 B 0 B
build/block-library/blocks/html/editor-rtl.css 281 B 0 B
build/block-library/blocks/html/editor.css 281 B 0 B
build/block-library/blocks/image/editor-rtl.css 717 B 0 B
build/block-library/blocks/image/editor.css 716 B 0 B
build/block-library/blocks/image/style-rtl.css 477 B 0 B
build/block-library/blocks/image/style.css 478 B 0 B
build/block-library/blocks/latest-comments/editor-rtl.css 159 B 0 B
build/block-library/blocks/latest-comments/editor.css 158 B 0 B
build/block-library/blocks/latest-comments/style-rtl.css 269 B 0 B
build/block-library/blocks/latest-comments/style.css 269 B 0 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B 0 B
build/block-library/blocks/latest-posts/editor.css 137 B 0 B
build/block-library/blocks/latest-posts/style-rtl.css 523 B 0 B
build/block-library/blocks/latest-posts/style.css 522 B 0 B
build/block-library/blocks/list/editor-rtl.css 65 B 0 B
build/block-library/blocks/list/editor.css 65 B 0 B
build/block-library/blocks/list/style-rtl.css 63 B 0 B
build/block-library/blocks/list/style.css 63 B 0 B
build/block-library/blocks/media-text/editor-rtl.css 191 B 0 B
build/block-library/blocks/media-text/editor.css 191 B 0 B
build/block-library/blocks/media-text/style-rtl.css 535 B 0 B
build/block-library/blocks/media-text/style.css 532 B 0 B
build/block-library/blocks/more/editor-rtl.css 434 B 0 B
build/block-library/blocks/more/editor.css 434 B 0 B
build/block-library/blocks/navigation-link/editor-rtl.css 395 B 0 B
build/block-library/blocks/navigation-link/editor.css 397 B 0 B
build/block-library/blocks/navigation-link/style-rtl.css 704 B 0 B
build/block-library/blocks/navigation-link/style.css 702 B 0 B
build/block-library/blocks/navigation/editor-rtl.css 1.34 kB 0 B
build/block-library/blocks/navigation/editor.css 1.34 kB 0 B
build/block-library/blocks/navigation/style-rtl.css 195 B 0 B
build/block-library/blocks/navigation/style.css 195 B 0 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B 0 B
build/block-library/blocks/nextpage/editor.css 395 B 0 B
build/block-library/blocks/page-list/editor-rtl.css 214 B 0 B
build/block-library/blocks/page-list/editor.css 214 B 0 B
build/block-library/blocks/page-list/style-rtl.css 527 B 0 B
build/block-library/blocks/page-list/style.css 526 B 0 B
build/block-library/blocks/paragraph/editor-rtl.css 109 B 0 B
build/block-library/blocks/paragraph/editor.css 109 B 0 B
build/block-library/blocks/paragraph/style-rtl.css 273 B 0 B
build/block-library/blocks/paragraph/style.css 273 B 0 B
build/block-library/blocks/post-author/editor-rtl.css 209 B 0 B
build/block-library/blocks/post-author/editor.css 209 B 0 B
build/block-library/blocks/post-author/style-rtl.css 183 B 0 B
build/block-library/blocks/post-author/style.css 184 B 0 B
build/block-library/blocks/post-comments-form/style-rtl.css 250 B 0 B
build/block-library/blocks/post-comments-form/style.css 250 B 0 B
build/block-library/blocks/post-content/editor-rtl.css 139 B 0 B
build/block-library/blocks/post-content/editor.css 139 B 0 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B 0 B
build/block-library/blocks/post-excerpt/editor.css 73 B 0 B
build/block-library/blocks/post-featured-image/editor-rtl.css 338 B 0 B
build/block-library/blocks/post-featured-image/editor.css 338 B 0 B
build/block-library/blocks/post-featured-image/style-rtl.css 100 B 0 B
build/block-library/blocks/post-featured-image/style.css 100 B 0 B
build/block-library/blocks/preformatted/style-rtl.css 63 B 0 B
build/block-library/blocks/preformatted/style.css 63 B 0 B
build/block-library/blocks/pullquote/editor-rtl.css 183 B 0 B
build/block-library/blocks/pullquote/editor.css 183 B 0 B
build/block-library/blocks/pullquote/style-rtl.css 316 B 0 B
build/block-library/blocks/pullquote/style.css 316 B 0 B
build/block-library/blocks/query-loop/editor-rtl.css 90 B 0 B
build/block-library/blocks/query-loop/editor.css 89 B 0 B
build/block-library/blocks/query-loop/style-rtl.css 315 B 0 B
build/block-library/blocks/query-loop/style.css 317 B 0 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B 0 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B 0 B
build/block-library/blocks/query-pagination/editor-rtl.css 270 B 0 B
build/block-library/blocks/query-pagination/editor.css 262 B 0 B
build/block-library/blocks/query-pagination/style-rtl.css 168 B 0 B
build/block-library/blocks/query-pagination/style.css 168 B 0 B
build/block-library/blocks/query/editor-rtl.css 159 B 0 B
build/block-library/blocks/query/editor.css 160 B 0 B
build/block-library/blocks/quote/editor-rtl.css 61 B 0 B
build/block-library/blocks/quote/editor.css 61 B 0 B
build/block-library/blocks/quote/style-rtl.css 169 B 0 B
build/block-library/blocks/quote/style.css 169 B 0 B
build/block-library/blocks/rss/editor-rtl.css 201 B 0 B
build/block-library/blocks/rss/editor.css 202 B 0 B
build/block-library/blocks/rss/style-rtl.css 290 B 0 B
build/block-library/blocks/rss/style.css 290 B 0 B
build/block-library/blocks/search/editor-rtl.css 165 B 0 B
build/block-library/blocks/search/editor.css 165 B 0 B
build/block-library/blocks/search/style-rtl.css 342 B 0 B
build/block-library/blocks/search/style.css 344 B 0 B
build/block-library/blocks/separator/editor-rtl.css 99 B 0 B
build/block-library/blocks/separator/editor.css 99 B 0 B
build/block-library/blocks/separator/style-rtl.css 236 B 0 B
build/block-library/blocks/separator/style.css 236 B 0 B
build/block-library/blocks/shortcode/editor-rtl.css 504 B 0 B
build/block-library/blocks/shortcode/editor.css 504 B 0 B
build/block-library/blocks/site-logo/editor-rtl.css 201 B 0 B
build/block-library/blocks/site-logo/editor.css 201 B 0 B
build/block-library/blocks/site-logo/style-rtl.css 117 B 0 B
build/block-library/blocks/site-logo/style.css 117 B 0 B
build/block-library/blocks/social-link/editor-rtl.css 164 B 0 B
build/block-library/blocks/social-link/editor.css 165 B 0 B
build/block-library/blocks/social-links/editor-rtl.css 696 B 0 B
build/block-library/blocks/social-links/editor.css 696 B 0 B
build/block-library/blocks/social-links/style-rtl.css 1.36 kB 0 B
build/block-library/blocks/social-links/style.css 1.36 kB 0 B
build/block-library/blocks/spacer/editor-rtl.css 302 B 0 B
build/block-library/blocks/spacer/editor.css 302 B 0 B
build/block-library/blocks/spacer/style-rtl.css 48 B 0 B
build/block-library/blocks/spacer/style.css 48 B 0 B
build/block-library/blocks/subhead/editor-rtl.css 99 B 0 B
build/block-library/blocks/subhead/editor.css 99 B 0 B
build/block-library/blocks/subhead/style-rtl.css 80 B 0 B
build/block-library/blocks/subhead/style.css 80 B 0 B
build/block-library/blocks/table/editor-rtl.css 489 B 0 B
build/block-library/blocks/table/editor.css 489 B 0 B
build/block-library/blocks/table/style-rtl.css 386 B 0 B
build/block-library/blocks/table/style.css 386 B 0 B
build/block-library/blocks/tag-cloud/editor-rtl.css 118 B 0 B
build/block-library/blocks/tag-cloud/editor.css 118 B 0 B
build/block-library/blocks/tag-cloud/style-rtl.css 94 B 0 B
build/block-library/blocks/tag-cloud/style.css 94 B 0 B
build/block-library/blocks/template-part/editor-rtl.css 557 B 0 B
build/block-library/blocks/template-part/editor.css 556 B 0 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B 0 B
build/block-library/blocks/text-columns/editor.css 95 B 0 B
build/block-library/blocks/text-columns/style-rtl.css 166 B 0 B
build/block-library/blocks/text-columns/style.css 166 B 0 B
build/block-library/blocks/verse/editor-rtl.css 62 B 0 B
build/block-library/blocks/verse/editor.css 62 B 0 B
build/block-library/blocks/verse/style-rtl.css 87 B 0 B
build/block-library/blocks/verse/style.css 87 B 0 B
build/block-library/blocks/video/editor-rtl.css 504 B 0 B
build/block-library/blocks/video/editor.css 503 B 0 B
build/block-library/blocks/video/style-rtl.css 193 B 0 B
build/block-library/blocks/video/style.css 193 B 0 B
build/block-library/common-rtl.css 1.08 kB 0 B
build/block-library/common.css 1.08 kB 0 B
build/block-library/editor-rtl.css 9.05 kB 0 B
build/block-library/editor.css 9.04 kB 0 B
build/block-library/theme-rtl.css 748 B 0 B
build/block-library/theme.css 748 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 11.1 kB 0 B
build/customize-widgets/index.js 4.08 kB 0 B
build/customize-widgets/style-rtl.css 168 B 0 B
build/customize-widgets/style.css 168 B 0 B
build/data-controls/index.js 830 B 0 B
build/date/index.js 31.8 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 576 B 0 B
build/dom/index.js 4.94 kB 0 B
build/edit-navigation/style-rtl.css 1.26 kB 0 B
build/edit-navigation/style.css 1.25 kB 0 B
build/edit-post/style-rtl.css 6.81 kB 0 B
build/edit-post/style.css 6.8 kB 0 B
build/edit-widgets/style-rtl.css 3.2 kB 0 B
build/edit-widgets/style.css 3.2 kB 0 B
build/editor/editor-styles-rtl.css 543 B 0 B
build/editor/editor-styles.css 545 B 0 B
build/editor/style-rtl.css 3.89 kB 0 B
build/editor/style.css 3.89 kB 0 B
build/element/index.js 4.61 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.77 kB 0 B
build/format-library/style-rtl.css 637 B 0 B
build/format-library/style.css 639 B 0 B
build/hooks/index.js 2.28 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 4.01 kB 0 B
build/is-shallow-equal/index.js 698 B 0 B
build/keycodes/index.js 1.95 kB 0 B
build/list-reusable-blocks/style-rtl.css 629 B 0 B
build/list-reusable-blocks/style.css 628 B 0 B
build/notices/index.js 1.85 kB 0 B
build/nux/index.js 3.41 kB 0 B
build/nux/style-rtl.css 731 B 0 B
build/nux/style.css 727 B 0 B
build/primitives/index.js 1.42 kB 0 B
build/priority-queue/index.js 790 B 0 B
build/react-i18n/index.js 1.45 kB 0 B
build/redux-routine/index.js 2.83 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 3.01 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@gziolo gziolo added this to Needs review in Core JS Feb 22, 2021
return getReturnTypeAnnotation( tag, token );
}
default: {
return tag.type;

This comment has been minimized.

@nosolosw

nosolosw Feb 22, 2021
Member

I lifted the tag.type check here and that uncovered this path: this is when a type wasn't documented by JSDoc and there's no TS info we can extract from the ASTNode either. I suppose return an empty string or "unknown type" should be fine here.

This comment has been minimized.

@sarayourfriend

sarayourfriend Feb 22, 2021
Author Contributor

That's a good point. I think in that case an empty string is fine.

* @param {babelTypes.TSImportType} typeAnnotation
*/
function getImportTypeAnnotation( typeAnnotation ) {
// Should this just return the unqualified name (i.e., typeAnnotation.name || typeAnnotation.right.name)?

This comment has been minimized.

@sarayourfriend

sarayourfriend Feb 22, 2021
Author Contributor

This is still an outstanding question to me... I think we should just use the unqualified name without the import, for stylistic reasons, but maybe using the import would be best. In any case, import types are rare in actual TypeScript so it wouldn't happen often. The key is to use descriptive and contextually appropriate type names anyway.


/**
* @param {TypeAnnotation} typeAnnotation
* @return {string | null} The type or null if not an identifiable type.

This comment has been minimized.

@sarayourfriend

sarayourfriend Feb 22, 2021
Author Contributor

Suggested change
* @return {string | null} The type or null if not an identifiable type.
* @return {string} The type or an empty string if not an identifiable type.
case 'TSBooleanKeyword': {
return 'boolean';
}
case 'TSConditionalType': {

This comment has been minimized.

@sarayourfriend

sarayourfriend Feb 22, 2021
Author Contributor

Couldn't figure out how to make one of these. If anyone knows, would be much appreciated 😃

case 'TSConstructorType': {
return `new ${ getFunctionTypeAnnotation( typeAnnotation ) }`;
}
case 'TSExpressionWithTypeArguments': {

This comment has been minimized.

@sarayourfriend

sarayourfriend Feb 22, 2021
Author Contributor

Same here, no idea what this expression with type arguments is 🤔

typeAnnotation.parameterName.name
} is ${ getTypeAnnotation( typeAnnotation.typeAnnotation ) }`;
}
case 'TSTypeQuery': {

This comment has been minimized.

@sarayourfriend

sarayourfriend Feb 22, 2021
Author Contributor

Don't know what this is either

return getReturnTypeAnnotation( tag, token );
}
default: {
return tag.type;

This comment has been minimized.

@sarayourfriend

sarayourfriend Feb 22, 2021
Author Contributor

That's a good point. I think in that case an empty string is fine.

@nosolosw
Copy link
Member

@nosolosw nosolosw commented Feb 22, 2021

I think this is a good compromise: it addresses the concerns about having different parses for code and documentation and only requires us to add new TS types when we update the babel version if there are new ones. 👏

I was reading the code and pushed a few things to understand and clarify the flow, I hope you don't mind.

The only blocker I see for this to land is having tests. Happy to help with this. To land this sooner, we don't need to cover all the TS types available via babel, though, we can just add in this PR the flow + the TS types in use by the react-i18n package. All the rest can come in subsequent PRs. I've pushed some initial things, where can I see the babel nodes structure to add tests for the TS extraction?

@sarayourfriend
Copy link
Contributor Author

@sarayourfriend sarayourfriend commented Feb 22, 2021

https://astexplorer.com is the best place I've found, just make sure to choose the @babel/parser and enable the TypeScript plugin (and disable flow) otherwise you'll get the wrong types (like I did for the first 2 hours I spent working on this 😱).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Core JS
  
Needs review
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants