[apiFetch] Fix preloading middleware referencing stale data #25550
Conversation
jeyip
requested review from
youknowriad,
ellatrix,
noahtallen and
epiqueras
jeyip
changed the title
|
Size Change: +29 B (0%) Total Size: 1.17 MB
ℹ️ View Unchanged
|
The logic for preloading middleware in the apiFetch library caches preloaded data. As it stands today, all requests to the preloaded endpoint will reference the cache. The problem is that the cache is never invalidated during a session. When updates are persisted to the WordPress database, the cache doesn't change, and it will continue to reference the stale, preloaded data. This fix only allows references to cached data for each preloaded endpoint a single time, after which all subsequent requests to preloaded endpoints will skip preloading middleware.
jeyip
force-pushed
the
fix/api-fetch-preloading-stale-data
branch
from
0fd44f5 to
a570088
Compare
| cache[ path ] && | ||
| ! cache[ path ].hasBeenPreloaded | ||
| ) { | ||
| cache[ path ].hasBeenPreloaded = true; |
There was a problem hiding this comment.
maybe instead of adding a boolean we can just unset the cache key?
There was a problem hiding this comment.
That's a great point! Unsetting would probably work as well and be less verbose. 👍
There was a problem hiding this comment.
Seems logical, I wonder why we didn't hit this before :).
I wonder if it's dependent on the preloaded data and whether we need some kind of flag to say "number of uses", not sure it's needed though.
Did you check whether the number of API calls made when you load the post editor is the same before and after the PR? (with the document sidebar open)
Right?! This struck me as really odd. At least it's something we're catching now rather than later :P
That's a great question! I did a quick once-over, but not with the sidebar open. Do you mean the document / block settings sidebar? I'll be sure to take another look tomorrow. |
yes, and potentially with the "categories" and "tags" panel open as these do make some API request that might be cached as well. |
I can confirm that the number of requests is the same when I compare |
|
@youknowriad I unset the cache key instead of adding the boolean. I also double-checked the total number of requests on master and with this branch. It's still the same 👍 |
Description
Unblocks #25386
The logic for preloading middleware in the apiFetch library caches preloaded data. As it stands today, if an apiFetch request is made that matches a preloaded endpoint, it will reference the cache.
The problem is that the cache is never invalidated during a session. When updates are persisted to the WordPress database, the cache doesn't change, and requests to preloaded endpoints will continue to reference stale, preloaded data. This continues until a session has ended (browser is refreshed or closed).
Example of Unexpected Behavior:
getEntityRecords( 'postType', 'page' )selector. The selector resolution is invalidated because we've just persisted changes to the page title.getEntityRecords( 'postType', 'page' )will resolve an apiFetch request to/wp/v2/pages?context=edit, which is a preloaded path . This returns cached and stale preloaded data.Our solution is to only reference cached data a single time for each preloaded endpoint, after which subsequent requests skip preloading middleware.
Note:
I'm definitely going to lean on the expertise of others that have written code for the preloading middleware. This is my first time working extensively in the
apiFetchlibrary, and I'd appreciate any and all feedback 🙏How has this been tested?
I tested these fixes on #25386. I'm not certain if there is a better way to do this. If anyone has better suggestions, feel free to let me know.
try/add_page_name_to_document_settingsa. Execute
npx wp-env startin the terminalb. Execute
npm run devin the terminaladminand the passwordpassword.a. Click on Appearance < Themes in the sidebar and activate the seedlet blocks theme
b. Click on Gutenberg < Experiments in the sidebar and check the Full Site Editing Feature
a. Navigate to Settings > Reading > Your homepage displays
b. Select "A static page" and assign "sample page"
a. Click on the topbar label called "singular"
b. Type in a new page name
c. Note how the title of the page also changes in content of the block editor
d. Click on update design and then save
e. Note how the title of the page reverts to "sample page", but updates correctly upon page refresh
a. Open another terminal window and execute
git cherry-pick a570088(pulls in the bug fix from this branch)b. Revisit the site editor (referesh the page if you haven't already)
c. Click on the topbar label called "singular"
d. Type in a new page name
e. Note how the title of the page also changes in content of the block editor
f. Click on update design and then save
g. Note how the title of the page correctly reflects the newly updated value
h. Execute
git reset --hard HEAD~1to revert to original branch stateScreenshots
Types of changes
Bug fix that is blocking #25386
Checklist: