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
Feat: Implement speculative prerendering #13616
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for sharing a first draft! This already seems to work quite well and makes for near-instant navigations to single stories or the story editor. 👏 🚀
Hi @swissspidy ! |
The right hook to print the rules on the frontend would be However, the stories on the frontend all open in a lightbox using amp-story-player, so I don't think it makes sense to add a rule for those, as it won't have an effect (please correct me if I'm wrong). What we could add a rule for is the "View all stories" link that is displayed below the stories. That said: What I don't like about just hooking into That means we would need to design ➡️ Long story short, maybe let's hold off on any frontend stuff for now and focus on the admin side of things. |
Also remove URLs for archive page
The quickEdit test is failing here too. Debugged it, and it seems the response returned here is 403. web-stories-wp/packages/e2e-tests/src/specs/wordpress/quickEdit.js Lines 87 to 90 in da1f746
This test logs in as author, but doing the same manually does not return a 403. @swissspidy WDYT could be going wrong here? |
Are speculation rules added on that page and Is the browser perhaps preloading the link? Maybe that causes some issues? Could be that a simple Puppeteer update fixes this, but we have some more errors on that PR 🙃 |
I don't like that, because
The Web Stories block can be added dozens of times on a page. Adding the rules in |
That makes sense. I'll take a closer look at this. If you have any ideas, please do let us know. |
So I think we can actually simplify this a lot address this issue. We don't actually need to know whether a stories block is on the page or not. We can simply add the rules everywhere on the frontend, that little inline JSON doesn't hurt. And if there's no block, nothing happens as there is nothing to preload. In
In
|
- Remove the Dashboard injection - Hook print_rules into admin_footer and wp_footer. - Remove generate_matches, get_matches_for_page and load_rules - In get_rules() add an apply_filters() call
I have pushed some commits that implement your suggestions. Would you please take a look? |
Interestingly it seems to pass now with the new version! |
includes/Admin/Dashboard.php
Outdated
'where' => [ | ||
'and' => [ | ||
[ | ||
'href_matches' => [ $edit_story_url, $new_story_url ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add $view_story_url
here too so that links to the story permalink can be preloaded.
We should also test whether $view_story_url
should be in prefetch
instead, not prerender
, given that it's usually loaded in a lightbox anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested both prefetch
and prerender
for $view_story_url
, and I couldn't see a noticeable difference. Prerender appeared to be just slightly faster, in terms of page loading times.
…com/GoogleForCreators/web-stories-wp into feat/13603-speculative-prerendering
OK so we have some timeouts apparently? yay fun 🙃 Can you try updating all our Puppeteer/e2e related dependencies (maybe in a new PR)? Just so that the browser we use during tests understands the speculation rules correctly. |
Summary
This PR makes use of the Speculation Rules API to enable dynamically prerendering editor and story URLs.
User-facing changes
Prerendering for these URLs has been enabled for the following pages:
Testing Instructions
This PR can be tested by following these steps:
WIP
Reviews
Does this PR have a security-related impact?
No.
Does this PR change what data or activity we track or use?
🤔
Does this PR have a legal-related impact?
No.
Checklist
Type: XYZ
label to the PRFixes #13603