Fix broken editor when passing attributes as undefined or null and add warning for block authors #73149
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
6 tasks
|
Size Change: +80 B (0%) Total Size: 2.54 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 82e6464. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19246788593
|
Member
|
Can anyone provide small example block plugin to reproduce the issue? Generally, I would expect bootstrapped or provided settings to omit the attributes property when not defined, instead of overriding the default value. P.S. I think proposal makes sense, just want to check why it's happening. |
Member
|
I would also keep in mind that if this is happening at this time, we might want to always set attributes as an empty object earlier, since it might otherwise happen again in another place. I would also encourage adding a couple of accompanying e2e tests. |
Contributor
Author
|
I added a
So, should we set attributes when it's passed as undefined/null? Or should we add a warning like we do for other invalid properties? @kmanijak - here's the list of other woocommerce blocks that hit the undefined attributes path: |
Contributor
|
Wow, that's a nice finding!
What about mixed solution:
BTW I'll fix those blocks in WooCommerce (FYI @Aljullu). |
Contributor
|
@jeryj, I think what you described may be a separate problem. Block that I found problematic was
That makes me think the actual problem is/was hidden somewhere else 🤔 @jeryj, are you still able to reproduce the problem? |
Contributor
|
One difference I see is:
I'll try to debug further later on but honestly without ability to reproduce the problem anymore, I don't see it productive. |
12 tasks
Contributor
Author
|
Register block type will accept either format. I don't see an issue with I'll update this PR to JUST add a console warning. That way it will help debug further and also not be any kind of breaking change. That should be very safe to merge and will at least give us more info. |
Contributor
Author
|
@kmanijak - I can still reproduce the error on Gutenberg trunk with the public latest WooCommerce release with default settings. I added the warning in at the earliest entry point (
Others also pass Screen.Recording.2025-11-13.at.10.13.48.AM.mov |
Mamaduka
reviewed
Nov 14, 2025
Comment on lines
260
to
275
| // Warn if attributes is explicitly set to undefined or null in metadata or settings. | ||
| if ( | ||
| ( isObject( blockNameOrMetadata ) && | ||
| 'attributes' in blockNameOrMetadata && | ||
| ( blockNameOrMetadata.attributes === null || | ||
| blockNameOrMetadata.attributes === undefined ) ) || | ||
| ( 'attributes' in settings && | ||
| ( settings.attributes === null || | ||
| settings.attributes === undefined ) ) | ||
| ) { | ||
| warning( | ||
| 'The block "' + | ||
| name + | ||
| '" must not register attributes as null or undefined. Use an empty object or exclude the attributes key.' | ||
| ); | ||
| } |
Member
There was a problem hiding this comment.
The processBlockType seems to be housing most of the similar wanings. I think the condition can also be simplified there.
Contributor
Author
There was a problem hiding this comment.
I was thinking that too originally, but putting it in the block registration captures it at the earliest possible case. If we wait until process block, then it may be that we are setting something incorrectly. Checking it here is the best warning for devs using the system. But I'm open to either way!
…thin registration" This reverts commit 43a1493.
… is a temporary fix in gutenberg
tyxla
reviewed
Nov 20, 2025
| ! blockType.attributes || | ||
| typeof blockType.attributes !== 'object' | ||
| ) { | ||
| warning( |
Member
There was a problem hiding this comment.
Can we also include a test for this?
tyxla
reviewed
Nov 20, 2025
Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
tyxla
reviewed
Nov 20, 2025
| warning( | ||
| 'The block "' + | ||
| name + | ||
| '" must not register attributes as `null` or `undefined`. Use an empty object (`attributes: {}`) or exclude the `attributes` key. In the next Gutenberg release, passing `null` or `undefined` for `attributes` will cause the block editor to crash.' |
Member
There was a problem hiding this comment.
Also, this "in the next release it will crash", isn't accurate, is it? I think we can omit that whole sentence.
Contributor
Author
There was a problem hiding this comment.
I wasn't sure. I figured having it be the most aggressive and then relaxing it would be better than starting with a longer timeline.
Member
There was a problem hiding this comment.
But it's technically not true, is it? I'd just remove it and keep the warning forever.
Contributor
Author
There was a problem hiding this comment.
But it's technically not true, is it?
I was planning to open a PR that removes it, set a reminder and then get it approved/merged when the time came :)
I'd just remove it and keep the warning forever.
That's fine. I'll just remove it.
tyxla
reviewed
Nov 20, 2025
| ), | ||
| }; | ||
|
|
||
| // Temporary fix to allow block authors time to fix blocks that register attributes as null or undefined. |
Member
There was a problem hiding this comment.
Doesn't have to be temporary. Folks might encounter this in the future, too.
Contributor
Author
There was a problem hiding this comment.
I'm fine with it not being temporary if others agree. I'd defer to folks with more expertise in block registration first.
Contributor
Author
|
The things to figure out:
|
tyxla
reviewed
Nov 20, 2025
Member
I prefer this to be permanent. If it failed once, it may fail again, so we might as well keep it. |
Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
Contributor
Author
If this is permanent, should we even bother adding a warning? |
Member
I think so, for the same reason we have the "Block names must be strings" and "Block names must contain a namespace prefix" warnings - essentially instances of |
Contributor
|
Thank you for the fix! 🙇 I just double-checked it from Woo side and all good, thanks again! |
mikachan
pushed a commit
that referenced
this pull request
Nov 21, 2025
…l and add warning for block authors (#73149) Co-authored-by: jeryj <jeryj@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: kmanijak <karolmanijak@git.wordpress.org>
Member
|
I just cherry-picked this PR to the release/22.1 branch to get it included in the next release: c91e119 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Closes #73135 (comment)
Add a warning if attributes is set incorrectly (undefined or null) when registering a block, and fixes it by setting the attributes as an empty object.
History
#71982 added color support checks to more blocks that did not typically hit the color styles editing path. As a result, it exposed a root architectural issue that allowed some blocks to not have
attributesdefined if the block was registered specifically with attributes asundefinedornullWhy?
Block structure should be consistent, and having attributes: undefined can cause odd issues. Give a warning when this happens and fix it. This should not be necessary, as block authors should not be passing attributes: undefined, but in case they are, this gives them time to fix it instead of crashing the editor.
How?
In registerBlockType, if block attributes is passed as undefined or null, output a warning to the block author so they know what went wrong and fix it by setting attributes as an empty object.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Screen.Recording.2025-11-13.at.10.13.48.AM.mov