Improve Nav block loading UX by preloading Navigation menu requests #48683
Conversation
|
Preloading requests are dispatched here |
getdave
changed the title
getdave
added
[Type] Performance
Outstanding questions
|
|
This is a tricky problem since server-side preloading can affect TTFB. Preloading also will affect every editor, even if that page/post or template doesn't use the Navigation block. There was an interesting conversation on a similar subject in this core PR WordPress/wordpress-develop#3894 (review). So maybe folks from the performance team could help us with the metrics.
The first comes from the Site Editor's Navigation sidebar and the second from the Navigation block (that request is only used for fallback).
Not on the server side since we need to know the ID(s) used in the editor. |
Yes this is what I was concerned about. You articulated it much better than me though! My arguments would probably be that
Once Nav blocks move to using |
|
Related #48924 |
| * @param array $preload_paths Preload paths to be filtered. | ||
| * @return array | ||
| */ | ||
| function gutenberg_preload_navigation_posts( $preload_paths, $context ) { |
There was a problem hiding this comment.
This implies that regardless of the theme and current setup of the site, the site will always have a navigation menu. I think that's a false assertion - maybe the theme doesn't provide a navigation by default, or maybe the user has currently removed it from their site, or used a different block to set a navigation menu up. That way we'll always preload the wp_navigation posts, regardless of whether they're necessary.
Just imagine if the previous theme supported those and the site has a ton of those navigation posts, but the current theme doesn't make use of them. That will preload all those posts in vain.
Could be a longshot, but I was wondering if we could add some sort of a check to ensure that these navigation posts are actually going to be necessary?
There was a problem hiding this comment.
This implies that regardless of the theme and current setup of the site, the site will always have a navigation menu. I think that's a false assertion...
That's certainly an argument that can be made. On the other hand you're probably talking about an 20% use case, with the other 80% of sites being almost certain to be using some kind of Nav menu.
That way we'll always preload the wp_navigation posts, regardless of whether they're necessary.
My question is how bad is it exactly for those 20% users?
Just imagine if the previous theme supported those and the site has a ton of those navigation posts, but the current theme doesn't make use of them. That will preload all those posts in vain.
This is certainly valid but very much an edge case IMHO. How does this weight against the majority of users who will experience a poor initial editor experience due to interdependent, network requests running in sequence and blocking the initial "load" state of the block (and thus the editor as a whole)?
Could be a longshot, but I was wondering if we could add some sort of a check to ensure that these navigation posts are actually going to be necessary?
Yes ideally we would do that. But I cannot see a way to do that without making more queries which is then self defeating.
There was a problem hiding this comment.
Thank you for the explanation. That makes sense to me 👍 It's a compromise and it seems like it's better to have the optimization than to not have it. Would be nice to see some measurements and statistics that confirm the 80% and 20% assumptions.
There was a problem hiding this comment.
Yes it would be useful. Obviously I'm generalising heavily from intuition only 😅
I wonder how we could determine stats on how many Themes include a Nav block. Maybe @MaggieCabrera or @scruffian would have an idea?
There was a problem hiding this comment.
It definitely would be hard to say 😉 Especially given that one could install a plugin for their menu (like one of those megamenu plugins) and not use the Nav block at all.
There was a problem hiding this comment.
Worth noting that it's possible to disable preloading by filtering on block_editor_rest_api_preload_paths and removing all requests that contain wp/v2/navigation. So it's not something folks are locked into. But a non-developer user would find this difficult.
There was a problem hiding this comment.
most sites will have a Nav block.
I think this is a safe assumption, also keep in mind that fresh installs have tt3 on them, which has the block on the theme.
|
In my opinion the time to first byte is much more of a concern on the frontend of the site, where it impacts all users and has SEO implications, than in the editor where only the site owner is impacted. The increased speed of preloading navigations is vastly better than very slight slow down in TTFB, and given how many sites use navigation this seems a good compromise. Having said that, if we can find a way to only do this if a site uses a navigation menu that would be even better. I also notice that we do the same thing with the list of pages, so if my navigation block contains a page list (and many do) then I still get a loading state while the list of pages preloads. It might be worth considering also preloading the list of pages, in a different PR of course :) |
|
Does the code highlight some kind of problem in the navigation block? We need to preload published and draft menus and then preload only published menus. We should have published menus from the previous call, right? I agree with @scruffian that TTFB in admin seems less of a problem when it improves the experience so much, but I wonder if the block itself is not really trying too much with so many requests? I wonder, since the navigation is such an essential feature, could these data be preloaded clientside in the site editor? In WordPress/wordpress-develop#3894 (review) the result was to change the approach and warm up the client before the user opens the view that needs the data. Since we're not using menu data in the sidebar, that could be a potential route too? To summarize:
|
|
Thank you for your feedback @draganescu.
Yes but I see it as a tangential issue. We need to look separately at making the initial Nav block render require a single "all menus" request only.
Yes we should be exploring how the client side network requests can avoid being run in sequence and avoid be dependent on each other. However, again I see this as an aside. Even if we make all the requests run in parallel then we still need a network request to resolve for one of the most important pieces of data to making the editor feel "ready" more quickly. (As you know) I think as developers we are often guilty of assuming everyone is on a Fibre connection when in fact that isn't the case for the majority. As a result I think the key issue is minimising the need for additional network requests for "essential" data (from the Gutenberg SPA) after the initial request to the server for the I think it's super easy to hope that new client side tech will solve these problems (and it will help) but sometimes it's just down to ensuring the key data is available immediately.
We will be using that data very shortly in Browse Mode so I don't think that's a viable route. IMHO It's skirting around the actual issue. |
@scruffian Yes that would be ideal. But you can't know that without doing more queries so its self defeating. |
|
I created a PR to remove the extra request for drafts: #49143 |
|
We could/should revisit this in context of changes in #48698 We could now preload the fallback request which would mean it would return very quickly and the resolver would immediately push the resulting embedded entity into state. Only issue is that this request would be preloaded even in situations where a fallback is not required (i.e. all the Nav blocks already have I think Nav loading is still a big enough pain point that this is worth revisiting, especially now we can almost guarantee that every single site will have a Navigation menu as a result of #48698 |
getdave
force-pushed
the
try/preload-navigation-requests
branch
from
b72b923 to
c28dcb2
Compare
|
Noting that we could also preload the fallback requests by: $preload_paths[] = array(
add_query_arg(
array(
'_embed' => true,
),
'wp-block-editor/v1/navigation-fallback'
),
'GET',
);...and then changing the related resolver to use export const getNavigationFallbackId =
() =>
async ( { dispatch } ) => {
const fallback = await apiFetch( {
path: addQueryArgs( '/wp-block-editor/v1/navigation-fallback', {
_embed: 1,
} ),
} );
/// |
There was a problem hiding this comment.
I know this preloading technique does not come for free but:
- the number of websites with no menu is infinitely smaller than the ones with menus
- the preloading happens only in the site editor, not in all block editors
- the preloading theoretially slows down only admin sections
- the navigation REST endpoints for menus and fallback are now needed in two places
- the navigation block
- the navigation screen of the site editor sidebar
- all navigation requires multiple calls because our API is RESTful so we do OPTIONS and then call various resources in turn, we don't just smash everything into one endpoint
- successive network requests are terrible to fix in UI in a proper way
- if the navigation block is bearable to load async and show spinners and placeholders, as other items in the canvas do this, the same is not true for the navigation screen of the site editor canvas which is "chrome" UI which should not blink in and out of existence (nobody expects the taskbar or menu bar or home screen icons to load async on their devices)
For these reasons I think we should not only merge this PR but also add the fallback endpoint as @getdave described above.
The performance effects can always be mititgated on a case by case basis by:
- filtering out what the REST API does
- using good cache setups
|
Ok we have a general consensus here. I think we should merge and follow up with work to preload the fallbacks. |
|
@draganescu the follow up to preload fallbacks is here #50545 |
What?
Preloads Navigation entities in the Site Editor.
Why?
We have lots of requests going out async for Navigation entities. This causes the block (and resultingly the editor) to feel very sluggish as these requests can take a while to resolve on slower connections.
The waterfall is:
to find a fallbackfor other things).These also have
OPTIONSrequests which take time to resolve. By preloading these requests we improve the perceived performance in the editor as the requests are not dispatched.How?
Preload the requests that cause network waterfalls for Navigation posts.
We can't preload the individual Nav block, but we can preload a list of all the Nav blocks and OPTIONS requests as well.
In the future, once we reference Navigation posts by
slugthen it will be possible to preload the Nav with theheaderslug which will allow us to have the primary Navigation menu available in editor fetch cache on editor being loaded.Testing Instructions
On trunk
http://localhost:8888/wp-admin/edit.php?post_type=wp_navigationOn this branch
Do the same as the above but this time notice that once the template part is loaded there is almost no (or a reduced) loading state for the Nav block at all. The menu is almost instantly available.
Observe the network tab (filtered by "navigation") and notice that only a single requests is dispatched for
OPTIONSfor the single Navigation by ID. This is actually a request to get permissions on that Navigation.Testing Instructions for Keyboard
Screenshots or screencast
Editor loading UX
Before
Notice how once the template part resolves the Navigation block shows its own loading state for quite a while before resolving to the menu:
Screen.Capture.on.2023-03-08.at.09-58-12.mp4
After
Notice how once the template part resolves the menu is almost instantly available in the Navigation block:
Screen.Capture.on.2023-03-08.at.09-56-57.mp4
Network timings
Before
Notice the request for the single Navigation menu is not dispatched until the requests for all Navigation Menus are resolved.
After
Now only a single OPTIONS request for the single Navigation Menu.