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

WordPress Trac noreply at wordpress.org
Thu Feb 22 16:29:33 UTC 2024


#58087: Fix the 'data' dynamic property in WP_Term
-------------------------------------+-------------------------------------
 Reporter:  antonvlasenko            |       Owner:  hellofromTonya
     Type:  defect (bug)             |      Status:  reviewing
 Priority:  normal                   |   Milestone:  6.5
Component:  Taxonomy                 |     Version:  6.3
 Severity:  minor                    |  Resolution:
 Keywords:  php82 needs-testing      |     Focuses:  coding-standards, php-
  needs-patch                        |  compatibility
-------------------------------------+-------------------------------------
Changes (by hellofromTonya):

 * keywords:  php82 has-patch has-unit-tests needs-testing changes-requested
     => php82 needs-testing needs-patch


Comment:

 Thinking more about moving the prop to the constructor ...

 How are the term's properties updated such as after an edit response (from
 the UI)?

 Handled within `wp_update_term()`. What's interesting is: the properties
 instance of `WP_Term` is not updated with the latest values, but instead
 its original values are collected in the `data` dynamic property and then
 used for merging with the new values.

 But the term properties are public and thus available for direct changes.
 Right now, the `data` dynamic property gets updated with those direct
 changes when `wp_update_term()` runs.

 What's the BC risk for moving `data` to the constructor?

 The risk only exists for a highly specific use case:

 * the `data` property is being directly used outside of Core.
 * and the other `WP_Term` properties are directly changed outside of Core.

 I'm thinking the overall risk to users is very minimal to none, given
 `wp_update_term()` will no longer use `data`.

 In comparison, what's the BC risk for removing the `data` property and
 getter?

 Again it's the same highly specific use case. But it cause an error when
 the use case exists due to the plugin or theme directly using the `data`
 property.

 Thus, I agree with @antonvlasenko and the original thinking from the live
 working session to move it to the constructor and get rid of the getter.

 Working on an updated patch. Updated the keywords to reflect the new
 direction.

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


More information about the wp-trac mailing list