chore: add typedefs for registerBlockType

[chaance]

Description

Adding JSDoc typedefs for registerBlockType.

As I'm a new contributor, I would love some feedback on this before spending too much time on the items marked TODO. I'm also unsure if there is a set standard for documenting function properties in typedefs (re: edit/save, JSDoc isn't super clear on a standard here and userland suggestions seem to be all over the place). There has to be a way to be more specific than just declaring Function!

How has this been tested?

Checked against other typedefs in the project and my approach seems consistent and satisfies lint rules. Intellisense in VS Code works as expected.

Types of changes

Only JSDoc + a new .gitignore entry to avoid adding project-level editor settings.

[aduth]

cc @dsifford , as I wonder if you might have some insight to add here with regard to some of the questions (function documentation, enumerables).

I haven't personally seen any pattern (in this project or elsewhere) for detailed function-type documentation in JSDoc.

[aduth]

Historically we've advocated that this be included in a developer's own global .gitignore to avoid a proliferation of user-specific entries in this file, but this has definitely come up on multiple occasions and I'm curious to potentially consider again (for developer experience' sake) whether to make exceptions at least for the most common cases.

cc @gziolo

[aduth]

It's a bit outside the scope of what you're trying to accomplish here, and not of huge consequence one way or the other, but perhaps a topic worth covering in tomorrow's weekly JavaScript chat in Slack.

Edit: I added a topic to the agenda

[aduth]

Do you have a reference for this syntax of defining options of a set that you can share?

I can't find anything about it on the @type documentation, and my experience with similar options like @enum are they aren't quite what we're looking for for what you're expressing here.

[chaance]

I had to dig for it, but this came from jsdoc/jsdoc#629 (comment). So it's not documented as the "official" solution but it is parsed correctly, and this seemed more appropriate than an enum here.

[aduth]

Makes sense to me!

The only thing I might suggest is that if a block is to be registered with a single category, it would make sense that this be singular "Category" as well, vs. "Categories"? Similar to WPBlockAttributeType and WPBlockAttributeSource.

[dsifford]

Yep, this is valid... String union type 👍

The surrounding parens aren't required though. But that's a matter of style.

[aduth]

As minor points of code style convention (not currently enforced at this level of detail):

• Consider to wrap at 80-100 line length (reference guideline
• Consider to align respective "sections" of a grouping (i.e. horizontally aligned types, names, descriptions within a @property grouping) (reference guideline, though it should be noted the document in general has fallen somewhat out of date)

Suggested change

	* @property {string} meta Attributes may be obtained from a post’s meta rather than from the block’s representation in saved post content. For this, an attribute is required to specify its corresponding meta key under the meta key
	* @property {string} meta Attributes may be obtained from post
	* meta rather than from the block’s
	* representation in saved post content.
	* For this, an attribute is required to
	* specify its corresponding meta key
	* under the meta key.

[chaance]

Noted, thanks! This entire block is marked as TODO but I will format it accordingly after adding all of the descriptions and correct types.

[dsifford]

Looks like a good start to me! Nice work @chancestrickland!

[dsifford]

Is this type valid? I haven't seen any written like this before.

There are other variants of this that are valid so this may be a matter of style...

Examples:

// Using a `Record` type
/**
 * @typedef {Record<string, WPBlockAtributeOptions>} WPBlockAttributes
 */

// Using an indexed type
/**
 * @typedef { {[k: string]: WPBlockAttributeOptions} } WPBlockAttributes
 */

[chaance]

https://jsdoc.app/tags-type.html > third row under syntax examples uses the same syntax. I don't have a style preference though so if we're using a different syntax elsewhere I am happy to conform.

[aduth]

I find it to be pretty intuitive at it's proposed in this pull request, and it's a syntax I'm familiar with using in my own projects.

[aduth]

Some prior art (omitting ., which upon reflection I am uncertain whether to be valid):

gutenberg/bin/packages/build-worker.js

Line 83 in 711f1f7

* @type {Object<string,Function>}

gutenberg/packages/block-editor/src/store/reducer.js

Line 1274 in 711f1f7

* @param {Object<string,Object>} state Current state.

gutenberg/packages/block-library/src/columns/utils.js

Line 87 in 711f1f7

* @return {Object<string,number>} Column widths.

gutenberg/packages/block-library/src/columns/utils.js

Line 106 in 711f1f7

* @return {Object<string,number>} Redistributed column widths.

gutenberg/packages/blocks/src/api/serializer.js

Lines 147 to 150 in 711f1f7

	* @param {Object<string,*>} blockType Block type.
	* @param {Object<string,*>} attributes Attributes from in-memory block data.
	*
	* @return {Object<string,*>} Subset of attributes for comment serialization.

gutenberg/packages/compose/src/higher-order/with-global-events/index.js

Line 32 in 711f1f7

* @param {Object<string,string>} eventTypesToHandlers Object with keys of DOM

gutenberg/packages/e2e-tests/config/setup-test-framework.js

Line 31 in 711f1f7

* @type {Object<string,string>}

gutenberg/packages/viewport/src/listener.js

Line 28 in 711f1f7

* @type {Object<string,MediaQueryList>}

[dsifford]

Cool. Looks good to me assuming the typescript compiler is cool with that.

(also I agree no dot is better)

[dsifford]

Yep, this is valid... String union type 👍

The surrounding parens aren't required though. But that's a matter of style.

[dsifford]

I see that this is marked todo, but see here for what I'd recommend typing this as....

(same for save below)

[dsifford]

I guess I should say that I do understand that generics aren't as well supported in JSDoc, but ComponentType<any> should work at the very least!

[chaance]

@aduth @dsifford I believe this should be ready for review. I went ahead and implemented most of the prior feedback, but let me know if I missed anything or if something looks off.

[aduth]

Two surface-level observations:

• Currently, the generated documentation resides in the repository, and needs to be re-generated when associated code comments are changed. Can you run npm run docs:build and commit the resulting changes? This will allow the failing "Build artifacts" Travis job to pass.
• There is a conflict in the branch.

In the meantime, I'll give a look over the specific changes.

[aduth]

Hm, is it fair to call this a callback? It's a component, which in many cases will be something of a callback (function component), but not always. And while technically a Component class is a function, I'm not sure "callback" can apply to a value which needs to be constructed?

https://reactjs.org/docs/components-and-props.html

I suppose the main purpose here is in providing a type for the props it can expect to receive? Ultimately that seems to go back to the purpose propTypes is meant to serve. I could imagine if we adopted propTypes, we might be able to integrate this into the docs tooling to extract documentation for component props.

Then again, with the direct React is taking, we seem to be reaching a point where there's few reasons anyone should ever create a new component that's not a function component, in which case the documentation as you've proposed here might be the most reasonable approach. And above all, it would be good to have something available to document the expected props when used this way.

[chaance]

I would agree in theory, unfortunately I couldn't get JSDoc to understand using any other syntax for documenting functions and their arguments in a detailed manner. I tried @function and just adding @param tags to the typedefs but the parser was not satisfied AFAICT. Perhaps it's just a limitation we accept, use a generic Function and just explain the parameters and return type in the comment block?

[aduth]

I would agree in theory, unfortunately I couldn't get JSDoc to understand using any other syntax for documenting functions and their arguments in a detailed manner.

In a side project, I've had luck with the following:

/**
 * @typedef {function(SLAuth): Promise<SLStream[]>} SLProviderGetStreams
 */

(specifically the {function(SomeArgumentType): SomeReturnType} format)

The TypeScript checking verifies the parameter:

...and has Intellisense for the result:

[dsifford]

Anything between the brackets in a typedef is parsed identically to how typescript type definitions are parsed...

So if you have a function that looks like this...

function foo(someString) {
  return Promise.resolve(someString);
}

You could make a typedef like this...

/**
 * @typedef {(someString: string) => Promise<string>} FooFunc
 */

Or you could reference it inline like this...

/**
 * Function that takes foo as a parameter.
 * @prop {(someString: string) => Promise<string>} fooFunc
 */
function takeFooFunc(fooFunc) { /* ... */ }

[chaance]

That's good to know about TS compatibility, thanks! I did get arrow function notation to work with VS Code at least but not seeing a single example anywhere in the JSDoc spec made me nervous about committing to that approach 😅

[aduth]

Thanks @dsifford , I learned something new!

@chancestrickland : If it's compatible with (a) TypeScript, (b) eslint-plugin-jsdoc, and (c) our documentation generator†, I think it should be fine. I like in the alternate forms @dsifford presents that it gives an option to name the arguments (in my prior comment, it would show as arg0).

[dsifford]

@chancestrickland Yep, I guess I should have mentioned that the way I describe is not strictly aligned with the rules of jsdoc proper. But the editor features and code hints and all that good stuff that are parsed from these type definitions are done so using Microsoft's tsdoc standards, which is an extension of jsdoc.

So, I suppose it should also be mentioned that doing it this was would probably not pair well with a documentation generator that uses the vanilla jsdoc parser.

[aduth]

Should we / do we need to document that all of these properties are optional?

For documentation, it could be useful to describe how they're associated, even if by types it might not be possible to enforce.

e.g.

• A value for selector is optionally supported when source is one of the HTML-based sources
• A value for attribute is required when source: 'attribute'
• A value for meta is required when source: 'meta'

[aduth]

Per consistency with below, and some previous conversations about minimizing line length, I assume we might want to remove the spaces around |.

[aduth]

FWIW, many conventions support omitting @return when the return value is undefined (including TypeScript?), and I think this is the general approach we've taken in the rest of the project.

If a function that is not in externs has no return value, you can omit the @return tag, and the compiler will assume that the function returns undefined.

https://developers.google.com/closure/compiler/docs/js-for-compiler#tag-return

[dsifford]

Just chiming in here for a bit of added distinction...

As far as typescript is concerned, a function that returns undefined must return either undefined or simply return; for all code paths.

A function that returns void may or may not return undefined somewhere, but does not need to return in all code paths.

[aduth]

Should there be a description?

[aduth]

This description is inaccurate (I believe it's copy-pasted from the prior typedef).

[aduth]

I don't think all of these options are accurate?

'number', 'string', and 'integer' are not options for an attribute source, and should be removed.

Conversely, there's a few missing, though they're not any I think we should really be recommending: 'children', 'node', 'property', 'tag'.

[aduth]

Since a plugin or theme can add their own custom options, I think we can't assume it would be part of this limited set.

https://developer.wordpress.org/block-editor/developers/filters/block-filters/#managing-block-categories

[aduth]

I understand there may not be a desire or availability to continue work here, and at the time some of the points around specific syntaxes were still rather unclear in the project. That said, I would very much like to see someone pick up this effort again in the future, especially if it can be integrated into documentation to help clarify available options.

There's been quite a bit of work since this was originally opened to improve on how we author and use type definitions:

• Testing: Add TypeScript types validation for modules #18838: Testing: Add TypeScript types validation for modules
• Docs: Add Gutenberg-specific JSDoc recommendations #18920: Docs: Add Gutenberg-specific JSDoc recommendations
• Build: Output package type declarations #18942: Build: Output package type declarations
• Replace espree and doctrine with typescript in docgen. #21238: Replace espree and doctrine with typescript in docgen