-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Bug/24286 create block error #24287
Bug/24286 create block error #24287
Conversation
packages/blocks/src/api/factory.js
Outdated
*/ | ||
export function createBlock( name, attributes = {}, innerBlocks = [] ) { | ||
// Get the type definition associated with a registered block. | ||
const blockType = getBlockType( name ); | ||
|
||
if ( undefined === blockType ) { | ||
warning( `Block type '${ name }' is not registered.` ); | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making this non-blocking changes the expectation from someone calling this function.
We do things like that in general in Gutenberg configs.map( config => createBlock( config.name, config.attributes, config.innerBlocks )
With the current implementation, this will hide bugs and just move them down the road. IMO, we should throw an exception here instead of calling warning
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@youknowriad thanks for the review here! I have updated the PR to throw an exception.
@@ -46,6 +46,10 @@ export function createBlock( name, attributes = {}, innerBlocks = [] ) { | |||
// Get the type definition associated with a registered block. | |||
const blockType = getBlockType( name ); | |||
|
|||
if ( undefined === blockType ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not important but yoda conditions are not a guideline in JavaScript because we enforce strict equality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR. The error message is way clearer this way.
closes #24286
Description
This PR implements the proposed solution indicated in #24286
How has this been tested?
Running `wp.blocks.createBlock('not/registered') in the console.
Screenshots
Types of changes
This introduces a warning() call and an early return if the blockType is not registered
Checklist: