@wordpress/blocks: Register block from metadata #55245

ofhouse · 2021-08-19T14:43:23Z

Hi,

the registerBlockType method now accepts metadata input from the block.json metadata.

Changes

Extends the registerBlockType to accept the metadata as first and optionally second argument.
Extends the Block type with the missing properties from the metadata (https://developer.wordpress.org/block-editor/reference-guides/block-api/block-registration)

Related docs

This was introduced in version 9.1.0: https://github.com/WordPress/gutenberg/blob/trunk/packages/blocks/CHANGELOG.md#910-2021-05-20
Related PR: Blocks: Deprecate registerBlockTypeFromMetadata in favor of registerBlockType WordPress/gutenberg#32030

Please fill in this template.

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Add or edit tests to reflect the change.
Follow the advice from the readme.
Avoid common mistakes.
Run npm test <package to test>.

Select one of these and delete the others:

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes: <>
If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.

registerBlockType now accepts input from the block.json metadata WordPress/gutenberg#32030

typescript-bot · 2021-08-19T14:44:00Z

@ofhouse Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

wordpress__blocks — on npm, on unpkg
- owner-approval: @dmsnell

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

You can test the changes of this PR in the Playground.

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
✅ Most recent commit is approved by type definition owners, DT maintainers or others

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 55245,
  "author": "ofhouse",
  "headCommitOid": "5124723a68244a49b68a49a9a5172736be783377",
  "lastPushDate": "2021-09-23T07:22:42.000Z",
  "lastActivityDate": "2021-09-23T18:49:45.000Z",
  "mergeOfferDate": "2021-09-23T18:46:45.000Z",
  "mergeRequestDate": "2021-09-23T18:49:45.000Z",
  "mergeRequestUser": "dmsnell",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "popularityLevel": "Well-liked by everyone",
  "pkgInfo": [
    {
      "name": "wordpress__blocks",
      "kind": "edit",
      "files": [
        {
          "path": "types/wordpress__blocks/api/registration.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/wordpress__blocks/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/wordpress__blocks/wordpress__blocks-tests.tsx",
          "kind": "test"
        }
      ],
      "owners": [
        "dsifford",
        "sirreal",
        "dmsnell"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "dmsnell",
      "date": "2021-09-23T18:46:09.000Z",
      "isMaintainer": false
    }
  ],
  "mainBotCommentID": 901975566,
  "ciResult": "pass"
}

typescript-bot · 2021-08-19T14:44:02Z

🔔 @dsifford @sirreal @dmsnell — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

types/wordpress__blocks/api/registration.d.ts

types/wordpress__blocks/index.d.ts

dmsnell · 2021-08-19T15:13:12Z

types/wordpress__blocks/index.d.ts

+        readonly attributes: {
+            readonly [k in keyof T]: any;
+        };
+        readonly innerBlocks?: any[];


this isn't really any[] here since the innerBlocks need to be blocks. does Block<any> or Block<unknown> work?

Block<any> would not work since it only needs takes the name and attributes properties:

example: { attributes: { cover: 'https://example.com/image.jpg', }, innerBlocks: [ { name: 'core/paragraph', attributes: { /* translators: example text. */ content: __( 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Praesent et eros eu felis.' ), }, }, ], },

Example from here.

Would go with

readonly innerBlocks?: ReadonlyArray<{ name: string; attributes: Record<string, any> }>;

for now.

if we only need those properties that seems like we could use this to do that…

type InnerBlock = Partial<Block> & Pick<Block, 'name' | 'attributes'>;

this prevents us from needing to duplicate the definitions of the block types and also preserves the fact that in many (all?) cases those other properties will be there, even if not mandatory for some reason. otherwise we're saying, among other things, that it's not possible to have inner blocks which themselves contain inner blocks.

Would generally agree, but the attributes type seems different:
While Block.attributes expects a declarative object in the form of:

attributes: { foo: { source: 'text'; type: 'string'; } }

The attributes property of the innerBlocks seem to be passed by value:

attributes: { foo: 'bar' }

While Block.attributes expects a declarative object in the form of:

This is a good point. I think the block attributes types are a big confused at this point. There's the BlockConfiguration which takes those attribute declarations, but the Block objects themselves take attributes that are the real types, which cannot be those same types as given for the BlockConfiguration

don't let this hold up the PR. if we can't reuse Block['attributes'] as we ideally would, we can fix that later.

Ah okay, yes your proposed type seem to work fine with the example.
However looking at the implementation it seems that innerBlocks can be recursive:

https://github.com/WordPress/gutenberg/blob/87d761eb48820936f71568eb81d7130be4dfc25a/packages/blocks/src/api/factory.js#L579-L587

I've now updated the type to:

type BlockExampleInnerBlock = ReadonlyArray< Partial<Block> & Pick<Block, 'name' | 'attributes'> & { innerBlocks?: BlockExampleInnerBlock; } >;

it seems that innerBlocks can be recursive:

yep! that's one of the reasons I brought it up.

though…we shouldn't limit it to BlockExampleInnerBlock. I think in this case Pick<Block, 'name' | 'attributes' | 'innerBlocks'> would work, otherwise what we're saying is not just that the block can be recursive, but that it can only be recursive on its own type.

consider the group block; it can have child blocks of any type because its role is to be a kind of neutral container. if we kept this definition then we wouldn't be able to show any meaningful example for a column block because it would demand we only allow group blocks to be the children of a group block.

my last comment was wrong; sorry for saying otherwise. I didn't see at first what you were doing here.

concretely though I think we might want to drop attributes because not every block requires it and I don't want to overconstrain our types on the example code. on the example property that means maybe mark it also as optional or do this

example?: Readonly<Partial<Block> & Pick<Block, 'name'> & {innerBlocks: ReadonlyArray<BlockExampleInnerBlock>}>;

that's the only thing I think that would likely need to change here. I don't want to hold up this PR and I'm afraid I made comments pushing you to something that I'm now suggesting walking back from. if you don't agree with me I'll still give my approval - these make the types better. primarily I just want to avoid falsely indicating a type error where none exists.

Sorry for the late response, thanks for the detailed explanations 👍
example.name path doesn't seem to be required from the docs, so I guess

example?: Readonly<Partial<Block> & { innerBlocks: ReadonlyArray<BlockExampleInnerBlock> }>;

is fine here?

types/wordpress__blocks/index.d.ts

DefinitelyTyped#55245 (comment)

typescript-bot · 2021-08-20T14:02:56Z

@dmsnell Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

DefinitelyTyped#55245 (comment)

typescript-bot · 2021-08-21T13:27:08Z

@dmsnell Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

typescript-bot · 2021-09-01T18:56:04Z

Re-ping @dsifford, @sirreal, @dmsnell:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

dmsnell

This all seems fine to me. We can update as we go if we find blockers.

typescript-bot · 2021-09-12T01:51:10Z

Re-ping @ofhouse / @dsifford, @sirreal:

This PR has been ready to merge for over a week, and I haven't seen any requests to merge it. I will close it on Oct 1st (in three weeks) if this doesn't happen.

(If there's no reason to avoid merging it, please do so. Otherwise, if it shouldn't be merged or if it needs more time, please close it or turn it into a draft.)

ZebulanStanphill · 2021-09-22T01:35:48Z

Could someone please merge this? Thanks.

ofhouse · 2021-09-22T07:52:19Z

Sorry for the delay, will try to take another look at it soon.

typescript-bot · 2021-09-22T18:07:44Z

@dmsnell Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

dmsnell · 2021-09-22T21:18:12Z

types/wordpress__blocks/index.d.ts

+     *
+     * @see {@link https://developer.wordpress.org/block-editor/reference-guides/block-api/block-metadata/#example}
+     */
+    readonly example?: Readonly<Partial<Block> & { innerBlocks: ReadonlyArray<BlockExampleInnerBlock> }>;


quick update here: I believe that we need to make innerBlocks optional here otherwise people will have to provide inner blocks in their examples even if the blocks don't support them.

… & { innerBlocks?: ReadonlyArray<BlockExampleInnerBlock> }

Yes, good point. Fixed it 👍

dmsnell · 2021-09-22T21:18:28Z

One quick update and then happy to get this merged.

typescript-bot · 2021-09-23T07:29:35Z

@dmsnell Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

dmsnell

Thanks!

dmsnell · 2021-09-23T18:49:45Z

Ready to merge

ZebulanStanphill · 2021-09-23T20:49:29Z

Thanks for merging this! Looks like there's still a bit of an issue though. The following should work, but the 2nd arg is considered invalid:

import { registerBlockType } from '@wordpress/blocks'
import edit from './edit'

const metadata = {
	apiVersion: 2,
	name: 'foo/bar',
	category: 'widgets',
	attributes: {},
	example: {},
	title: 'Block title',
	textdomain: 'foo',
	script: 'file:./script.js',
	editorScript: 'file:./editor-script.js',
}

registerBlockType(metadata, { edit })

The problem is that the 2nd arg, while optional, still requires all the same props if present, whereas with the new signature, it shouldn't require any.

dmsnell · 2021-09-23T21:09:54Z

@ZebulanStanphill are you saying that the second argument should be Partial<BlockConfiguration<T>> or some variant of that?

any interest in submitting a small PR to fix that?

ZebulanStanphill · 2021-09-23T22:18:10Z

@dmsnell Here you go: #55960.

@wordpress/blocks: Register block from metadata

c5cc106

registerBlockType now accepts input from the block.json metadata WordPress/gutenberg#32030

typescript-bot added this to Waiting for Code Reviews in New Pull Request Status Board Aug 19, 2021

dmsnell reviewed Aug 19, 2021

View reviewed changes

types/wordpress__blocks/api/registration.d.ts Outdated Show resolved Hide resolved

dmsnell reviewed Aug 19, 2021

View reviewed changes

types/wordpress__blocks/index.d.ts Outdated Show resolved Hide resolved

dmsnell reviewed Aug 19, 2021

View reviewed changes

types/wordpress__blocks/index.d.ts Outdated Show resolved Hide resolved

ofhouse added 4 commits August 20, 2021 15:30

Derive example attributes from block

bfef66a

DefinitelyTyped#55245 (comment)

Stronger typing for example/innerBlocks

1c0f0ab

DefinitelyTyped#55245 (comment)

Provides stronger typings for providesContext

159546a

DefinitelyTyped#55245 (comment)

Rename generic type from T -> TAttributes

7178773

DefinitelyTyped#55245 (comment)

Derive innerBlocks from Block properties

4944fc3

DefinitelyTyped#55245 (comment)

typescript-bot added the Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. label Sep 1, 2021

dmsnell approved these changes Sep 1, 2021

View reviewed changes

typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Self Merge This PR can now be self-merged by the PR author or an owner and removed Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. labels Sep 1, 2021

typescript-bot moved this from Waiting for Code Reviews to Waiting for Author to Merge in New Pull Request Status Board Sep 1, 2021

typescript-bot added the Unmerged The author did not merge the PR when it was ready. label Sep 12, 2021

Reuse block props in example

d3378ce

typescript-bot removed Unmerged The author did not merge the PR when it was ready. Self Merge This PR can now be self-merged by the PR author or an owner Owner Approved A listed owner of this package signed off on the pull request. labels Sep 22, 2021

typescript-bot moved this from Waiting for Author to Merge to Waiting for Code Reviews in New Pull Request Status Board Sep 22, 2021

dmsnell reviewed Sep 22, 2021

View reviewed changes

Make innerBlocks in example optional

5124723

dmsnell approved these changes Sep 23, 2021

View reviewed changes

typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Self Merge This PR can now be self-merged by the PR author or an owner labels Sep 23, 2021

typescript-bot moved this from Waiting for Code Reviews to Waiting for Author to Merge in New Pull Request Status Board Sep 23, 2021

typescript-bot moved this from Waiting for Author to Merge to Recently Merged in New Pull Request Status Board Sep 23, 2021

typescript-bot merged commit 01442fa into DefinitelyTyped:master Sep 23, 2021

typescript-bot removed this from Recently Merged in New Pull Request Status Board Sep 23, 2021

ofhouse deleted the wordpress__blocks-registerblocktype branch September 23, 2021 19:55

ZebulanStanphill mentioned this pull request Sep 23, 2021

@wordpress/blocks: allow Partial BlockConfiguration as 2nd arg when 1st arg is metadata. #55960

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

@wordpress/blocks: Register block from metadata #55245

@wordpress/blocks: Register block from metadata #55245

ofhouse commented Aug 19, 2021 •

edited

typescript-bot commented Aug 19, 2021 •

edited

typescript-bot commented Aug 19, 2021

dmsnell Aug 19, 2021

ofhouse Aug 20, 2021

dmsnell Aug 20, 2021

ofhouse Aug 20, 2021

dmsnell Aug 20, 2021

ofhouse Aug 21, 2021

dmsnell Aug 23, 2021

dmsnell Aug 23, 2021

dmsnell Aug 23, 2021

ofhouse Sep 22, 2021

typescript-bot commented Aug 20, 2021

typescript-bot commented Aug 21, 2021

typescript-bot commented Sep 1, 2021

dmsnell left a comment

typescript-bot commented Sep 12, 2021

ZebulanStanphill commented Sep 22, 2021

ofhouse commented Sep 22, 2021

typescript-bot commented Sep 22, 2021

dmsnell Sep 22, 2021

ofhouse Sep 23, 2021

dmsnell commented Sep 22, 2021

typescript-bot commented Sep 23, 2021

dmsnell left a comment

dmsnell commented Sep 23, 2021

ZebulanStanphill commented Sep 23, 2021 •

edited

dmsnell commented Sep 23, 2021

ZebulanStanphill commented Sep 23, 2021

@wordpress/blocks: Register block from metadata #55245

@wordpress/blocks: Register block from metadata #55245

Conversation

ofhouse commented Aug 19, 2021 • edited

Changes

Related docs

typescript-bot commented Aug 19, 2021 • edited

1 package in this PR

Code Reviews

Status

typescript-bot commented Aug 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

typescript-bot commented Aug 20, 2021

typescript-bot commented Aug 21, 2021

typescript-bot commented Sep 1, 2021

dmsnell left a comment

Choose a reason for hiding this comment

typescript-bot commented Sep 12, 2021

ZebulanStanphill commented Sep 22, 2021

ofhouse commented Sep 22, 2021

typescript-bot commented Sep 22, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmsnell commented Sep 22, 2021

typescript-bot commented Sep 23, 2021

dmsnell left a comment

Choose a reason for hiding this comment

dmsnell commented Sep 23, 2021

ZebulanStanphill commented Sep 23, 2021 • edited

dmsnell commented Sep 23, 2021

ZebulanStanphill commented Sep 23, 2021

ofhouse commented Aug 19, 2021 •

edited

typescript-bot commented Aug 19, 2021 •

edited

ZebulanStanphill commented Sep 23, 2021 •

edited