[wp-trac] [WordPress Trac] #61890: Handle WP_Term known, named dynamic properties for PHP 8.2
WordPress Trac
noreply at wordpress.org
Tue Aug 27 14:34:27 UTC 2024
#61890: Handle WP_Term known, named dynamic properties for PHP 8.2
-------------------------------------+-------------------------------------
Reporter: hellofromTonya | Owner: (none)
Type: defect (bug) | Status: new
Priority: normal | Milestone: 6.7
Component: Taxonomy | Version: 4.4
Severity: minor | Resolution:
Keywords: php82 has-patch has- | Focuses: coding-standards, php-
unit-tests needs-testing needs- | compatibility
dev-note has-testing-info |
-------------------------------------+-------------------------------------
Comment (by hellofromTonya):
This ticket has 2 proposals:
* [https://github.com/WordPress/wordpress-develop/pull/7224 Approach 1]:
remove support for dynamic properties by (a) declaring all of Core's
known, named dynamic properties and (b) deprecating getting dynamic
properties. The goal: Extenders remove their dynamic properties and they
develop a different approach for handling the extra data.
* [https://github.com/WordPress/wordpress-develop/pull/7231 Approach 2]:
add full support for dynamic properties (with a full set of magic methods)
in `WP_Term`. This is done for BC and gives extenders the means to
continue declaring their dynamic properties on the `WP_Term` class.
Replying to [comment:21 adrianduffell]:
> Thanks for working on this and reaching out to us in WooCommerce!
>
> It looks like a tricky job for WooCommerce to fully audit potential
issues. If I understand correctly, we would need to identify:
>
> * A) dynamic properties added to a WP_Term object, in combination with
>
> * B) that object being cast to an array
Yes and no.
A) Yes or no. Depends upon which approach is adopted for `WP_Term`.
* Approach 1: Yes, you will need to (a) identify each of the dynamic
properties being added by WooCommerce plugins/products and (b) develop a
different approach to handle them.
* Approach 2: No, you don't need to identify the dynamic properties being
added within WooCommerce plugins/products as they are handled within
`WP_Term` without deprecation notices (for PHP 8.2+) or fatal errors (for
PHP 9+).
B) Yes or maybe.
* Approach 1: `(array) $term` and `$term->to_array()` will give all of the
new declared (previously known, named dynamic) properties plus any unknown
dynamic properties added outside of Core.
* Approach 2: more flexible. When you need an array that includes the
dynamic properties, use `$term->to_array()`; else, use `(array) $term` (no
dynamic properties). (This will have a dev note BTW.)
> That said, dynamic properties have been a source of clashes within our
ecosystem. I would prefer to see their usage removed / discouraged.
Generally, we have seen issues in the wild where two different WooCommerce
extensions independently add the same named dynamic property to an object
but expect different types - e.g. one may expect a string and the other a
boolean. Clashes ensue when the object is passed between functions.
Overall we have decided to swim in the direction PHP is taking and
discourage their use (see issue
https://github.com/woocommerce/woocommerce/issues/45286).
>
> > How can extenders declare their dynamic property on the class? They
can't.
>
> I could see this being an opportune time to deprecate such use in
WP_Term and encourage safer coding patterns. As WordPress doesn’t seem to
have been actively encouraging extenders to add dynamic properties to the
class, I think it would be reasonable to drop support for it in-line with
PHP’s motives.
>
> What are your thoughts on adding the full set of magic methods to
WP_Term as a temporary workaround but including a _doing_it_wrong notice
when a dynamic property is used? For WooCommerce at least, this would help
us identify any usage in the ecosystem and promote safer patterns being
used.
Approach 1 achieves this suggestion but without adding the full set of
magic methods (it's only on the existing `__get()`). I'm concerned about
this approach, which I shared in comment:12.
If the goal is to guide / nudge extenders to remove their dynamic
properties, what guidance can be given to them to change their approach?
How can they handle their extra data needs (which are currently dynamic
properties)?
And how might adding Core's known, named (previously dynamic) properties
affect plugins?
@adrianduffell what downsides do you anticipate with
[https://github.com/WordPress/wordpress-develop/pull/7231 Approach 2]?
--
Ticket URL: <https://core.trac.wordpress.org/ticket/61890#comment:22>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list