Skip to content
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 Web Vitals information using web-vitals.js #765

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

adamsilverstein
Copy link
Sponsor

@adamsilverstein adamsilverstein commented May 11, 2023

This PR aims to address #714.

Here is what I have so far -

  • Adds a new "Web Vitals" panel to Query monitor.
  • Unlike other panels in QM data is collected on the front end, when the page loads (FCP & TTFB) and when the user interacts with the page (FID, LCP) - I'm not sure how valuable FID is currently since it is more of a real user metric. Two final metrics CLS and INP are not available until page unload and the web-vitals docs suggests sending these off to a collection endpoint, I discuss some options below.

To use this on the front end, you need to have the wp-admin toolbar enabled, then open query monitor to the Web Vitals tab and reload the page. You will now see the initial metrics:

image

Once you interact with the page, you will see LCP and FID appear:
image

Some ideas to measure INP and CLS would be:

  • Output the details to the console when available (maybe log all values to console?) that way developers could leave console uncleared ("Preserve log") and check values from previous loads
  • send the values to a REST endpoint and store them, maybe with the existing data collector?

We could also leave these off for now and work on adding them in a follow up change. What do you think @johnbillion?

@crstauf
Copy link
Contributor

crstauf commented Jun 21, 2023

Does this auto run, or wait to be requested? If it auto runs, can we please include a way to disable it from auto running? I'm certainly not interested in this running on every page load.

@crstauf
Copy link
Contributor

crstauf commented Jul 3, 2023

@adamsilverstein Can you please respond to my previous comment?

Does this auto run, or wait to be requested? If it auto runs, can we please include a way to disable it from auto running? I'm certainly not interested in this running on every page load.

@adamsilverstein
Copy link
Sponsor Author

adamsilverstein commented Jul 3, 2023

@adamsilverstein Can you please response to my previous comment?

Does this auto run, or wait to be requested? If it auto runs, can we please include a way to disable it from auto running? I'm certainly not interested in this running on every page load.

Hi @crstauf

Good question (just saw it now)! Turns out, this was enqueueing on every page load which seems excessive (even though the actual impact is very low). I pushed an update (0aaf299) so that it only runs when the admin-bar is showing which means only when you are logged in. I think this makes sense, since you need the admin-bar to access QM anyway, right? Does this address your concern about running on every page load?

Note: also move the enqueue to the footer, and will add "defer" once 6.3 ships and this is part of the Script API. The script doesn't need to load early to work ref

Looks like QM queues up a bunch of stuff regardless, so I assume it is best practice to not run QM in production?

@crstauf
Copy link
Contributor

crstauf commented Jul 3, 2023

@adamsilverstein I was thinking a constant or filter to enable the panel: running a performance evaluation on every request is unnecessary, even if it's impact is low.

@adamsilverstein
Copy link
Sponsor Author

@adamsilverstein I was thinking a constant or filter to enable the panel: running a performance evaluation on every request is unnecessary, even if it's impact is low.

Sounds reasonable - I'll leave that suggestion up to @johnbillion.

Small clarification on the operation: the script doesn't actually run any additional performance evaluation, instead the script uses the performance observer API to show information that the browser performance APIs have already collected (see ref). So the impact is really just the overhead of loading the script itself which is why I said it is minimal. (Not disagreeing that it should only be enqueued when needed!)

@crstauf
Copy link
Contributor

crstauf commented Jul 3, 2023

Oh? The browser has already done the evaluation, all the script does is get the info from the browser? Fascinating. That resolves my concern then.

May be smart to have a default output, waiting for the info to populate, as well as a situation where web-vitals has been dequeued.

@crstauf
Copy link
Contributor

crstauf commented Jul 10, 2023

May be smart to have a default output, waiting for the info to populate, as well as a situation where web-vitals has been dequeued.

@adamsilverstein ☝️

@crstauf
Copy link
Contributor

crstauf commented Jul 19, 2023

@adamsilverstein Submitted PR for the notice and handling missing web-vitals: adamsilverstein#1.

@adamsilverstein
Copy link
Sponsor Author

@adamsilverstein Submitted PR for the notice and handling missing web-vitals: adamsilverstein#1.

Thanks @crstauf! will review.

adamsilverstein and others added 2 commits July 20, 2023 14:00
Add waiting notice and support missing `web-vitals` script
@adamsilverstein
Copy link
Sponsor Author

@adamsilverstein Submitted PR for the notice and handling missing web-vitals: adamsilverstein#1.

Thanks @crstauf! will review.

merged, thanks! Still awaiting review from @johnbillion :)

@johnbillion
Copy link
Owner

Thanks for the reminder! I've put a temporary pause on new features until the spring.

@adamsilverstein
Copy link
Sponsor Author

adamsilverstein commented Mar 25, 2024

Thanks for the reminder! I've put a temporary pause on new features until the spring.

Happy spring! 💐 @johnbillion 💐

I realize you may be busy with the rewrite you mentioned, let me know if you can use some help or testing there.

I updated the PR here with the latest web-vitals.js library. I'm doing that manually so far to keep it simple. Since the package is available on npm, we could easily change this to be automatically imported as part of the build process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants