Update polyfills service to load unconditionally

[thelovekesh]

Summary

• Update Polyfills service to load unconditionally on required pages where we might need polyfills to keep React 17 compatibility.
• Update AfterActivationSiteScan conditional to avoid running site scan on themes and plugins page if React version is >18.

Checklist

• My code is tested and passes existing tests.
• My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

[github-actions]

Plugin builds for 8d3fb24 are ready 🛎️!

• Download development build
• Download production build

[westonruter]

@thelovekesh Can you check with the latest Gutenberg (v14.8, which includes React 18) if the site scanning that happens after activating a theme/plugin works properly? Since this functionality is not on a dedicated screen, we can't polyfill in that case. If it doesn't work, we may need to adjust the is_needed logic in the following method to return false if we determine React 18 is active:

amp-wp/src/Admin/AfterActivationSiteScan.php

Lines 84 to 114 in dfb5402

	public static function is_needed() {
	global $pagenow;
	return (
	is_admin()
	&&
	Services::get( 'dependency_support' )->has_support()
	&&
	! is_network_admin()
	&&
	AMP_Validation_Manager::has_cap()
	&&
	(
	(
	'plugins.php' === $pagenow
	&&
	(
	! empty( $_GET['activate'] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended
	||
	! empty( $_GET['activate-multi'] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended
	)
	)
	||
	(
	'themes.php' === $pagenow
	&&
	! empty( $_GET['activated'] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended
	)
	)
	);
	}

[westonruter]

Yeah, I just checked with Gutenberg 15.1 and on the plugin screen the scan works but I get console warnings:

(The second warning about key also happens with React 17, so it's not unique to React 18.)

It says that it will behave as if React 17 is active, but I wonder if the upgrade to React 19 will remove that back-compat layer.

I see we have two options:

1. Include React 17 in the bundle for amp-site-scan-notice.js.
2. Turn off Plugin/Theme Site Scan Notices when the version of React in core is greater than 17.

I think we should do the latter for now.

[westonruter]

In other words:

diff --git a/src/Admin/AfterActivationSiteScan.php b/src/Admin/AfterActivationSiteScan.php
index 1425505c1..ecfc4c1ae 100644
--- a/src/Admin/AfterActivationSiteScan.php
+++ b/src/Admin/AfterActivationSiteScan.php
@@ -11,6 +11,7 @@
 
 namespace AmpProject\AmpWP\Admin;
 
+use _WP_Dependency;
 use AMP_Options_Manager;
 use AMP_Validation_Manager;
 use AmpProject\AmpWP\Infrastructure\Conditional;
@@ -84,6 +85,8 @@ public function __construct( RESTPreloader $rest_preloader ) {
 	public static function is_needed() {
 		global $pagenow;
 
+		$react = wp_scripts()->query( 'react' );
+
 		return (
 			is_admin()
 			&&
@@ -91,6 +94,8 @@ public static function is_needed() {
 			&&
 			! is_network_admin()
 			&&
+			( $react instanceof _WP_Dependency && version_compare( $react->ver, '18', '<' ) )
+			&&
 			AMP_Validation_Manager::has_cap()
 			&&
 			(

[westonruter]

It looks like the E2E tests need updating to account for this condition.

[thelovekesh]

@westonruter Now using Gutenberg plugin 14.7.3 (for React 17 compatibility) while testing the After plugin activation spec.

[westonruter]

Thank you 🙇

[thelovekesh]

QA Passed ✅

Update Polyfills service to load unconditionally on required pages where we might need polyfills to keep React 17 compatibility.

Before:
No polyfills were being enqueued to the latest WP version by the AMP plugin.

Page Source: /wp-admin/plugins.php, Plugin Version: 2.3.0, WP: 6.2-alpha

After:
Polyfills are being enqueued from the AMP plugin.

Page Source: /wp-admin/plugins.php, Plugin Version: 2.3.0, WP: 6.2-alpha

---

Update AfterActivationSiteScan conditional to avoid running site scan on themes and plugins page if React version is >18.

Note: Gutenberg v15.0.1 has been used since it now enqueues React 18 by default.

Before:
Site Scan Notice is visible on the plugins/themes admin pages when React 18 is being enqueued.

After:
Site Scan Notice is disabled on plugins/themes admin pages when React 18 is being enqueued.

[westonruter]

Re: #7458, I just realized an alternative approach to this which could solve some headaches we've encountered: we could restore this and add the conditions for when the polyfills should be registered. Namely, on the various admin screens. Perhaps that is actually not better since then we'd have to maintain the list of screens here, whereas if we go the other (current) approach we only do the polyfills which the respective service fires the amp_register_polyfills action. We just need to make sure that the amp_register_polyfills action only ever fires on our dedicated screens.

So yeah, nevermind.

[thelovekesh]

With this conditional amp_register_polyfills will only get fired on the AMP plugin screens:

amp-wp/src/Admin/ValidationCounts.php

Lines 132 to 142 in a931e7a

	/**
	* Whether the current screen is pages inside the AMP Options menu.
	*/
	public function is_dedicated_plugin_screen() {
	return (
	AMP_Options_Manager::OPTION_NAME === get_admin_page_parent()
	||
	AMP_Validated_URL_Post_Type::POST_TYPE_SLUG === get_current_screen()->post_type
	);
	}
	}