-
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
Block Editor: Warn when useBlockProps
hook used with a wrong Block API version
#33274
Conversation
Size Change: +5.46 kB (+1%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
* | ||
* @type {Record<string, true | undefined>} | ||
*/ | ||
export const logged = Object.create( null ); |
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.
Is it Object.create( null )
instead of just {}
to avoid prototype pollution? Very clever! 👍
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.
Yes, I copied it over from @wordpress/deprecated
.
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.
Would we be better off with Set
?
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.
Good point, we could use Set
👍🏻
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.
#33380 should address it.
I see the warning even when I'm just hovering over the block on the block inserter (I guess that's because the preview is calling the hook). Is this expected? |
@@ -128,6 +132,12 @@ export function useBlockProps( props = {}, { __unstableIsHtml } = {} ) { | |||
} ), | |||
] ); | |||
|
|||
if ( blockAPIVersion < 2 ) { | |||
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.
Should we error or warn?
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.
warning
is our custom helper that works nearly the same as React warnings that get applied only for development builds.
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.
Looks good to me, though it's not clear to me why we have to downgrade the heading block.
…API version (#33274) * Block Editor: Warn when `useBlockProps` hook used with a wrong Block API version * Revert apiVersion change in the Heading block
Description
API version mismatch when using
useBlockProps
seems to be the most common issue reported recently. This PR proposes that we detect when someone is usinguseBlockProps
withapiVersion
set to1
for a block type that is using alsouseBlockProps
. The warning is going to be printed only in the development mode.@wordpress/warning
changesThis PR also ensures that a given unique message is reported only once when calling
warning
method from@wordpress/warning
. It mirrors the implementation used withdeprecated
from@wordpress/deprecated
. The rationale is to avoid seeing a flood of the same messages reported on the Browser Console. It was very annoying for theuseBlockProps
hook that would print the warning for every block and on every re-render.How has this been tested?
Create a new post and insert the Heading block that has the
apiVersion
downgraded to1
to throw the warning. Observe the warning on the Browser Console.Screenshots
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).