Handle WP_Term dynamic properties for PHP 8.2

[hellofromTonya]

For PHP 8.2+ compatibility, the scope of this ticket is to handle WP_Term's dynamic properties and its incomplete set of magic methods.

As noted in epic ticket #56034:

Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0, though this last part is not 100% certain yet.

RFC: ​https://wiki.php.net/rfc/deprecate_dynamic_properties

The epic ticket provides recommendations for how to handle dynamic properties:

Mitigation Approaches

This ticket offers 2 different approaches. It uses the epic ticket #56034 as the guide for mitigation.

Approach 1: Declare Core's known, named and deprecate dynamic properties.

Per #56034:

Situation: Known, named, dynamic property
Resolution: Declare the property on the (parent) class

This approach proposes to:

• Soft remove support of dynamic properties by deprecating getting dynamic properties.
• Identify and declare all of Core's known, named dynamic properties on the WP_Term class.

Goals:

• Remove Core's dynamic properties.
• Alert extenders via deprecation notice and dev note.
• Extenders to remove their WP_Term dynamic properties and replace with a different custom handling approach. (Note: need to figure out what to recommend to extenders for this migration process.)

Tasks:

1. Identify all of the known, named dynamic properties Core adds and uses.

Beyond the 'data' dynamic property (previously discussed in #58087), Core adds and uses a lot of other known, named dynamic properties. For example:

• wp_tag_cloud() adds link and id dynamic properties.
• _make_cat_compat() adds cat_ID, category_count, category_description, cat_name, category_nicename, category_parent.

A first step is to identify all of the dynamic properties that Core adds and uses.

2. Determine affects / impacts of the preferred solution

#56034 provides guidance for known, named dynamic properties:

Declare the property on the (parent) class

Consideration will be needed for how declaring these properties on the class will affect the usage of the WP_Term::to_array() method.

If found to have an unwanted affect, the compatible fields approach (e.g. WP_User_Query) can be explored (see #58897).

Approach 2: Add full support for dynamic properties on WP_Term.

Update: Approach 2 was found to not be viable as it will not support native PHP (array) type casting.

Per #56034:

Situation: Known use of unknown dynamic properties
Solution: Declare the full set of magic methods on a class
...
If the class really should support dynamic properties, the magic methods should be implemented on the class itself (or its parent).

This approach proposes to:

• Fully support dynamic properties on WP_Term.
• Add a full set of magic methods on the WP_Term class.

What if extenders are also adding and using dynamic properties?
It's been possible since WP_Term was introduced in 2015 in WP 4.4.0 via [34997].

Has it been supported since introduction?
Yes, as Core itself needed it supported.

Since its introduction, this class has supported and actively added / used dynamic properties. Core itself active uses and adds dynamic properties to WP_Term objects. Following its example, extenders also add dynamic properties.

With WP_Term being marked as final, there's not a way for extenders to declare their dynamic property needs on a custom child class.

It's reasonable to determine that WP_Term should fully support dynamic properties. Doing so maintains backward compatibility (BC) with the (hopefully) the lowest risks of breaking things for users.

References:

• The changes are part of #56034, which is the larger Dynamic Properties proposal and initiative.
• See #58087 for an extensive discussion and contextual history of the WP_Term::$data dynamic property.

Removes the unset() of the filter property within the term tests.

Why?

Prior to the introduction of WP_Term, the term was added to the cache _when_ its filter property was empty. To test the cache, the tests unset this property to trigger wp_cache_add() in get_term(). r34997 changed that behavior to trigger wp_cache_add when the term was not found after wp_cache_get() (i.e. happened in WP_Term::get_instance().

Unsetting the filter property is and was not needed. Prior to WP_Term, the condition was an empty value. With WP_Term, the filter property is no longer part of the conditional logic for caching.

Follow-up to [34997], [30954], [34035].

Trac ticket: https://core.trac.wordpress.org/ticket/61890

[hellofromTonya]

In 58919:

Tests: Remove WP_Term::$filter property unset() within term tests.

Removes the unset() of the WP_Term::$filter property within the term tests.

Why?

Prior to the introduction of WP_Term, the term was added to the cache when its filter property was empty. To test the cache, the tests unset this property to trigger wp_cache_add() in get_term(). [34997] changed that behavior to trigger wp_cache_add() when the term was not found after wp_cache_get() (i.e. happened in WP_Term::get_instance()).

Unsetting the filter property is and was not needed. Prior to WP_Term, the condition was an empty value. With WP_Term, the filter property is no longer part of the conditional logic for caching.

Follow-up to [34997], [30954], [34035].

See #61890, #61530.

Committed via https://core.trac.wordpress.org/changeset/58919.

[hellofromTonya]

What's the current behavior?

See it in action ​https://3v4l.org/2BeVs#veol.

Current behavior with known and unknown dynamic properties:

• ✅ get, isset, set, unset of known dynamic property works.
• ✅ get, isset, set, unset of unknown dynamic property works.
• ✅ WP_Term::$data is initialized on get and is not included when using WP_Term::to_array() or (array) $term.
• ✅ WP_Term::to_array() returns the declared properties and all dynamic properties. For example, the link and unknown dynamic properties in the test code were both included in the array.
• ✅ Same results when converting object to array via WP_Term::to_array() and (array) $term.
• No deprecation notices are thrown.

[hellofromTonya]

Declaring the known, named dynamic properties

This approach is the one recommended in #56034:

Declare the property on the (parent) class

What will the behavior be if all of the known, named dynamic properties are declared on the class?

See in action ​https://3v4l.org/SpW2N#veol.

The only change is to declare the known, named dynamic properties. Note: For this strategy, would also want to throw a deprecation for unknown dynamic properties.

Results as compared to the current behavior:

• ✅ get, isset, set, unset of known dynamic property works.
• ✅ get, isset, set, unset of unknown dynamic property works.
• ✅ WP_Term::$data is initialized on get and is not included when using WP_Term::to_array() or (array) $term.
• ✅ WP_Term::to_array() returns all of the declared properties and all dynamic properties.
  • ❌ Difference from the current behavior: all of the known, named dynamic properties are also included in the resultant array, even if they were not used or initialized.
• ✅ Same results when converting object to array via WP_Term::to_array() and (array) $term.
• No deprecation notices are thrown.

A few observations:

1. The differences between current behavior to this approach when converting from object to array is not a BC break IMO. Why? It's additional information / elements, but should not break existing behavior. Furthermore, adding new properties to existing Core classes is not considered a BC break; else, it would limit the ability for Core's classes to be enhanced over the years.
2. It is confusing why unused properties are returned, when previously they were only there when needed (such as when using a tag cloud).
3. It's also confusing why these properties are in the class, which could be a maintenance concern (will confuse contributors).
4. That said, WP_Term::to_array() could be modified to exclude these unused/uninitialized properties to retain the current behavior and array shape.
5. Doing 4 though means type casting would result in a different array shape. But this doesn't concern me either, as WP_Term::to_array() is the preferred way to convert to an array. A dev note can provide guidance for extenders to make changes.

[antonvlasenko]

Core adds and uses a lot of other known, named dynamic properties.

Yes, this has been discovered in ​https://github.com/WordPress/wordpress-develop/pull/6426.

I believe the WP_Term::$object_id property should also be added to the list of known properties (i.e., declared on the WP_Term class), as it is added by the WP_Term_Query::get_terms() method.

Additionally, the WP_Term::$auto_add property is added by the WP_REST_Menus_Controller::get_term() method.

Declares all of the known, named dynamic properties Core adds / uses. Each of the declared properties documents the function that adds the property, i.e. to help contributors understand why each exists on the WP_Term class (especially those that are duplicates with a new name).

Also throws a deprecation in __get() to identify other Core added dynamic properties and to alert extenders of their added dynamic properties.

Trac ticket: https://core.trac.wordpress.org/ticket/61890

Removes the assertion checking if the 'errors' property is empty. This property exists on WP_Error but not on WP_Term. Checking that the term created is not a WP Error is sufficient in this test.

Trac ticket: https://core.trac.wordpress.org/ticket/61890

[hellofromTonya]

In 58920:

Tests: Remove 'errors' assertion when not WP_Error in Tests_Term_WpInsertTerm.

Removes the assertion for 'errors' being empty when the instance is WP_Term and not WP_Error. This property exists on WP_Error.

This assertion always passed because it was checking a dynamic property on WP_Term that does not exist and is not added within Core. Thus, this assertion is not needed and fails with dynamic property deprecations.

Follow-up to [51403], [34646], [29830].

See #61890, #61530.

Committed via https://core.trac.wordpress.org/changeset/58920.

[hellofromTonya]

Replying to antonvlasenko:

I believe the WP_Term::$object_id property should also be added to the list of known properties (i.e., declared on the WP_Term class), as it is added by the WP_Term_Query::get_terms() method.

Additionally, the WP_Term::$auto_add property is added by the WP_REST_Menus_Controller::get_term() method.

Thanks for finding those. I added them in the ​working patch.

[hellofromTonya]

Should WP_Term support dynamic properties?

I've been pondering this question, especially when thinking about how to guide extenders with a deprecation notice in __get(). In other classes, the deprecation said:

A property {$name} is not declared. Unsetting a dynamic property is
deprecated since version 6.4.0! Instead, declare the property on the class.

How can extenders declare their dynamic property on the class? They can't. Hmm, this has me thinking about the above question:

Should WP_Term support dynamic properties?

Start with what's known

WP_Term is a model of a term. Its properties are joined columns from the $wpdb->terms and $wpdb->term_taxonomy db tables.

CREATE TABLE $wpdb->terms (
 term_id bigint(20) unsigned NOT NULL auto_increment,
 name varchar(200) NOT NULL default '',
 slug varchar(200) NOT NULL default '',
 term_group bigint(10) NOT NULL default 0,

CREATE TABLE $wpdb->term_taxonomy (
 term_taxonomy_id bigint(20) unsigned NOT NULL auto_increment,
 term_id bigint(20) unsigned NOT NULL default 0,
 taxonomy varchar(32) NOT NULL default '',
 description longtext NOT NULL,
 parent bigint(20) unsigned NOT NULL default 0,
 count bigint(20) NOT NULL default 0,

In Core, it's used in different ways, some of which add additional (known, named dynamic) properties. Thus, the term object has a base set of database column declared properties and possibly additional properties used within the scope and context of the functionality processing the term.

When updating a term (handled in wp_update_term()), the class handles limiting the properties to only base set of declared properties that correlate to the database columns.

WP_Term:

• Has a base set of declared properties that correlate to the columns in $wpdb->terms and $wpdb->term_taxonomy database tables.
• It's a final class.
• It allows for dynamic properties.
• Core adds known, named dynamic properties for use within its functionality.

Using what's known

What if extenders are also adding and using dynamic properties?
It's been possible since WP_Term was introduced in 2015 in WP 4.4.0 via [34997].

Has it been supported since introduction?
Yes, as Core itself needed it supported.

I'm thinking the answer to the first question of this comment is: Yes, WP_Term should support dynamic properties for Core and extenders.

Okay, how does it change the solution?

Path Forward

Going back to the epic ticket #56034:

Known use of unknown dynamic properties: Declare the full set of magic methods on a class (preferred) or let the class extend stdClass (discouraged)

and in the Proposal section:

If the class really should support dynamic properties, the magic methods should be implemented on the class itself (or its parent).

I'm currently thinking: continue to allow WP_Term to support dynamic properties by adding a full set of magic methods.

[hellofromTonya]

There are 2 different solutions available for testing:

• Declare the known, named dynamic properties and deprecate the getter magic method: ​patch PR 7224.
• [Preferred] Support dynamic properties - adds full set of magic methods: ​patch PR 7231.

As explained in comment:12, IMO the latter option to support dynamic properties is the needed and preferred the approach.

What's the downside?
Type casting to an array will no longer return the dynamic properties. Extenders will need to modify to use WP_Term::to_array(), which has been available and is recommended. A dev note will be needed to alert extenders. Plus, outreach to ​plugin authors will help to mitigate issues when it ships.

[hellofromTonya]

Testing Instructions for Default Site

Step up

1. WordPress version: use WordPress 6.6.1 for the before and use trunk with the patch to test the after.
2. PHP Version: use at least PHP 8.2.
3. Use a default site, meaning no plugins or theme activated unless listed in the set up or testing instructions.
4. For testing a classic / non-block site:
   • Install and activate the Classic Editor plugin.
   • Install and activate the Twenty Twenty theme.
   • No must-use scripts.
5. For testing a default (i.e. block) site:
   • No plugins should be activated.
   • No must-use scripts.
   • Activate Twenty Twenty-Four (TT4) theme.

Steps to Test

Do these test steps for both a default block site and classic / non-block site:

1. Add multiple tags.
2. Test the Bulk Delete: add 2 new tags and then bulk delete them.
3. Add multiple categories including at least one with a parent.
4. Test the Bulk Delete: add 2 new categories and then bulk delete them.
5. Test assigning tags and categories:
   • Add at least one new post and assign the tags and categories to it.
   • Go to Posts > Categories and verify each category's Count matches the number of posts using that category.
   • Go the same with the Tags.
   • View the post in the frontend.
     • Verify the categories and tags are listed.
     • Click on one of the categories. Observe it's linkable and properly goes to the term's archive page.
     • Repeat with a tag.
6. Feed: Go to the site's home page. Append /feed/ to its URL. Check that the feed has each of the terms (listed as category) for the post(s) you added.
7. Add a tag cloud block or widget.
8. Add a category list block or widget.

Expected Results

Terms should function and render exactly the same before and after applying the patch. There should be no errors in the logs associated with terms.

• ✅ Should be able to add new categories and tags.
• ✅ Should be able to edit categories and tags.
• ✅ Should be able to bulk delete categories and tags.
• ✅ Should be able to assign categories and tags in a post.
• ✅ Each term count should match to the number of posts using it.
• ✅ Term archives should be linkable and render posts using it.
• ✅ Categories and tags should be shown and linkable in each post in the front-end.
• ✅ Category widget and block should show all categories.
• ✅ Tag cloud widget and block should include all assigned tags.
• ✅ Server logs and/or the debug.log should be clear or at a minimum not include any errors associated with terms.

[hellofromTonya]

Test Result for Default Site

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/7231

Environment

• OS: macOS
• Web Server:
  • For after: Nginx (with wp docker)
  • For before: Nginx via Local
• PHP: 8.3.8
• WordPress: 6.6.1 (for before) and trunk (for after)
• Browser: Firefox 129.0.1
• Theme: See testing instructions
• Active Plugins: See testing instructions

Actual Results

Testing on classic / non-block site:

• ✅ No differences between WP 6.6.1 without the patch (before) and trunk with the patch (after). No errors. Each of the expectations matched.

Testing on block site:

• ✅ No differences between WP 6.6.1 without the patch (before) and trunk with the patch (after). No errors. Each of the expectations matched.

[hellofromTonya]

Test Result for WooCommerce

WooCommerce type casts to an array, instead of using WP_Term::to_array(). ​Example.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/7231

Environment

• OS: macOS
• Web Server:
  • For after: Nginx (with wp docker)
  • For before: Nginx via Local
• PHP: 8.3.8
• WordPress: 6.6.1 (for before) and trunk (for after)
• Browser: Firefox 129.0.1
• Theme: See testing instructions
• Active Plugins:
  • WooCommerce

Testing Instructions

1. Add a new product.
   • Configure its Inventory, Shipping, etc.
   • Add Attributes such as Brand, Material, Color.
   • Add a Product category.
   • Add a Product tag.
2. View the product in the frontend.
   • Navigate through its tabs.
   • Click on its product category.
   • Click on its tag(s).
3. View the Shop page. Navigate to the product.
4. View the server logs or debug.log, noting any errors (especially for terms).

Actual Results

• ✅ No differences between WP 6.6.1 without the patch (before) and trunk with the patch (after).
• ✅ No errors in server logs or debug.log.
• ✅ Each of the expectations worked as expected.

[hellofromTonya]

Proposed Patch: ​https://github.com/WordPress/wordpress-develop/pull/7231

Hello @lopo (Yoast), @louiswol94 @illuminea (Elementor), @adrianduffell @nerrad @beaulebens (WooCommerce), @ironprogrammer (to help test and/or identify a contact at Jetpack), @jjj (BuddyPress and bbPress), @takayukister (Contact Form), @borkweb @bordoni (The Events Calendar).

Appreciate you testing the proposed patch with your products and providing feedback. This patch is currently earmarked to land in 6.7.0.

The What
The proposed patch adds full support for dynamic properties in WP_Term by providing a full set of magic methods. Note: isset() will now work for WP_Term dynamic properties.

It also recommends using WP_Term::to_array() instead of directly casting from an object to an array (i.e. instead of doing (array) $term).

The Why
For PHP 8.2+ compatibility and to resolve the anticipated future fatal error of PHP 9+ when dynamic properties are hard removed.

I've tried to test this PR since it's the preferred solution out of the two, but the unit tests are failing. Could you please take a look, @hellofromtonya?

I've tried to test this PR since it's the preferred solution out of the two, but the unit tests are failing. Could you please take a look, @hellofromtonya?

The tests that are failing are for the __get() magic method. I'm still working on that dataset and its test setup. For now, I force pushed a change to skip over that test.

[antonvlasenko]

Test Report

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/7231

Environment

• WordPress: 6.7-alpha-58576-src / WordPress 6.6.1
• PHP: 8.3.3
• Server: Apache/2.4.57 (Unix) PHP/8.3.3
• Database: mysqli (Server: 5.7.43 / Client: mysqlnd 8.3.3)
• Browser: Safari 17.6 (macOS)
• Theme: Twenty Twenty-One / Twenty Twenty-Four
• Plugins: None

Actual Results

• ✅ The patch works as expected when tested on trunk with the patch applied using a block theme (Twenty Twenty-Four);
• ✅ The patch works as expected when tested on trunk with the patch applied using a non-block theme (Twenty Twenty-One);

Additional Notes

• ✅ No errors were observed when testing on WordPress 6.6.1 using a block theme (Twenty Twenty-Four);
• ✅ No errors were observed when testing on WordPress 6.6.1 using a non-block theme (Twenty Twenty-One);

"The patch working as expected" means that:
1) there were no UI errors/fatal errors;
2) there were no errors in debug.log.

[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

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.

[hellofromTonya]

This ticket has 2 proposals:

• ​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.
• ​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 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 ​Approach 2?

[hellofromTonya]

Updated the summary and description to match the 2 approaches proposed in this ticket to mitigate the issue.

• ​Approach 1: WP_Term: Declare known, named dynamic properties and deprecate __get()
• ​Approach 2: WP_Term: Add dynamic property support

[hellofromTonya]

Test Result for Customer Reviews for WooCommerce with Approach 2

Plugin:​Customer Reviews for WooCommerce version 5.59.0.

Test: Does this plugin work with its (array) $tag? Or does it require changing to use $tag->to_array()?

This plugin is ​type casting a tag object to an `array` instead of using to_array() method:

'supplemental' => (array) $tag,

Patch tested: Approach 2 ​https://github.com/WordPress/wordpress-develop/pull/7231

Environment

• OS: macOS
• Web Server:
  • For after: Nginx (with wp docker)
  • For before: Nginx via Local
• PHP: 8.2.22
• WordPress: 6.6.1 (for before) and trunk (for after)
• Browser: Firefox 129.0.1
• Theme: See testing instructions
• Active Plugins:
  • Customer Reviews for WooCommerce version 5.59.0
  • WooCommerce version 9.2.3

Testing Instructions

1. Navigate to Reviews > Review Tagging
2. Add a new tag in the "Create New Tag" text field.
3. Press/click/trigger the "Create New Tag" button.

Expected Results

The tag should be created, page refreshes automatically, and the tag is shown in the list table.

Actual Results

See it in action ​https://a.supportally.com/v/K7ERkQ.

• ✅ On WP 6.6.1: the tag should be created, page refreshes automatically, and the tag is shown in the list table.

• On trunk with this patch:
  • ❌ the spinner spins and the page does not refresh.
  • ✅ When manually refreshing the tag, the tag was created and it shows in the tag list table.

After modifying the code in the plugin to use $tag->to_array():

• ✅ On trunk with this patch: the tag should be created, page refreshes automatically, and the tag is shown in the list table.
• ✅ On WP 6.6.1: the tag should be created, page refreshes automatically, and the tag is shown in the list table.

Conclusion

This plugin will need to change from (array) $tag to $tag->to_array(). The change will work on older and new WordPress versions.

[adrianduffell]

Replying to @hellofromTonya

Thanks for the info on what to audit.

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)?

In many cases I think term meta can be used instead:

$term->my_custom_property = 'the_value';

becomes:

add_term_meta( $term->term_id, 'my_custom_property', 'the_value' );

If storing the data in term meta is undesirable, and there is a genuine need for extra properties, OOP-wise I think this is a strong signal a new class should be created to properly represent the data. I see an example of this in WP's in wp_tag_cloud function. It currently adds dynamic properties to a WP_Term object:

$tags[ $key ]->link = $link;
$tags[ $key ]->id   = $tag->term_id;

At this point, the object resembles a WP_Term, but has morphed into something slightly different. A problem arises in the wp_generate_tag_cloud function, which states in its documentation that an array of WP_Term objects should be passed to it, but actually expects the dynamic properties to be present in the object as well. The problem is developers won't know from the documentation to add the dynamic properties when using wp_generate_tag_cloud. I think it would be better OOP-wise to handle the data in a new class, e.g. WP_Tag, that explicitly includes the extra properties in it's definition along with full documentation about them. This way, wp_generate_tag_cloud could clearly document its requirement for the additional properties just by stating WP_Tag objects are passed to it.

And how might adding Core's known, named (previously dynamic) properties affect plugins?

I don't think it changes anything from the status quo. A plugin might be adding a dynamic property with an identical name and creating a clash with Core, but this is possible with Core using dynamic properties too. Ideally I think Core would benefit from migrating to the approaches above too.

what downsides do you anticipate with Approach 2?

It looks like there is a backwards compatibility break in a scenario where dynamic properties are added to WP_Termobjects and those are being cast with (array). However this seems really difficult for plugin devs to audit their codebase for instances of it. For example, it doesn't seem possible to inspect and create a log every time (array) is performed on WP_Term objects. So while the backwards compatibility break might be necessary, it looks difficult to find instances where it could occur.

[hellofromTonya]

Replying to @adrianduffell comment:25

It looks like there is a backwards compatibility break in a scenario where dynamic properties are added to WP_Term objects and those are being cast with (array)

IMO Approach 2 is not a backward compatibility (BC) break.

Why? The WP_Term::to_array() method has been available since WP_Term introduction back in WP 4.4.0. While ​type casting returned the same result, this method is there to be used as the means/mechanism to convert the term object to an array.

[adrianduffell]

Replying to hellofromTonya:

IMO Approach 2 is not a backward compatibility (BC) break.

Why? The WP_Term::to_array() method has been available since WP_Term introduction back in WP 4.4.0. While ​type casting returned the same result, this method is there to be used as the means/mechanism to convert the term object to an array.

Interesting, as an extender I don't find this intuitive. I would see that all PHP language constructs such as (array) type casting would be supported by WordPress, unless forbidden in the coding standards. Has there been previous guidance about it not being supported?

[takayukister]

Hello @hellofromTonya

Sorry to be a late reply. I think this change won't cause any problems with Contact Form 7. In case if some issues found, I promise we'll fix them by the 6.7.0 release.

FYI, for Contact Form 7, we have already prepared for the dynamic properties retirement. See this GitHub issue for details => ​https://github.com/rocklobster-in/contact-form-7/issues/946

@adrianduffell raised a valid point:

I would see that all PHP language constructs such as (array) type casting would be supported by WordPress, unless forbidden in the coding standards.

This PR (Approach 2) does not meet that expectation.

In order to continue support (array) $terms type casting while ensuring it matches the same results as the WP_Term::to_array(), this PR is not a viable solution.

[hellofromTonya]

Replying to adrianduffell:

I would see that all PHP language constructs such as (array) type casting would be supported by WordPress, unless forbidden in the coding standards. Has there been previous guidance about it not being supported?

Sorry for my late response. You raised a valid and good point.

In order to continue supporting (array) type casting while also ensuring it matches the same results with to_array(), this means ​Approach 2 is not a viable solution.

In reconsidering, I think @adrianduffell is right (thank you for raising it). ​I closed Approach 2's PR.

Doing so then means WP_Term will not support dynamic properties.

I'll update this ticket's description for clarity with what is known today.

[hellofromTonya]

Description updated to denote Approach 2 was found to not be a viable solution.

[hellofromTonya]

With Approach 2 no longer being viable (i.e. to fully support dynamic properties), what is / are the possible ways forward to resolve it?

From this ticket's description

Approach 1: Declare Core's known, named and deprecate dynamic properties.

Patch: ​https://github.com/WordPress/wordpress-develop/pull/7224

Per #56034:

Situation: Known, named, dynamic property
Resolution: Declare the property on the (parent) class

This approach proposes to:

• Soft remove support of dynamic properties by deprecating getting dynamic properties.
• Identify and declare all of Core's known, named dynamic properties on the WP_Term class.

Goals:

• Remove Core's dynamic properties.
• Alert extenders via deprecation notice and dev note.
• Extenders to remove their WP_Term dynamic properties and replace with a different custom handling approach. (Note: need to figure out what to recommend to extenders for this migration process.)

What to recommend to extenders for mitigating the deprecation notices?
@adrianduffell shared an idea of recommending extenders use term meta.

Approach 3: Introduce new classes for each instance where Core is adding dynamic properties to extend WP_Term

@adrianduffell raised this idea:

At this point, the object resembles a WP_Term, but has morphed into something slightly different. A problem arises in the wp_generate_tag_cloud() function, which states in its documentation that an array of WP_Term objects should be passed to it, but actually expects the dynamic properties to be present in the object as well. The problem is developers won't know from the documentation to add the dynamic properties when using wp_generate_tag_cloud(). I think it would be better OOP-wise to handle the data in a new class, e.g. WP_Tag, that explicitly includes the extra properties in it's definition along with full documentation about them. This way, wp_generate_tag_cloud() could clearly document its requirement for the additional properties just by stating WP_Tag objects are passed to it.

I think the idea is interesting and could/should be extended to each instance where Core is adding dynamic properties to extend WP_Term.

Are there instances where extenders are using or expecting these Core dynamic properties? If yes, then BC is a concern.

[hellofromTonya]

IMO this kind of change will benefit from a long soak time. Preparing the change for an early (earlier the better) Alpha commit can help maximize the opportunities for feedback, testing, and any needed follow-ups.

Plus, this solution is still being discussed and needs more time.

As 6.7 Beta 1 is a few days, early Alpha has passed. Moving this ticket to the next major. That said, let's take the time between now and early 6.8 Alpha to solidify a solution so that it's ready to be committed once alpha is open.