-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Interactivity API: Defer hydration until node is scrolled near the viewport #58284
base: trunk
Are you sure you want to change the base?
Conversation
Co-authored-by: Luis Herranz <luisherranz@gmail.com>
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) ...
packages/interactivity/src/init.js
Outdated
@@ -30,17 +30,47 @@ function yieldToMain() { | |||
|
|||
// Initialize the router with the initial DOM. | |||
export const init = async () => { | |||
const pendingNodes = new Set(); | |||
|
|||
const intersectionObserver = new window.IntersectionObserver( |
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.
If there are no nodes
below, then this wouldn't need to be constructed in the first place.
packages/interactivity/src/init.js
Outdated
if ( pendingNodes.size === 0 ) { | ||
intersectionObserver.disconnect(); | ||
} |
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.
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 cfe2b04. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7792764477
|
@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? |
@luisherranz Sorry for the delay.
Forcing hydration right before navigation could potentially cause a long task at the worst possible point as the user expects a navigation to happen right away, yeah? |
@luisherranz Actually, this doesn't seem to be an issue because the gutenberg/packages/interactivity/src/init.ts Lines 48 to 54 in f763e73
When client-side navigation is enabled, I found that the only |
I has having a hard time testing this because I was getting what seem to be unrelated errors which I've filed in #63880. |
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. |
…ry/interactivity-lazy-hydration
@felixarntz I just re-tested and I cannot reproduce the issue anymore. So yes, I think this is unblocked! |
…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? |
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
IntersectionObserver
to watch for an interactive node entering the viewport (scrolled from the bottom or the top) and hydrate it when it approaches. TherootMargin
is 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
isInputPending
before proceeding to hydrate.Testing Instructions
See instructions from #58227.
Screenshots or screencast