Interactivity API: Defer hydration until node is scrolled near the viewport#58284
Interactivity API: Defer hydration until node is scrolled near the viewport#58284westonruter wants to merge 19 commits into
Conversation
Co-authored-by: Luis Herranz <luisherranz@gmail.com>
westonruter
added
[Type] Performance
|
Size Change: +149 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
|
This is a great exploration, thanks Weston!! These are my current thoughts:
So, I'm very excited about this, but it also comes with its own set of challenges and opportunities 🙂 What are your thoughts about those points? |
Yeah, that makes sense. Just hopefully authors won't be writing interactive blocks in the mean time that would break if later hydration is delayed. I suppose that wouldn't be the case if they are intended to be hydrated at a later point anyway during client-side navigation.
Good idea. So using
Actually it is currently hydrating before the island enters the viewport. The
Good! Yes, deferring the loading of the modules will further reduce main thread time spent on parsing JS.
Is this currently implemented as it stands now, however? Currently hydration happens at
🎉 |
…zy-hydration * origin/trunk: (47 commits) Interactivity API: Break up long hydration task in interactivity init (#58227) Add supports.interactivity to Query block (#58316) Font Library: Make notices more consistent (#58180) Fix Global styles text settings bleeding into placeholder component (#58303) Fix the position and size of the Options menu, (#57515) DataViews: Fix safari grid row height issue (#58302) Try a fix (#58282) Navigation Submenu Block: Make block name affect list view (#58296) Apply custom scroll style to fixed header block toolbar (#57444) Add support for transform and letter spacing controls in Global Styles > Typography > Elements (#58142) DataViews: Fix table view cell wrapper and BlockPreviews (#58062) Workflows: Add 'Technical Prototype' to the type-related labels list (#58163) Block Editor: Optimize the 'useBlockDisplayTitle' hook (#58250) Remove noahtallen from .wp-env codeowners (#58283) Global styles revisions: fix is-selected rules from affecting other areas of the editor (#58228) Try: Disable text selection for post content placeholder block. (#58169) Remove `template-only` mode from editor and edit-post packages (#57700) Refactored download/upload logic to support font faces with multiple src assets (#58216) Components: Expand theming support in COLORS (#58097) Implementing new UX for invoking rich text Link UI (#57986) ...
| export const init = async () => { | ||
| const pendingNodes = new Set(); | ||
|
|
||
| const intersectionObserver = new window.IntersectionObserver( |
There was a problem hiding this comment.
If there are no nodes below, then this wouldn't need to be constructed in the first place.
| if ( pendingNodes.size === 0 ) { | ||
| intersectionObserver.disconnect(); | ||
| } |
There was a problem hiding this comment.
This would be problematic for islands that are added dynamically during client-side navigations. The IntersectionObserver may need to remain persistently, if init() isn't called again after new islands are added.
Exactly. The only thing I'd like to do that we are not doing is to know which JS/CSS assets belong to each island/block, so we can also control how we load them before hydrating.
Yep!
Ohhh, I didn't see that, sorry. Brilliant!
Exactly. As we hydrate all the islands (which includes all the regions) at |
…ry/interactivity-lazy-hydration
|
Flaky tests detected in d27985e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12503502008
|
|
@westonruter I've been thinking about this, and I believe that if we leave out the part about downloading the JavaScript, we could try to land this in WordPress 6.7. Instead of giving the devs lots of options, it would be good to make all blocks lazy-loaded by default and provide an option to opt-out. Something like this (with a better name): <nav
data-wp-interactive='{
"namespace": "core/navigation",
"load": "asap!"
}'
>
...
</nav>And by default, we hydrate the blocks when they are about to enter the viewport or when the CPU is idle, just as we discussed here. Would you have some bandwidth to work on this during this cycle? |
|
@luisherranz Yes, I do intend to work on this during this cycle! |
|
Awesome! 🙂👏 Ideally, we should merge an initial version into Gutenberg that makes blocks lazy-load by default as soon as possible to provide enough time for people to report any issues with this change in behavior before WP 6.7. To be able to merge this pull request, I think the only thing missing is a way to force the hydration of all blocks that haven't hydrated yet so the Interactivity Router can hydrate all blocks before performing the navigation (to do the HTML diffing during the navigation, we need an initial virtual DOM). We can add later that the blocks hydrate when the CPU is idle and the opt-out. What do you think? |
westonruter
force-pushed
the
try/interactivity-lazy-hydration
branch
from
f763e73 to
fb15565
Compare
My opinion about this:
Oh, there are two types of client-side navigation: region-based client-side navigation and full-page client-side navigation. In the first, the navigation only replaces the content within the regions marked with the So that's true for the upcoming full-page client-side navigation, but not for region-based client-side navigation. Full-page client-side navigation is an option to turn WordPress into something more like a SPA (Single Page Application). However, it will always be an opt-in option. It will never be something that WordPress can do by default. Region-based client-side navigation is what the Query block is using when you disable the "force page reload" option, for example. A while ago, I tried to see if by manually modifying some internal parts of Preact vDOM we could move from a partial hydration (hydrating only some interactive blocks) to hydrating the entire page (the Anyway, why don't we do an initial version without this, and test it out to see what happens? Maybe it's not so bad not to do the diffing of the regions that have not hydrated yet, and we can just remove them and replace them with the new ones. |
westonruter
added
the
[Status] Blocked
…ry/interactivity-lazy-hydration
|
@felixarntz I just re-tested and I cannot reproduce the issue anymore. So yes, I think this is unblocked! |
westonruter
removed
the
[Status] Blocked
…ry/interactivity-lazy-hydration
|
Lazy-hydration is working: Screen.recording.2024-12-21.10.32.40.webmAlso working in the context of a Query Loop block which has page reloads turned off: Screen.recording.2024-12-21.10.33.38.webmAnd interactive blocks work as expected when the full page client side navigation experiment is enabled: Screen.recording.2024-12-21.10.42.24.webmI will note that lazy-hydration basically doesn't do anything when client-side navigation is in effect since only the |
|
It's not clear to me why the Playwright - 6 E2E test is failing. Perhaps not related? In any case, I'm marking this as ready for review now. |
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Those tests do seem to be Interactivity API e2e tests so they're worth investigating. It looks like it's some of the recently added tests that are failing, where new elements are added to the bottom of the page. Could it be that they're outside of a virtual viewport and the tests need to scroll in order to trigger hydration? |
|
I pushed a couple of commits to fix the e2e test issues. It does appear that it was a viewport related hydration issue. The last commit always scrolls to the bottom of the page to hydrate all the interactive regions on the page and not require it for each test. |
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
@westonruter This looks great, only a few minor notes.
| pendingNodes.add( node ); | ||
| intersectionObserver.observe( node ); |
There was a problem hiding this comment.
I think it might be a good idea to keep the hydratedIslands.has check here? No need to even observe nodes that are already hydrated, e.g. if init was called multiple times. Unlikely, but probably a good safety net to have.
| pendingNodes.add( node ); | |
| intersectionObserver.observe( node ); | |
| if ( ! hydratedIslands.has( node ) ) { | |
| pendingNodes.add( node ); | |
| intersectionObserver.observe( node ); | |
| } |
There was a problem hiding this comment.
The hydratedIslands.has() check was moved to the IntersectionObserver callback. It could be added here as well, but after abd3747 that would complicate things a bit since it could be that the nodes.length number would end up being larger than the number of nodes actually observed, meaning the IntersectionObserver would never get disconnected.
|
|
||
| if ( ! hydratedIslands.has( node ) ) { | ||
| const fragment = getRegionRootFragment( node ); | ||
| const vdom = toVdom( node ); |
There was a problem hiding this comment.
In the old logic, this used to populate initialVdom. Is this no longer needed? If not, is there even any value in keeping the initialVdom const around?
There was a problem hiding this comment.
It seems this is a merge conflict resolution on my part. This may be needed:
--- a/packages/interactivity/src/init.ts
+++ b/packages/interactivity/src/init.ts
@@ -48,6 +48,7 @@ export const init = async () => {
if ( ! hydratedIslands.has( node ) ) {
const fragment = getRegionRootFragment( node );
const vdom = toVdom( node );
+ initialVdom.set( node, vdom );
await splitTask();
hydrate( vdom, fragment );
await splitTask();There was a problem hiding this comment.
Looks like it is only being used here:
gutenberg/packages/interactivity-router/src/index.ts
Lines 189 to 200 in ef7afef
There was a problem hiding this comment.
Restored initialVdom.set( node, vdom ) in 387ab6c.
I found that this code in interactivity-router runs as the page loads when the "iAPI: full page client side navigation" experiment is enabled, and it also runs when interacting with a block that makes use of client-side navigation (i.e. the Query block when "Reload full page" is disabled). Nevertheless, the behavior of the page seems to work just as well whether or not initialVdom.set( node, vdom ) is added here.
@DAreRodz For this page cache, is it a problem that initialVdom is not initially populated with all of the interactive regions of the page since they get added only when they come into view?
| const node = entry.target; | ||
| intersectionObserver.unobserve( node ); | ||
| pendingNodes.delete( node ); | ||
| if ( pendingNodes.size === 0 ) { |
There was a problem hiding this comment.
If I understand correctly, the pendingNodes variable is only being populated to check this? I guess there's no way to check if the intersectionObserver is "empty"?
There was a problem hiding this comment.
That's right. There is no "observed count" exposed on the IntersectionObserver interface. We could change this to instead be a simple counter.
--- a/packages/interactivity/src/init.ts
+++ b/packages/interactivity/src/init.ts
@@ -29,7 +29,7 @@ export const initialVdom = new WeakMap< Element, ComponentChild[] >();
// Initialize the router with the initial DOM.
export const init = async () => {
- const pendingNodes = new Set();
+ let observedNodeCount = 0;
const intersectionObserver = new window.IntersectionObserver(
async ( entries ) => {
@@ -40,8 +40,8 @@ export const init = async () => {
const node = entry.target;
intersectionObserver.unobserve( node );
- pendingNodes.delete( node );
- if ( pendingNodes.size === 0 ) {
+ observedNodeCount--;
+ if ( observedNodeCount === 0 ) {
intersectionObserver.disconnect();
}
@@ -76,8 +76,8 @@ export const init = async () => {
setTimeout( resolve, 0 );
} );
+ observedNodeCount = nodes.length;
for ( const node of nodes ) {
- pendingNodes.add( node );
intersectionObserver.observe( node );
}
};This should have the same effect, with a slight benefit that the element references wouldn't be stored in the Set, meaning that there wouldn't be the possibility of a memory leak. (I should have used a WeakSet here originally, probably.)
…ry/interactivity-lazy-hydration * 'trunk' of https://github.com/WordPress/gutenberg: (143 commits) Update: Bundle upload media. (#68522) Add: Media field changing ui to Dataviews and content preview field to posts and pages (#67278) Bump the react-native group with 2 updates (#68095) Check Storybook build on CI for PRs (#68466) Bump the github-actions group across 1 directory with 2 updates (#68436) Classic theme preview: remove admin-bar class name (#68519) Remove geriux as code owner (#68523) Post Featured Image: Adds control to clear the the overlay color (#68525) Components: Standardize reduced motion handling using media queries (#68421) Upgrade Playwright to v1.49 (#68504) Document Outline: Use block client ID as unique 'key' (#68502) Storybook: Add UnitControl story (#67346) Details: Add allowedBlocks and TemplateLock attributes (#68489) Post Comment Link: Show Border Control By Default (#68506) Query Total: Show Border Controls By Default (#68507) RSS: Added Colour support (#66419) Refactor: Separate input form styles to a dedicated stylesheet (#68501) Code quality: Fix typos (#67304) Page List: Added color support (#66430) Fix flaky DataViews list arraow nav e2e tests (#68503) ...
@sirreal Thanks so much! |
| if ( observedNodeCount === 0 ) { | ||
| intersectionObserver.disconnect(); | ||
| } |
There was a problem hiding this comment.
Something else to consider, perhaps for a future PR: What if new content is added to the page dynamically without using the Interactivity API? For example, what if there is a custom infinite scroll implementation that appends content to the page? In this case, the disconnect() here will mean that none of the new elements will get hydrated. This is no breakage compared with before, since any such nodes wouldn't get hydrated anyway since they aren't included in the initial list of nodes returned by document.querySelectorAll( '[data-wp-interactive]' ). If we wanted to account for such nodes being added, then perhaps we should add a MutationObserver which listens for new content being added to the page, and when it is added, look for any new nodes via root.querySelectorAll( '[data-wp-interactive]' ) where root is the new element added.
There was a problem hiding this comment.
In my opinion, if we try to account for every DOM modification that people might make outside of the framework, we can end up with an explosion of complexity. Just imagine React, Vue, or Svelte trying to track DOM modifications made externally to their frameworks. So I don’t think we should consider that scenario. If someone wants to modify the DOM and have the Interactivity API recognize those changes, they should do it through the methods provided by the Interactivity API.
Co-authored-by: felixarntz <flixos90@git.wordpress.org>
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
@westonruter Just one final point of follow-up feedback.
Co-authored-by: Felix Arntz <flixos90@gmail.com>
There was a problem hiding this comment.
Great work. Code-wise, this looks good to me so far 🙂
I still don't have a clear idea of what we should do with the regions that have not been hydrated when there is a region-based client-side navigation.
Right now, we are using Preact's render method in a section of the DOM that is not hydrated. Therefore, the DOM elements of the current page are being deleted, and the elements of the new page are being added by Preact. That means:
- All regions that have not yet been hydrated of the current page are being hydrated, even though they are not yet in the viewport (so we could say that the lazy hydration is not preserved during the navigation).
- All DOM elements of the regions that have not yet been hydrated are being deleted and re-added, which is not ideal.
We have two other options:
-
If the region has not been hydrated yet, simply replace the elements (delete the ones from the current page and add the ones from the new page), but keep tracking the hydration with the intersection observer so that it hydrates (using the
hydratemethod of Preact) when it enters the viewport. That means:- All the DOM elements of the regions that have not yet been hydrated would be deleted and re-added, which is not ideal.
- But at least the lazy hydration would be preserved on the new page.
-
Force the hydration of all regions that have not yet been hydrated so that Preact can do the diffing between the elements of the current page and the elements of the new page. That means:
- The DOM elements are preserved and only the minimum necessary modifications are made.
- All regions that have not yet been hydrated of the current page are being hydrated, even though they are not yet in the viewport (so we could say that the lazy hydration is not preserved during the navigation).
@DAreRodz I'd love to have your input here. Is there any other option?
By the way, when we decide which method to use, we should add some end-to-end tests to confirm everything is working as intended before merging this, as these situations can be tricky to test manually.
Finally, I think it'd be good to introduce the ability to hydrate when the CPU is idle in this PR, so we can test everything together.
5 participants
This was a sub-PR off of #58227. See #58225.
What?
Instead of hydrating all nodes at DOMContentLoaded, further performance improvements can be gained by delaying initialization until the node enters the viewport. This was discussed in #52723 by @luisherranz:
Why?
Avoid running JS tasks needlessly during page load so that the browser is freed up to do other rendering tasks.
How?
Use an
IntersectionObserverto watch for an interactive node entering the viewport (scrolled from the bottom or the top) and hydrate it when it approaches. TherootMarginis set to hydrate the node when scrolling within one viewport of height away from the node.Additionally, when the InteractionObserver callback runs it now will check if
isInputPendingbefore proceeding to hydrate.Testing Instructions
See instructions from #58227.
Screenshots or screencast