Add JS build step to utilize local copy of web-vitals.js

[westonruter]

Feature Description

As noted in #876 (comment), the Image Loading Optimization module (see #869) is currently loading web-vitals.js directly from unpkg.com:

performance/modules/images/image-loading-optimization/detection/detect.js

Lines 317 to 321 in 03f3252

	// TODO: Use a local copy of web-vitals.
	const { onLCP } = await import(
	// eslint-disable-next-line import/no-unresolved
	'https://unpkg.com/web-vitals@3/dist/web-vitals.js?module'
	);

This is not ideal from a performance perspective (due to the additional DNS lookup) nor from a privacy perspective (potentially). In any case, for any future consideration for merging this module into WordPress core, a local copy off the JS file will certainly be required.

A build process is also needed for the proposed Partytown module (#556 (comment)).

In short, I believe this would entail having a new command added to the npm scripts which would copy the web-vitals.js module script into the plugin, while keeping this copy .gitignore'ed. This file would nevertheless need to be included in any PerfLab plugin ZIP/deploy or when eventually when the standalone plugin is built.

[thelovekesh]

@westonruter can you please assign it to me?

[westonruter]

Thinking about this some more...

I think we could actually avoid having to do any build step at all during development, as we can refer directly to the file in node_modules. But what would be needed is during the plugin dist build step to copy the node_modules/web-vitals/dist/modules directory into a .gitignore'd directory like like web-vitals-{version}, where the versioned directory is key for cache-busting when new versions are released. Then the build process can replace any references to node_modules and package.json with the actual file path. For example, consider this patch:

diff --git a/modules/images/image-loading-optimization/detection.php b/modules/images/image-loading-optimization/detection.php
index 7eb999ec..f73488d5 100644
--- a/modules/images/image-loading-optimization/detection.php
+++ b/modules/images/image-loading-optimization/detection.php
@@ -35,6 +35,11 @@ function ilo_get_detection_script( string $slug, array $needed_minimum_viewport_
 	 */
 	$detection_time_window = apply_filters( 'ilo_detection_time_window', 5000 );
 
+	// BEGIN: Web Vitals library import.
+	$web_vitals_version = json_decode( file_get_contents( __DIR__ . '/../../../package.json' ), true )['dependencies']['web-vitals'];
+	$web_vitals_lib_src = add_query_arg( 'ver', $web_vitals_version, plugin_dir_url( __FILE__ ) . '/../../../../node_modules/web-vitals/dist/modules/index.js' );
+	// END: Web Vitals library import.
+
 	$detect_args = array(
 		'serveTime'                   => microtime( true ) * 1000, // In milliseconds for comparison with `Date.now()` in JavaScript.
 		'detectionTimeWindow'         => $detection_time_window,
@@ -45,6 +50,7 @@ function ilo_get_detection_script( string $slug, array $needed_minimum_viewport_
 		'urlMetricsNonce'             => ilo_get_url_metrics_storage_nonce( $slug ),
 		'neededMinimumViewportWidths' => $needed_minimum_viewport_widths,
 		'storageLockTTL'              => ilo_get_url_metric_storage_lock_ttl(),
+		'webVitalsLibrarySrc'         => $web_vitals_lib_src,
 	);
 	return wp_get_inline_script_tag(
 		sprintf(
diff --git a/modules/images/image-loading-optimization/detection/detect.js b/modules/images/image-loading-optimization/detection/detect.js
index 48b002fd..d5f532be 100644
--- a/modules/images/image-loading-optimization/detection/detect.js
+++ b/modules/images/image-loading-optimization/detection/detect.js
@@ -141,6 +141,7 @@ function getCurrentTime() {
  * @param {string}                   args.urlMetricsNonce             Nonce for URL metrics storage.
  * @param {Array<number, boolean>[]} args.neededMinimumViewportWidths Needed minimum viewport widths for URL metrics.
  * @param {number}                   args.storageLockTTL              The TTL (in seconds) for the URL metric storage lock.
+ * @param {string}                   args.webVitalsLibrarySrc         URL to the web-vitals module script.
  */
 export default async function detect( {
 	serveTime,
@@ -152,6 +153,7 @@ export default async function detect( {
 	urlMetricsNonce,
 	neededMinimumViewportWidths,
 	storageLockTTL,
+	webVitalsLibrarySrc,
 } ) {
 	const currentTime = getCurrentTime();
 
@@ -271,11 +273,7 @@ export default async function detect( {
 		} );
 	}
 
-	// TODO: Use a local copy of web-vitals.
-	const { onLCP } = await import(
-		// eslint-disable-next-line import/no-unresolved
-		'https://unpkg.com/web-vitals@3/dist/web-vitals.js?module'
-	);
+	const { onLCP } = await import( webVitalsLibrarySrc );
 
 	/** @type {LCPMetric[]} */
 	const lcpMetricCandidates = [];

The PHP code in between the BEGIN and END comments would need to get modified by the dist build step to refer to the files that are copied from node_modules.

What's more is that instead of loading index.js we can actually instead load onLCP.js since this is the only file we actually need. This reduces the overall payload by about a half. Update: Better to link to the standard web-vitals.js build since it is a minified bundle.

[thelovekesh]

e6a51ce adds a copy-vendor-libs command which can handle the cases for ILO and Partytown module and any other such future requirements where vendor files might need to be shipped with plugin builds.

Also see: #556 (comment)

[westonruter]

Fixed by #953

[westonruter]

I just realized that in order for the built files to be included in releases, we'll need to migrate from .gitattributes over to .distignore. Otherwise, the web-vitals file won't be included in the release. See https://github.com/10up/action-wordpress-plugin-deploy?tab=readme-ov-file#excluding-files-from-deployment

[thelovekesh]

.gitattributes was originally added in #223

But given the plugin size is increasing which brings scope for adding more files, should we switch to a more robust option? Maybe create a task in Webpack to handle the plugin build better? I am not sure if .distignore gives the flexibility of using glob patterns but surely we can use glob for "files to be ignored" with Webpack, and we can directly pass the build path with BUILD_DIR env.

[westonruter]

Hummm. After reading #217 it seems the reason for switching from .distignore to .gitattributes was for the sake of being able to obtain a release ZIP via GitHub rather than have to do a build. Nevertheless, this is pretty much becoming obsolete because of the JS build step and the move to standalone plugins, both of which require a build step anyway. So I think we should go back to .distignore. The .distignore file does support globs. It's the same format as .gitignore and is utilizing the GitignoreChecker PHP library.

[thelovekesh]

@westonruter Opened a proposal PR to bring plugin build as a Webpack step - #1008

[westonruter]

Marking this as blocked per #1008 (review).

[westonruter]

@thelovekesh This is now unblocked and can be closed, correct?