[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