performance icon indicating copy to clipboard operation
performance copied to clipboard

Remove constant from the deprecated.php in optimization-detective plugin

Open hbhalodia opened this issue 9 months ago • 8 comments

Bug Description

Part of - https://github.com/WordPress/performance/issues/1859 and in PR - https://github.com/WordPress/performance/pull/1865, We have removed the use of the constants from the file, instead we have added those constant in a class.

For backward compatibility, we have kept those constant in deprecated.php file, since these constants are being used in Image Priortizer plugin as well.

The constants are OD_REST_API_NAMESPACE and OD_URL_METRICS_ROUTE, ~in newer versions after 2 or 3 releases,~ (see https://github.com/WordPress/performance/pull/1943#discussion_r2008201104) we need to update the plugin to use the class constants instead of deprecated once and remove it from the file. See

We need to pick this once the PR - https://github.com/WordPress/performance/pull/1865 is merged and Optimization detective plugin is released with Image Priortizer as well.

Steps to reproduce

  • None

Screenshots

  • None

Cc: @westonruter @felixarntz

hbhalodia avatar Mar 12 '25 09:03 hbhalodia

As part of this issue, let's also remove:

https://github.com/WordPress/performance/blob/da320f3705a95a379993451d30818c379486aaad/plugins/optimization-detective/class-od-tag-visitor-context.php#L123-L136

westonruter avatar Mar 14 '25 21:03 westonruter

Also these class aliases:

https://github.com/WordPress/performance/blob/da320f3705a95a379993451d30818c379486aaad/plugins/optimization-detective/load.php#L110-L111

westonruter avatar Mar 14 '25 21:03 westonruter

Hi @westonruter, Should we work on this issue? as we have merged, the PR related to the REST API class for URL metrics? Or should we wait for the releases for the plugins in Optimization Detective and Image Priortizer?

hbhalodia avatar Mar 17 '25 09:03 hbhalodia

@hbhalodia it can be worked on now. I've created the release branch for the release today, so if this gets merged it won't go out into the next release.

We probably will need to update the od_init action handler in Image Prioritizer to require Optimization Detective 1.0.0-beta3 or higher. If not, we should show the upgrade admin notice.

westonruter avatar Mar 17 '25 13:03 westonruter

In other words, we can just bump the $required_od_version:

https://github.com/WordPress/performance/blob/35b9ae15d5850e8f8b6b821be3cddb53791077a3/plugins/image-prioritizer/helper.php#L23-L40

We can also get rid of strtok() here since version_compare() handles pre-release version suffixes:

The function first replaces _, - and + with a dot . in the version strings and also inserts dots . before and after any non number so that for example '4.3.2RC1' becomes '4.3.2.RC.1'. Then it compares the parts starting from left to right. If a part contains special version strings these are handled in the following order: any string not found in this list < dev < alpha = a < beta = b < RC = rc < # < pl = p. This way not only versions with different levels like '4.1' and '4.1.2' can be compared but also any PHP specific version containing development state.

westonruter avatar Mar 17 '25 19:03 westonruter

Hi @westonruter, I have raised the PR with the required changes - https://github.com/WordPress/performance/pull/1943, Can you please review the same.

Thank You,

hbhalodia avatar Mar 18 '25 08:03 hbhalodia

Hi @westonruter, I spent some time resolving the failed unit test, but not able to identify the root caues from the PR. I am guessing it would be related to the part we removed from alias and switch case removal.

Also just to confirm, we need to remove the entire case in switch or just the below part and keep the case as it is.

// TODO: Remove this when no plugins are possibly referring to the url_metrics_group_collection property anymore. 
 	_doing_it_wrong( 
 		esc_html( __CLASS__ . '::$' . $name ), 
 		esc_html( 
 			sprintf( 
 				/* translators: %s is class member variable name */ 
 				__( 'Use %s instead.', 'optimization-detective' ), 
 				__CLASS__ . '::$url_metric_group_collection' 
 			) 
 		), 
 		'optimization-detective 0.7.0' 
 	); 

Thank You,

hbhalodia avatar Mar 18 '25 08:03 hbhalodia

We'll remove the entire case, yes. There should no longer be a url_metrics_group_collection.

westonruter avatar Mar 18 '25 14:03 westonruter