-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add JS build step to utilize local copy of web-vitals.js #893
Comments
@westonruter can you please assign it to me? |
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 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
|
e6a51ce adds a Also see: #556 (comment) |
Fixed by #953 |
I just realized that in order for the built files to be included in releases, we'll need to migrate from |
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 |
Hummm. After reading #217 it seems the reason for switching from |
@westonruter Opened a proposal PR to bring plugin build as a Webpack step - #1008 |
Marking this as blocked per #1008 (review). |
@thelovekesh This is now unblocked and can be closed, correct? |
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
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.The text was updated successfully, but these errors were encountered: