[wp-trac] [WordPress Trac] #58087: Fix the 'data' dynamic property in WP_Term

WordPress Trac noreply at wordpress.org
Wed Apr 5 14:51:53 UTC 2023


#58087: Fix the 'data' dynamic property in WP_Term
-------------------------------------------------+-------------------------
 Reporter:  antonvlasenko                        |       Owner:
                                                 |  hellofromTonya
     Type:  enhancement                          |      Status:  assigned
 Priority:  normal                               |   Milestone:  6.3
Component:  Taxonomy                             |     Version:  trunk
 Severity:  minor                                |  Resolution:
 Keywords:  dev-feedback php82 needs-unit-tests  |     Focuses:  coding-
                                                 |  standards
-------------------------------------------------+-------------------------

Comment (by hellofromTonya):

 Some contextual information to help with determining next steps for this
 ticket:


 * `WP_Term` was introduced in 4.4.0 via [34997] / #14162.

 * The declared `$data` property and the `__get` magic method were also
 introduced in 4.4.0 via [35032] / #34262. Why? As a technique to:
 >get raw data from a `WP_Term` object. Mirroring `WP_User`, we introduce a
 data property on term objects, which `get_the_terms()` uses to fetch
 cacheable term info.

 * The declared `data` property was later removed in 4.4.0 via [35269] /
 #34348 to fix a data type incompatibility with `WP_Ajax_Response`:
 >`wp_ajax_add_term()` fetches a term using `get_term()`, and passes the
 term to
 `WP_Ajax_Response`, which expects each of the term's properties to be
 scalar.
 Having `$data` as a `stdClass` (meant to mimic `WP_User::data`, populated
 by
 a `get_row()` database query) violated this expectation, causing fatal
 string
 conversion errors. As a workaround, `$term->data` is converted so that it
 is
 no longer an actual property of the term object, but is assembled only
 when
 requested in the magic `__get()` method.

 Notice the following:
 * Its design was introduced back in 4.4.0.
 * Removing the declared property and doing the conversion in the magic
 method were "a workaround".
 [https://core.trac.wordpress.org/ticket/34348#comment:2 A longer strategy
 was shared here] to make `WP_Ajax_Response` "do something more
 intelligent".

 Given the current design is an intentional workaround to deal with "non-
 scalar properties", a deeper dive is needed to better understand what's
 happened since 4.4.0 in the usage of the `data` property and the non-
 scalar issue. For example:

 * Does this non-scalar issue still exist?
 * Could the property be computed in the constructor (per its original
 design)?
 * How is the `data` property being used today in Core? How does this
 differ from 4.4.0's design intent?

 Removing the magic method and/or adding the `data` property onto the class
 should not happen until the deep dive is completed and side-effects and
 impacts understood.

 Automated tests should also be added to document the current behavior,
 especially in the areas of concern. Doing so could also provide a better
 understanding.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/58087#comment:4>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list