Site Editor: preload always required settings #61088
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ellatrix
added
[Type] Performance
|
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. |
ellatrix
commented
Apr 25, 2024
| const base = getUnstableBase(); | ||
| // This resource is preloaded. | ||
| const templates = getEntityRecords( 'postType', TEMPLATE_POST_TYPE, { | ||
| per_page: -1, | ||
| } ); |
There was a problem hiding this comment.
Note that this resource was already preloaded before this PR.
Mamaduka
reviewed
Apr 25, 2024
Comment on lines
34
to
35
| $paths[] = array( '/wp/v2/settings', 'OPTIONS' ); | ||
| $paths[] = '/wp/v2/settings'; |
There was a problem hiding this comment.
The /settings OPTIONS endpoint is already preloaded, but the middle removes an item from the cache after the first request. It works fine when core data makes requests because resolvers are cached, but you can get duplicate REST API calls when it's a pure apiFetch call.
See related conversation - #59243 (comment).
There was a problem hiding this comment.
Are we making random api-fetch API calls?
There was a problem hiding this comment.
Not random:
- First call to the
getSite()selector - The
loadSiteEntitymakes anOPTIONScall, fulfilled by the cache. - Then somewhere, code calls
canUser( 'edit', 'settings' ); this will hit the network.
I think it's a similar case for the /types GET path since it also uses dynamic entity loading.
There was a problem hiding this comment.
So you're saying that getSite and canUser trigger the same apiFetch call? That's weird. Can we use getSite with canUser maybe?
There was a problem hiding this comment.
The getOrLoadEntitiesConfig will trigger the OPTIONS request to load labels (used by getEntityRecord) for the site entity, and canUser also triggers OPTIONS requests. Only one of these requests is fulfilled by preloaded data.
There was a problem hiding this comment.
Hm, not in my testing, createPreloadingMiddleware receives a object with paths as keys.
There was a problem hiding this comment.
I meant the PHP side, where you can end up with duplicates, but there might be checks when actual preloading happens.
Screenshot
This is my local example showing duplicate /settings.
There was a problem hiding this comment.
Right but on the client side I don't think that means the data can be used twice
There was a problem hiding this comment.
I'm not seeing any duplicates in the site editor. Is that the post editor? I'm only changing the site editor here
array(16) {
[0]=>
array(2) {
[0]=>
string(12) "/wp/v2/media"
[1]=>
string(7) "OPTIONS"
}
[1]=>
string(25) "/wp/v2/types?context=view"
[2]=>
string(37) "/wp/v2/types/wp_template?context=edit"
[3]=>
string(42) "/wp/v2/types/wp_template-part?context=edit"
[4]=>
string(41) "/wp/v2/templates?context=edit&per_page=-1"
[5]=>
string(46) "/wp/v2/template-parts?context=edit&per_page=-1"
[6]=>
string(40) "/wp/v2/themes?context=edit&status=active"
[7]=>
string(39) "/wp/v2/global-styles/12464?context=edit"
[8]=>
string(26) "/wp/v2/global-styles/12464"
[9]=>
string(44) "/wp/v2/global-styles/themes/twentytwentyfour"
[10]=>
array(2) {
[0]=>
string(17) "/wp/v2/navigation"
[1]=>
string(7) "OPTIONS"
}
[11]=>
array(2) {
[0]=>
string(101) "/wp/v2/navigation?context=edit&per_page=100&order=desc&orderby=date&status[0]=publish&status[1]=draft"
[1]=>
string(3) "GET"
}
[12]=>
array(2) {
[0]=>
string(15) "/wp/v2/settings"
[1]=>
string(7) "OPTIONS"
}
[13]=>
string(15) "/wp/v2/settings"
[14]=>
array(2) {
[0]=>
string(16) "/wp/v2/templates"
[1]=>
string(7) "OPTIONS"
}
[15]=>
string(25) "/wp/v2/types?context=edit"
}
There was a problem hiding this comment.
Right but on the client side I don't think that means the data can be used twice
I think that makes sense.
I'm not seeing any duplicates in the site editor. Is that the post editor? I'm only changing the site editor here.
As I mentioned in the comment, it was a local made-up example showcasing that duplicates can exist in the $paths array. It was an example from the post editor.
youknowriad
reviewed
Apr 25, 2024
youknowriad
reviewed
Apr 25, 2024
|
Size Change: +658 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
ellatrix
commented
Apr 25, 2024
| homepageId: _homepageId, | ||
| postsPageId: _postsPageId, | ||
| url: base?.home, | ||
| url: siteData?.home, |
There was a problem hiding this comment.
We're in the site editor, which is only accessible to admins, so we can use the normal settings.
|
I'm sorry, @ellatrix. I think I got you sidetracked with the I think this PR can be merged after failing e2e test issues are resolved. I'll create a separate issue for |
|
I started working on the |
4 tasks
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Some resources block the editor from fetching the post, which increases the site editor load time. For example, knowing whether a different page is loaded for the index, and which page ID is required, is needed before fetching the post.
Additionally I added an
/templatesOPTIONS request which seems to be needed for a command. Here I'm wondering whether these commands can't be initialised whenever the command UI is shown though, instead of initialising all commands on page load.Why?
We have lots of requests for different interdependent resources, which slows down load.
How?
We can preload the always required data.
Testing Instructions
Check the network tab, ideally the network panel in a performance recording.
Testing Instructions for Keyboard
Screenshots or screencast