Editor: Avoid unnecessary term re-fetches in FlatTermSelector - #80623
Conversation
| // Sort ids so reordering alone doesn't produce a new query key and re-fetch. | ||
| include: _termIds?.length | ||
| ? [ ..._termIds ].sort( ( a, b ) => a - b ).join( ',' ) | ||
| : undefined, |
There was a problem hiding this comment.
The getEditedPostAttribute can return term IDs in a different order after the post is saved.
|
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. |
|
Size Change: +22 B (0%) Total Size: 7.75 MB 📦 View Changed
|
tyxla
left a comment
There was a problem hiding this comment.
LGTM, nice optimizations there!
Just a few minor questions before we 🚢
| include: _termIds?.length | ||
| ? [ ..._termIds ].sort( ( a, b ) => a - b ).join( ',' ) | ||
| : undefined, |
There was a problem hiding this comment.
Should we maybe not provide include to the query object at all if it's undefined? I've seen include: '' before, so which one is the right thing to do?
There was a problem hiding this comment.
This matches previous types. Query isn't performed when there are no itemIds.
| // Optimistically update term values. | ||
| // The selector will always re-fetch terms later. | ||
| setValues( uniqueTerms ); | ||
| setSearch( '' ); |
There was a problem hiding this comment.
Can we confirm that this won't have timing problems with the debounced search?
There was a problem hiding this comment.
Can't really think of any practical cases. In theory, maybe 🤷 😅
|
Thanks for the review, @tyxla! |
What?
PR makes two small optimizations to the flat term selector's
getEntityRecordsqueries:setSearch('')), so a stale search value doesn't trigger an extra search request after selecting/creating a term.Note: There's one more unwanted query for stale
termIds, but it requires too much bookkeeping to fix.Testing Instructions
Testing Instructions for Keyboard
Same.
Screenshots or screencast
Use of AI Tools
None.