Skip to content

BoxControl: Fix critical error when null value is passed#65287

Merged
t-hamano merged 3 commits into
trunkfrom
fix/block-gap-crash
Sep 12, 2024
Merged

BoxControl: Fix critical error when null value is passed#65287
t-hamano merged 3 commits into
trunkfrom
fix/block-gap-crash

Conversation

@t-hamano

Copy link
Copy Markdown
Contributor

Found this while researching this comment: #64971 (comment)

What?

This PR fixes a critical error that occurs when null is passed to the values ​​prop of the BoxControl component.

The only time this error can occur in Gutenberg project is if the following conditions are met:

  • The active theme is a block theme
  • The theme supports blockGap (settings.appearanceTools is true or settings/spacing.blockGap is true)
  • There is no spacing preset
  • Access blocks with axial gap support via the global styles (e.g. Columns block)
c85e87ab95a6e370ed70e561cc7b0ee3.mp4
Browser error log
Uncaught TypeError: Cannot convert undefined or null to object
    at Function.values (<anonymous>)
    at isValuesDefined (utils.ts:183:10)
    at BoxControl (index.tsx:93:41)
    at renderWithHooks (react-dom.js?ver=18:15496:20)
    at mountIndeterminateComponent (react-dom.js?ver=18:20113:15)
    at beginWork (react-dom.js?ver=18:21636:18)
    at HTMLUnknownElement.callCallback (react-dom.js?ver=18:4151:16)
    at Object.invokeGuardedCallbackDev (react-dom.js?ver=18:4200:18)
    at invokeGuardedCallback (react-dom.js?ver=18:4264:33)
    at beginWork$1 (react-dom.js?ver=18:27500:9)

Why?

If the theme does not have a spacing preset and the block supports the axial gap, the block spacing UI will be rendered as a BoxControl component here:

<BoxControl
label={ __( 'Block spacing' ) }
min={ 0 }
onChange={ setGapValues }
units={ units }
sides={ gapSides }
values={ gapValues }
allowReset={ false }
splitOnAxis={ isAxialGap }
/>

Moreover, the initial value of the block gap is null, which is passed as the value of the values ​​prop here.

As a result, a critical error occurs in the isValuesDefined() function when Object.values( null ).filter is executed:

export function isValuesDefined( values?: BoxControlValue ) {
return (
values !== undefined &&
Object.values( values ).filter(

How?

Perhaps the ideal solution would be to investigate why the initial value of the blockGap is null and convert it to undefined. However, the SpacingSizesControl component also has the isValuesDefined() function, which checks for null in addition to undefined:

export function isValuesDefined( values ) {
if ( values === undefined || values === null ) {
return false;
}
return Object.values( values ).filter( ( value ) => !! value ).length > 0;
}

Therefore, it seems better to add a null check to the BoxControl component itself as well.

Testing Instructions

  • Activate the Emptytheme and update theme.json as follows:
    {
    	"version": 3,
    	"settings": {
    		"appearanceTools": true,
    		"layout": {
    			"contentSize": "840px"
    		},
    		"spacing": {
    			"spacingSizes": [],
    			"defaultSpacingSizes": false
    		}
    	}
    }
  • Access the Site Editor > Global styles > Blocks > Columns
    • trunk: The critical error occurs.
    • This PR: The critical error does not occur and the block spacing UI is rendered.

@t-hamano t-hamano added [Type] Code Quality Issues or PRs that relate to code quality [Package] Components /packages/components labels Sep 12, 2024
@t-hamano t-hamano self-assigned this Sep 12, 2024
@t-hamano t-hamano requested review from a team and andrewserong September 12, 2024 14:53
@t-hamano t-hamano marked this pull request as ready for review September 12, 2024 14:54
@t-hamano t-hamano requested a review from ajitbohra as a code owner September 12, 2024 14:54
@github-actions

github-actions Bot commented Sep 12, 2024

Copy link
Copy Markdown

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@tyxla tyxla left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the fix @t-hamano 🙌

A bummer that TS couldn't catch this one.

Comment thread packages/components/src/box-control/utils.ts Outdated
Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
@t-hamano t-hamano merged commit 6d32f4f into trunk Sep 12, 2024
@t-hamano t-hamano deleted the fix/block-gap-crash branch September 12, 2024 15:57
@github-actions github-actions Bot added this to the Gutenberg 19.3 milestone Sep 12, 2024
@andrewserong

Copy link
Copy Markdown
Contributor

Good catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants