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

PHP 8.2: remove deprecated callable in Style Engine value functions #44561

Merged
merged 2 commits into from
Sep 29, 2022

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Sep 29, 2022

Fixes: #44536

Props to @jrfnl for raising the issue and explaining the options.

What?

A sync of the 6.1 Core patch: WordPress/wordpress-develop#3363

Why?

The string syntax for callables will be deprecated in PHP version 9.

8.2 shows a deprecation notice.

See: https://wiki.php.net/rfc/deprecate_partially_supported_callables

See comment: https://core.trac.wordpress.org/ticket/56467#comment:242 for a description of the deprecation notice and explanation.

How?

This PR switches to use to array( self::class, '*' ) and makes the WP_Style_Engine class final with the understanding that we won't need to, nor should we, extend it.

Testing Instructions

Things should work as expected, PHP unit tests should pass.

As for the compat check, PHP Gutenberg doesn't run unit tests against 8.2 so for now, check the 8.2 unit tests logs as described over in WordPress/wordpress-develop#3363

Switch to array( self::class, '*' ) with the assumption that we won't need to extend WP_Style_Engine and therefore overwrite the get_individual_property_css_declarations method
@ramonjd ramonjd self-assigned this Sep 29, 2022
@ramonjd ramonjd added [Type] Bug An existing feature does not function as intended [Package] Style Engine /packages/style-engine labels Sep 29, 2022
@ramonjd ramonjd requested a review from jrfnl September 29, 2022 05:36
@ramonjd ramonjd added the [Priority] High Used to indicate top priority items that need quick attention label Sep 29, 2022
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this up quickly @ramonjd! This LGTM and I smoke tested to make sure that the individual border sides are still working correctly in server-rendered blocks (like Featured Image):

image

It'd be good to give @aristath a chance to weigh-in on the introduction of final, but this all looks good to me! ✨

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

@ramonjd Thanks for getting this set up, all good from me ✅

I can also confirm that the test errors on PHP 8.2, which were introduced by the previous iteration of this class, no longer show: https://github.com/WordPress/wordpress-develop/actions/runs/3148832341/jobs/5119781953#step:19:326

@ramonjd ramonjd merged commit ca19063 into trunk Sep 29, 2022
@ramonjd ramonjd deleted the update/style-engine-8-1-compat branch September 29, 2022 22:21
@github-actions github-actions bot added this to the Gutenberg 14.3 milestone Sep 29, 2022
@aristath
Copy link
Member

I'm late to the party, but I just checked this one to confirm everything looks OK. The code is simple, and this is a small change - I can confirm there are no issues with PHP 8.2 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Style Engine /packages/style-engine [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended
Projects
Status: 🏆 Done
Development

Successfully merging this pull request may close these issues.

PHP 8.2 | Gutenberg is introducing deprecated callables into WP Core
4 participants