WordPress
Add ability to prime URL metrics
Summary
Fixes #1311
Relevant technical choices
This PR introduces a new mechanism for priming URL metrics across the site. It uses a newly added submenu page in the Tools menu and automatically primes URL metrics when a post is saved in the block editor.
TODOS:
- [ ] Add tests
Demos
Settings page:
https://github.com/user-attachments/assets/b562027b-809a-4f13-acc9-6df9958f5e73
Saving post in Block Editor:
https://github.com/user-attachments/assets/b21b24d0-5e53-441f-a2dd-ecd07b408f95
Codecov Report
:x: Patch coverage is 6.59898% with 368 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 65.78%. Comparing base (191e853) to head (7afe854).
Additional details and impacted files
@@ Coverage Diff @@
## trunk #1850 +/- ##
==========================================
- Coverage 68.68% 65.78% -2.91%
==========================================
Files 90 93 +3
Lines 8093 8481 +388
==========================================
+ Hits 5559 5579 +20
- Misses 2534 2902 +368
| Flag | Coverage Δ | |
|---|---|---|
| multisite | 65.78% <6.59%> (-2.91%) |
:arrow_down: |
| single | 33.93% <3.29%> (-1.48%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Just to not leave you hanging: I'm probably not going to be able to review this in depth and provide feedback for a couple more weeks. I'm preparing for travel to WordCamp Asia this weekend and I'll be giving a talk about Optimization Detective, so I need to focus on preparing for that.
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.
If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
Co-authored-by: b1ink0 <[email protected]>
Co-authored-by: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>
Co-authored-by: phanduynam <[email protected]>
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@b1ink0 Thanks for the PR, this looks really promising, and I think it's going to be a super powerful feature. I'll take a closer look next week.
One early thought: I wonder whether we should make this feature conditionally available based on a check on how much content the site has. For most sites, this feature is probably great, but we may want to use caution for sites that have e.g. 1000s of posts. We could make the condition filterable so that sites can still opt in, but I think a safeguard like this would be useful.
Regarding the milestone, I think we should target this for a future release of Optimization Detective, such as 1.1.0. We're already in 1.0.0-beta, so it wouldn't be right to add an entirely new feature to 1.0.0 now.
@b1ink0 Sorry for the delay with reviewing. It's probably going to be another week before I'll get a chance. In the meantime, I have another area you can explore here related to this. Right now there are two methods for priming URL Metrics:
- Priming URLs by going to the Optimization Detective > Tools admin screen.
- Publishing a post in the block editor.
There is another use case which I mentioned in https://github.com/WordPress/performance/issues/1311#issuecomment-2269555288:
This would be cool to implement as a CLI tool as well, using Puppeteer.
In order to facilitate this, I think there should be a new REST API endpoint in the optimization-detective namespace (but beware code being touched in https://github.com/WordPress/performance/pull/1865) which exposes information such as:
- The viewport group min/max widths, i.e.
od_get_breakpoint_max_widths(). - The sample size for each URL Metric group.
- A list of the URLs captured in
od_url_metricsposts which have URL Metrics withtimestampolder thanod_get_url_metric_freshness_ttl(). - A list of URLs pulled from
wp-sitemap.xmlwhich don't have any URL Metrics collected for them.
This endpoint need not be public. It could require authentication.
With this information in hand, it should be possible to construct a script with Puppeteer which loops over all URLs needing URL Metrics with each of the viewport sizes in order to keep URL Metrics fresh across an entire site without requiring any frontend anonymous writes. This could be part of some daily system cron on the server (not wp-cron). As an alternative to the REST API endpoint, this data could be exposed instead via a new WP-CLI command for piping into a Puppeteer script. Similarly with the iframe on the admin screen for priming URL Metrics, the Puppeteer browser session would probably need to be logged-in as a user who can od_store_url_metric_now (though we may want to revisit how the capabilities work here).
With this in place, we could eliminate the short-circuiting currently being done when the REST API is not available, introduced in https://github.com/WordPress/performance/pull/1762. If the user is an administrator, the REST API must be available. So od_is_rest_api_unavailable() can be changed to always return true if is_user_logged_in() && current_user_can( 'od_store_url_metric_now' ). There's probably an alternative mechanism to better check if the REST API is available, like dispatching an OPTIONS request to /url-metrics:store via rest_get_server()->dispatch() which will cause the rest_authentication_errors filter to apply. (Granted, the REST API can still be disabled via the web server in which case we still need to do our loopback requests, but we can do so as an authenticated user and not anonymously.)
Conversely, if there are URL Metrics collected for the current response and the user is not authenticated and yet the REST API is not available, then in this case we can go ahead and optimize the page but we can prevent any of the detection logic from being served.
The ability to prime URL Metrics as you're exploring here will likely be critical to Optimization Detective to being feasible for deployment on high traffic and high profile sites.
Oh, and the Puppeteer browser will need to have the same ability as the iframe'ed page to determine whether the page is "done" loading (i.e. URL Metric has been constructed and has been forcibly submitted). When this signal is received, the Puppeteer browser can move on to load the next URL or the same URL in the next viewport.
Something else that comes to mind: the URL Priming process could be sped up by skipping URLs for viewports which already have a fresh URL Metric in the group.
This could be something else which is facilitated by #1919. In particular, the integration could hook into the od_start_template_optimization action to insert a SCRIPT in the page which sends a message to the parent window (or Puppeteer) to indicate the status of all the URL Metric groups. So let's say the process by default accesses all URLs with the mobile viewport. When that mobile viewport loads, we can learn at that point that the other viewport groups are populated with fresh URL Metrics and then the process can skip to the next URL.
@westonruter
I was experimenting with creating a WP CLI command to act as an orchestrator. Its purpose would be to check the system environment for node, which is required for Puppeteer, install Puppeteer if necessary, and then execute the priming logic. However, this approach relies on shell_exec, which might be disabled for security reasons.
As an alternative, I considered including a Puppeteer script project within the Optimization Detective plugin’s directory, along with its package.json. However, this would require the admin to manually navigate to the directory, run npm install, and then execute npm run start or node index.js.
I would like to know if you have any other idea for this?
@b1ink0
As an alternative, I considered including a Puppeteer script project within the Optimization Detective plugin’s directory, along with its
package.json. However, this would require the admin to manually navigate to the directory, runnpm install, and then executenpm run startornode index.js.
Yes, I think this is the way to go. We can include a subdirectory in the plugin which contains the Puppeteer script. Instead of a site owner doing npm install from that directory on the production site, it could also make sense to publish an npm package so that there's no node_modules directory in the document root (which could result in security vulnerabilities, depending on what assets are in there). But for development purposes, being able to install the dependencies from the plugin itself is useful. So if it were available as an npm package, then it could be installed globally and then call WP-CLI to obtain the required data to do the gathering of the URL Metrics:
npx @wordpress/performance-od-url-metric-collector
So yeah, I think the user would invoke node which would then do system calls to WP-CLI rather than calling WP-CLI which would do system calls to node. This is because WP-CLI is a dependency here for the Puppeteer script, not the other way around, I think.
@westonruter I have created this https://github.com/b1ink0/performance/commit/dd274ef6097c9ceabad9f7aba1aabca930a7ec08 (NOTE: POC branch is created from #1850 latest commit) POC for Puppeteer CLI. I would like to know if I am heading in the right direction.
https://github.com/user-attachments/assets/fce06bac-0cbe-45e1-9c57-d86f7f159be4
If this approach is correct, should I add these changes to current PR #1850?
Another important thing to note is that I had to bypass the following check in detect.js, as it was taking a long time to resolve and, in some cases, never resolved. I’m still investigating the cause when using Puppeteer CLI.https://github.com/WordPress/performance/blob/a3b0d7c3342521d0a23c2fbba2445488b01645c1/plugins/optimization-detective/detect.js#L477-L482
@b1ink0 yes, that is looking good!
In regards to the idle callback, have you looked at using waitUntil: 'networkidle0' as opposed to waitUntil: 'load'?
Nevertheless, that may not be right either. Seems like this is a known issue with Puppeteer and requestIdleCallback: https://github.com/puppeteer/puppeteer/issues/10350#issuecomment-1621956186. But maybe you'll find more info on this.
Aside: We may want to have Optimization Detective fire a custom event when it has gotten to the point where it is waiting for pagehide. This could be used be a script injected by Puppeteer as an alternative to the message that is passed to the parent window as in the case of the iframe.
@westonruter
In regards to the idle callback, have you looked at using waitUntil: 'networkidle0' as opposed to waitUntil: 'load'?
I tested both approaches: load with requestIdleCallback promise disabled and networkidle0 with requestIdleCallback promise enabled. The networkidle0 approach slows down URL metrics collection compared to the load event, as it waits for the network to become idle. However, if we prioritize accuracy, networkidle0 is the safer choice since the delay likely results in more accurate URL metrics.
-
load: -
networkidle0:
Regarding other methods I explored, we could replace requestIdleCallback with a custom implementation using setTimeout, but that wouldn't be a true solution.
Additionally, should the Puppeteer CLI code be added to the current PR, or should it be part of a separate PR after this one is merged into trunk?
Additionally, should the Puppeteer CLI code be added to the current PR, or should it be part of a separate PR after this one is merged into trunk?
@b1ink0 If it makes it easier for you to prototype this, I think it's fine to add it to this PR. We can always remove parts of the PR prior to review, and then immediately revert the removals in subsequent PRs.
The JS Lint workflow is currently failing because the dependencies for the Priming CLI are not being installed. This is happening because the workflow only installs dependencies from the root package.json, while the package.json of Priming CLI is located in a subdirectory.
This could be solved by updating js-lint.yml to include command to also install dependencies for Priming CLI.
diff --git a/.github/workflows/js-lint.yml b/.github/workflows/js-lint.yml
index 365d32b9..b840bc80 100644
--- a/.github/workflows/js-lint.yml
+++ b/.github/workflows/js-lint.yml
@@ -45,7 +45,9 @@ jobs:
node-version-file: '.nvmrc'
cache: npm
- name: npm install
- run: npm ci
+ run: |
+ npm ci
+ npm ci --prefix plugins/optimization-detective/priming-cli
- name: JS Lint
run: npm run lint-js
- name: TypeScript compile
cc: @westonruter
This could be solved by updating
js-lint.ymlto include command to also install dependencies for Priming CLI.
@b1ink0 This makes sense to me. Might as well include that in this PR.
Re: https://github.com/WordPress/performance/pull/1850#discussion_r2318986932
@b1ink0:
What about allowing
fetchpriority=highwhen any two group have theIMGas a LCP element?
I don't think that is safe because mobile and phablet may have the same LCP element, but then desktop has something else. Or desktop and tablet could have the same LCP element, but mobile and tablet have something else. This would mean the LCP image would be incorrectly prioritized more often. The current logic is to only add fetchpriority=high when the smallest viewport group and largest group both have the same LCP element, unless there are any intermediate groups have a different LCP element. (Often the intermediate groups will have no URL Metrics reported, given the current breakpoints, since mobile and desktop users are by far the most common.)
But adding fetchpriority=high to the actual element isn't so important because we are adding the preload links anyway. So adding fetchpriority to the IMG is just a little something extra.
there is no viewport for the group between
480and600. Additionally, due to the current max widths (480,600,782) that determine the group, the URL metric for the360width viewport gets overridden by the414viewport.
I think this highlights how uncommon the "phablet" viewport is, or rather that the "small" breakpoint in Gutenberg is very uncommon:
$break-medium: 782px; // adminbar goes big
$break-small: 600px;
$break-mobile: 480px;
When asking Gemini about devices with a viewport width between 480px and 600px, it says:
However, this range typically encompasses some older, larger smartphones when held in landscape mode, as well as some smaller tablets and the "portrait" view of some foldable phones.
And:
In the broader spectrum of device popularity, the 480px to 600px viewport range represents a smaller but still relevant segment of the market. Based on recent data, the most popular viewport widths for mobile devices tend to be narrower, clustering around the 360px to 430px range. For instance, resolutions like 360x800 and 390x844 hold a significant share of the mobile market.
Conversely, tablet and desktop viewports are considerably wider. Common tablet viewports often start at 768px, with popular resolutions being 768x1024 and 810x1080. Desktop resolutions are even larger, with 1920x1080 and 1536x864 being among the most prevalent.
Therefore, devices with viewport widths between 480px and 600px are less common than the dominant smaller mobile screen sizes and the much larger tablet and desktop screens. However, this intermediate range remains an important consideration for ensuring a truly comprehensive responsive design that caters to all users, regardless of their device. As a web developer, accounting for this "in-between" screen size in your media queries can help prevent layout issues and provide a seamless experience for users with these specific devices.
Now regards to this:
We could change the breakpoint max widths to (
360,414,782), which would solve this issue. However, I am not sure about this. What do you think?
These are supposed to be the max widths for the breakpoints, not the most common viewports. We should align them with the breakpoints defined in the Gutenberg's _breakpoints.scss as much as possible. The problem is that there is no breakpoint between 480px and 280px, at least as used in Gutenberg. The stylesheet does reference three others which are commonly used in WordPress elsewhere, however:
// max-width: 400px *
// max-width: 380px
// max-width: 320px *
Given this, we may need to introduce a new breakpoint lower than 480px so that we can capture URL Metrics for more accurate optimizations since many flavors of mobile fit in the 0-480px viewport group. I asked Gemini to give me a bar chart showing the popular viewport widths (which I do not know whether it is accurate!):
Importantly: there are no popular devices between the 412px and 782px. So the current 600px max breakpoint width is actually going to be largely unused on real sites. It would seem more useful to add a breakpoint that subdivides the large mobile set of mobile viewports, for large smartphone and small smartphone:
According to the common WordPress breakpoints referenced in that Gutenberg stylesheet, it seems like 380px would be the logical choice to divide those two groups. The devices that fall into those two groups, again per Gemini:
Less Than or Equal to 380px
These devices have a viewport width of 380px or less. This category is dominated by many popular iPhones and smaller Android devices.
- iPhone SE (1st gen): 320px
- iPhone 6, 7, 8, SE (2nd/3rd gen): 375px
- iPhone X, XS, 11 Pro, 12/13/14/15 mini: 375px
- Samsung Galaxy S8, S9: 360px
- Samsung Galaxy S10e, S20, S21, S22: 360px
- Google Pixel 3, 4, 5, 6a: 360px
- Nokia 8 Sirocco: 360px
Greater Than 380px (and less than 481px)
These devices have a viewport width greater than 380px but less than 481px. This group includes most standard and larger-sized modern smartphones.
- iPhone 6/7/8 Plus: 414px
- iPhone XR, 11: 414px
- iPhone XS Max, 11 Pro Max: 414px
- iPhone 12/13/14/15 Pro: 390px
- iPhone 12/13/14 Pro Max, 14/15 Plus: 428-430px
- Google Pixel 4 XL, 6, 7: 412px
- Samsung Galaxy S10, S21 Ultra: 412px
- Samsung Galaxy Note 9, 10, 20: 412-414px
- OnePlus 7T, 8, 9: 412px
- Samsung Galaxy Z Fold (unfolded): 480px
All this to say, we could add 380 to the existing breakpoint max widths: 480, 600, 782. This would segment the major mobile clusters as identified by Gemini:
For instance, resolutions like 360x800 and 390x844 hold a significant share of the mobile market.
This would allow you to use the following device viewports as you had compiled above:
$device_viewports = array(
array( 'width' => 360, 'height' => 780 ), // Small Smartphone.
array( 'width' => 414, 'height' => 896 ), // Large Smartphone.
array( 'width' => 768, 'height' => 1024 ), // Tablet.
array( 'width' => 1920, 'height' => 1080 ), // Desktop.
);
Each would get assigned to a different viewport group. And given the relative unpopularity of devices between 480px and 600px, it probably doesn't make sense to add a device specifically to test it. We even might want to consider removing 600 from the max viewport groups.