-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Block API: Normalize block type function argument #11490
Conversation
Thoughts on?
|
return getBlockType( blockTypeOrName ); | ||
} | ||
|
||
return blockTypeOrName; |
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.
A general question (not a blocker!) about our public APIs: Should we be validating their inputs more strictly and e.g. throwing an error if a number or boolean is passed here?
My fear is that we may lose some flexibility to change APIs in the future if there are popular plugins out there that are using our APIs incorrectly.
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.
Should we be validating their inputs more strictly and e.g. throwing an error if a number or boolean is passed here?
As bad as it sounds, wouldn't it be a breaking change? :)
In overall, this is something we should pay more attention to in the 2nd phase of the project.
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.
My 2¢ : While laudable, as a general pattern it's hard to anticipate all nonsense inputs, and in setting expectations of tolerance while being unable to deliver (or deliver consistently), we're doing more a disservice than we're helping.
9a238d5
to
896e4b9
Compare
I added change for:
See 896e4b9.
The thing is that
|
// When retrieving transforms for all block types, recurse into self. | ||
if ( blockName === undefined ) { | ||
if ( blockTypeOrName === undefined ) { | ||
return flatMap( | ||
getBlockTypes(), | ||
( { name } ) => getBlockTransforms( direction, name ) |
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.
This could be partial( getBlockTransforms, direction )
. I dunno that I'd care to change it though (given partial
's questionable future)
896e4b9
to
c169793
Compare
Description
Closes #5227.
This PR adds new internal helper method
normalizeBlockType
which makes use of the following methods more convenient for all plugin developers:getBlockAttributes
getSaveContent
getSaveElement
isValidBlockContent
It's going to be possible to pass either block's name as string or block type object as an object as the first param for all of the aforementioned methods. In the majority of cases passing string removes some boilerplate code.
How has this been tested?
All unit and e2e tests still pass.
Types of changes
Refactoring and code enhancement.
Checklist: