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

Implement new module to use Speculation Rules API for prerendering documents on hover #733

Merged
merged 17 commits into from
Dec 15, 2023

Conversation

felixarntz
Copy link
Member

@felixarntz felixarntz commented May 22, 2023

Summary

Fixes #897.

ThisPR is for a new module module to use the recently introduced Speculation Rules API to prerender URLs in WordPress as soon as the user hovers over them.

Learn more about the Speculation Rules API

Related: URL pattern testing tool

How to use

  • The Speculation Rules API is available in Chrome 108 or newer.
  • The full set of features however, which are needed for this module to work, are only available via origin trial in Chrome 113 or newer. You need to sign up for the origin trial with the origin of the site where you would like to test it. Sign up for the origin trial
    • Once you have signed up and received your origin trial token, you can set it in a PLSR_ORIGIN_TRIAL_TOKEN constant (e.g. in your wp-config.php file) and the module will take care of including its opt-in tag.
  • The API is expected to become fully available without origin trial in Chrome 121.
  • For debugging in Chrome Developer Tools, you can enable a new section by going to Settings > Experiments (within the developer toolbar) and check the box "Enable Preloading Status Panel in Application panel" (towards the very end of the list). This will enable a "Preloading & Prerendering" section in the "Application" panel (see screenshot below).
Preloading & Prerendering section in Chrome Developer Tools

Relevant technical choices

  • Currently, the list of speculation rules is hard-coded, which is just a starting point. Eventually, the module should be enhanced to include an API to manage the speculation rules programmatically in WordPress.

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@felixarntz felixarntz added [Focus] JS & CSS no milestone PRs that do not have a defined milestone for release labels May 22, 2023
@felixarntz
Copy link
Member Author

@westonruter @domenic I've addressed your feedback.

Note that the unit test failures are due to an unrelated problem in trunk, see #881 for a fix for that.

@domenic
Copy link

domenic commented Nov 14, 2023

This looks great to me!

Note that if you want to only target Chrome 121+, you could simplify a couple things:

@felixarntz
Copy link
Member Author

@domenic

Note that if you want to only target Chrome 121+, you could simplify a couple things:

For now, I'll keep the Origin-Trial. Eventually this should be removed for sure, before the module ships in a Performance Lab version. But now for early testing IMO worth keeping.

For your second point, probably the same. Unless this is already shipped or will ship in an earlier Chrome version? Is there any feature detection available for that change?

@domenic
Copy link

domenic commented Nov 15, 2023

For your second point, probably the same. Unless this is already shipped or will ship in an earlier Chrome version? Is there any feature detection available for that change?

This change is also Chromium 121+. It's harmless to keep the extra parts, so if you're targeting such browsers then I'd suggest keeping it.

You can feature-detect this using the following:

const hasNewSemantics = (new URLPattern("/*", "https://example.com/a?b&c")).test("https://example.com/d?e");

modules/js-and-css/speculation-rules/helper.php Outdated Show resolved Hide resolved
Comment on lines 20 to 21
'/wp-login.php',
'/wp-admin/*',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's also important to include any URL that includes a _wpnonce query parameter. Typically such nonce URLs point to the admin, but there are key examples where this is not the case: https://wpdirectory.net/search/01HFCME02F3HYMR93WDWV6KSX4

For example, BuddyPress has frontend nonce links to favorite/unfavorite a link.

Copy link
Member Author

@felixarntz felixarntz Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, this is a tricky one though. Nonces can also be sent through any other kind of parameter. I'm not sure how far we should go here by default, as what you're suggesting would just be a "best guess", but not actually reliable. So potentially it's better to support just what WordPress core does and require plugins to take care of this themselves. I don't think core uses nonces in the frontend.

I realize this may not work either (as adoption is hard), but also questioning whether it's reasonable to start "patching" for what plugins may do here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true that nonces can be sent via any parameter, but the wp_nonce_url() function uses the _wpnonce parameter by default. So I think it makes sense to omit them by default.

modules/js-and-css/speculation-rules/helper.php Outdated Show resolved Hide resolved
@jeremyroman
Copy link
Contributor

I noticed this unconditionally chooses prerender. Could/should this be an administrator-controlled setting? There are reasons a developer might prefer prefetch to prerender under some circumstances (for example, a script which is not ready for prerender runs, or a prefetch could succeed but the site would be too heavy for prerender to be practical).

// Prerender any URLs within the same site.
array(
'href_matches' => '/*\\?*',
'relative_to' => 'document',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

relative_to is unnecessary if the rules are inline in the document (but if you do have some reason for this to be relative_to, the below exclusion probably should be too.

Also, if WordPress is installed at a path other than /, does that need to be accounted for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added support for that in ce0e56b

@felixarntz felixarntz changed the base branch from trunk to feature/speculationrules November 20, 2023 17:52
$base_href_exclude_paths = array(
$prefixer->prefix_path_pattern( '/wp-login.php', 'site' ),
$prefixer->prefix_path_pattern( '/wp-admin/*', 'site' ),
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the other PHP files outside of the normal frontend:

  • wp-activate.php
  • wp-signup.php

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about those too, but left them out for now. We can probably add them, but I was thinking that it's not critical and could be done in its own issue once we have a first version of the module merged (i.e. this PR).


$base_href_exclude_paths = array(
$prefixer->prefix_path_pattern( '/wp-login.php', 'site' ),
$prefixer->prefix_path_pattern( '/wp-admin/*', 'site' ),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why exclude all wp-admin paths, wouldn't you want to enable prefetching on those to speed up wp-admin? or is this intended strictly for front end so far?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamsilverstein Yes, for now this is only frontend. The whole speculation rules tag is also printed in the frontend only.

Doing it in WP Admin has its own set of complexities, and is less beneficial IMO. Those pages are also a whole lot more dynamic, which makes prerendering difficult.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preloading the editor when hovering over the link in the admin bar would be interesting in the future.

@felixarntz
Copy link
Member Author

This PR is now ready for full review and, once approved, can be merged into the new feature branch. Please see the new module proposal #897 for additional context, including follow up work.

@felixarntz felixarntz changed the title Implement basic module to experiment with speculationrules for prerendering Implement new module to use Speculation Rules API for prerendering documents on hover Dec 6, 2023
Co-authored-by: Weston Ruter <westonruter@google.com>
@felixarntz felixarntz merged commit 307034e into feature/speculationrules Dec 15, 2023
7 checks passed
@felixarntz felixarntz deleted the speculationrules-experiment branch December 15, 2023 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no milestone PRs that do not have a defined milestone for release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants