Skip to content
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

Fix unset array key warning in block-bindings.php #66337

Merged
merged 2 commits into from
Nov 1, 2024

Conversation

benharri
Copy link
Contributor

This has been throwing an error for several weeks on one of my sites

Simplest fix is to just bail early if it's unset

PHP Warning: Undefined array key "show_in_rest" in /var/www/wp-content/plugins/gutenberg/lib/compat/wordpress-6.7/block-bindings.php on line 70

What?

Fix undefined array key warning

Why?

My error logs have been filling up

How?

Bail early if it's unset.

Testing Instructions

I'm not 100% sure which custom post type this is throwing from, but I'm leaning towards buddypress.

Testing Instructions for Keyboard

No more warning thrown from block-bindings.php

Screenshots or screencast

n/a

This has been throwing an error for several weeks on one of my sites

Simplest fix is to just bail early if it's unset

PHP Warning:  Undefined array key "show_in_rest" in /var/www/wp-content/plugins/gutenberg/lib/compat/wordpress-6.7/block-bindings.php on line 70
Copy link

github-actions bot commented Oct 22, 2024

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: benharri <[email protected]>
Co-authored-by: SantosGuillamot <[email protected]>
Co-authored-by: Mamaduka <[email protected]>

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

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Oct 22, 2024
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @benharri! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Feature] Block bindings labels Oct 22, 2024
@Mamaduka
Copy link
Member

Thanks for contributing, @benharri!

I would recommend an early bailout like we did in #60862.

Copy link
Contributor

@SantosGuillamot SantosGuillamot left a comment

Choose a reason for hiding this comment

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

I haven't been able to reproduce, even registering a meta field without show_in_rest, but I agree that an early bailout seems reasonable.

I guess that if we don't include an early bailout it could potentially fail in the next conditionals.

@benharri
Copy link
Contributor Author

Here's a stack trace from my case. It does seem to be buddypresss.

From Query Monitor:

Undefined array key "show_in_rest"
    wp-content/plugins/gutenberg/lib/compat/wordpress-6.7/block-bindings.php:70
    gutenberg_update_meta_args_with_label()
    wp-includes/class-wp-hook.php:326
    apply_filters('register_meta_args')
    wp-includes/meta.php:1453
    register_meta()
    wp-includes/taxonomy.php:1534
    register_term_meta()
    wp-content/plugins/buddypress/bp-core/bp-core-functions.php:3552
    bp_register_type_meta()
    wp-content/plugins/buddypress/bp-members/bp-members-functions.php:2841
    bp_register_member_type_metadata()
    wp-includes/class-wp-hook.php:324
    do_action('bp_register_type_metadata')
    wp-content/plugins/buddypress/bp-core/bp-core-dependency.php:110
    bp_register_type_metadata()
    wp-includes/class-wp-hook.php:324
    do_action('bp_register_taxonomies')
    wp-content/plugins/buddypress/bp-core/bp-core-dependency.php:95
    bp_register_taxonomies()
    wp-includes/class-wp-hook.php:324
    do_action('bp_init')
    wp-content/plugins/buddypress/bp-core/bp-core-dependency.php:291
    bp_init()
    wp-includes/class-wp-hook.php:324
    do_action('init')
    wp-settings.php:700

Applying this fixes this warning

@SantosGuillamot
Copy link
Contributor

To me, it seems safer to add this check. I would just move it to an early bailout, as suggested by @Mamaduka here, to follow existing patterns. I assume that should solve your issue as well.

@Mamaduka Mamaduka added the No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core label Oct 29, 2024
@benharri
Copy link
Contributor Author

This is an early bailout with &&. I can change the formatting if you want.

@SantosGuillamot
Copy link
Contributor

This is an early bailout with &&. I can change the formatting if you want.

But it still modifies the $args when it is not needed, right?

I'd personally replicate what was done here. We already have the check for the label, so I'd add the one checking if is set. I haven't tested it, but I assume everything should work as expected.

@benharri
Copy link
Contributor Author

empty() doesn't modify anything, it just checks if it's empty or not but yeah i can switch it to use isset() if that is preferred

@SantosGuillamot
Copy link
Contributor

empty() doesn't modify anything, it just checks if it's empty or not but yeah i can switch it to use isset() if that is preferred

I wasn't referring to empty() vs isset(), I meant moving it to its own early conditional to ensure we don't modify the $args property when it is not needed. Something like this:

// Don't update schema when a setting isn't exposed via REST API.
if ( ! isset( $args['show_in_rest'] ) ) {
	return $args;
}

@benharri
Copy link
Contributor Author

all set

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thank you, @benharri!

@gziolo gziolo merged commit d2157ff into WordPress:trunk Nov 1, 2024
62 checks passed
@github-actions github-actions bot added this to the Gutenberg 19.7 milestone Nov 1, 2024
@benharri benharri deleted the patch-1 branch November 11, 2024 06:47
karthick-murugan pushed a commit to karthick-murugan/gutenberg that referenced this pull request Nov 13, 2024
* Fix unset array key warning in block-bindings.php

This has been throwing an error for several weeks on one of my sites

Simplest fix is to just bail early if it's unset

PHP Warning:  Undefined array key "show_in_rest" in /var/www/wp-content/plugins/gutenberg/lib/compat/wordpress-6.7/block-bindings.php on line 70

* Update block-bindings.php

Co-authored-by: benharri <[email protected]>
Co-authored-by: SantosGuillamot <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block bindings First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants