10up / 10up-experience

The 10up Experience plugin configures WordPress to better protect and inform clients, aligned to 10up’s best practices.
GNU General Public License v2.0
129 stars 27 forks source link

Convert the Singleton class to a trait for PHP 8.1. compatibility #119

Closed johnwatkins0 closed 1 year ago

johnwatkins0 commented 1 year ago

Description of the Change

As of PHP 8.1.0, when a method using static variables is inherited (but not overridden), the inherited method will now share static variables with the parent method. This means that static variables in methods now behave the same way as static properties. [Source]

Because of this change, the static $instance in the Singleton abstract class's instance method was going to be shared between all classes that inherit from Singleton. As a result, the instance would be set to the first class whose instance method is called, and none of the other classes would be created.

This converts the class to a trait, which essentially gives each Singleton class its own instances of the instance static property and method.

Also included: If you review the instance method previously in the abstract class and now in the trait, it runs its class's setup method if that method exists. That method was also being run in the plugin entry file as classes were instantiated, so it was being run twice.

Closes #117

How to test the Change

Check out this branch. Confirm all functionality works as expected.

Changelog Entry

Added - New feature Changed - Existing functionality Deprecated - Soon-to-be removed feature Removed - Feature Fixed - Bug fix Security - Vulnerability

Changed - Converted the Singleton abstract class into a trait for PHP 8.1. compatability.

Credits

Props @username, @username2, ...

Checklist:

tlovett1 commented 1 year ago

This all looks good to me @claytoncollie @johnwatkins0 . My only question is what is the minimum version of PHP this will support?

johnwatkins0 commented 1 year ago

@tlovett1 Traits were introduced in PHP 5.4, so this might be problematic since this plugin's README says it supports 5.3. I'll revisit and see what can be done while still working with 5.3.

tlovett1 commented 1 year ago

@johnwatkins0 5.4 is fine. I'll update the readme. Thank you!