Repository navigation
Conversation
getTemplateId derived its template lookup slug from getEditedEntityRecord, which includes unsaved edits. Typing in the slug field therefore produced a new lookup slug on every keystroke, and since each distinct query is its own resolver cache key, each one fired a GET /wp/v2/templates/lookup request. Typing "hello" sent five requests, four of them for slugs that never existed. A slug only selects a template once it is saved, so read it from getRawEntityRecord instead. The explicitly assigned template still comes from the edited record, so choosing a template in the UI updates the canvas right away.
|
Important Review skippedAuto reviews are disabled on this repository. To trigger a review, include ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
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. |
🤖 PR meta 🤖📦 Bundle sizeSize Change: +15 B (0%) Total Size: 8 MB 📦 View Changed
⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
|
getRawEntityRecord returns undefined for a record that has never been saved, not an object with an empty slug. The mock returned the latter, so the test passed even without the optional chaining that guards it.
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation is covered by relevant tests, with no unresolved issues.
Pull request overview
Updates Core Data template resolution to use the saved slug, preventing redundant lookup requests while editing.
Changes:
- Resolves templates from persisted entity data.
- Adds coverage for saved, edited, and unsaved slugs.
- Documents the fix.
File summaries
| File | Description |
|---|---|
packages/core-data/src/test/private-selectors.jsdom.test.js |
Tests saved, edited, and unsaved slug behavior. |
packages/core-data/src/private-selectors.ts |
Uses the saved slug for template lookup. |
packages/core-data/CHANGELOG.md |
Records the template lookup fix. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Mamaduka
left a comment
Mamaduka
left a comment
There was a problem hiding this comment.
Thanks for the quick follow-up, @ramonjd!
The fix makes sense ✅. However, I'm not sure how useful these new unit tests are. cc @youknowriad
Dropped the test asserting the lookup slug for a saved post: it passes on trunk too, so it only checked that string concatenation still concatenates. The two that remain each fail when their defect is reintroduced. One catches reading the slug from the edited record, the other the crash when a record has never been saved.
Might have gone overboard. I think I can get rid of the first one at least and a bit of the scaffolding. Maybe I can just keep the regression guard. |
The mock scaffolding stubbed every dependency of the selector, leaving only string concatenation under test, and reached into private-apis and the registry selector's internals to do it. Neither test could assert the thing that matters, which is that typing sends no lookup requests.
Let's just rid of them actually. Happy to reinstate, but I don't think they add value. Thanks for the nudge. |
|
I feel like this is not correct though, if you're editing the slug, you want the template that applies to your post now, which means the edited slug. |
Querying every keystroke isn't ideal either; maybe we can find a good middle ground? |
|
If live preview while typing (rather than resolving from the saved slug here) is required, then I guess we go debounce or how about resolve I think the post and site editors use different slug UIs, so I think we should wait until they're united. I think it's this experiment? #76244 I can close this PR and create an issue to record the problem. I don't have much bandwidth to go deeper on this right now. |
It was late, I was confused 😄 I double checked this claim this morning. So I think a fix could be in
Then when DataForm is swapped in we revisit. |
|
Thanks, @ramonjd! Let's keep this open for now; maybe we'll have a "bingo" moment and solve it without affecting the wider area. I think |
What
Template resolution no longer runs while you type in the slug field.
Noticed by @Mamaduka in #78135
Before
Kapture.2026-09-07.at.10.55.24.mp4
After
Kapture.2026-09-07.at.11.02.30.mp4
Why
getTemplateIdbuilt its lookup slug fromgetEditedEntityRecord, which includes unsaved edits. Every keystroke produced a new lookup slug. Each distinct query is its own resolver cache key, so each one fired aGET /wp/v2/templates/lookuprequest.Typing
hellosent five requests. Four asked about half-typed slugs the user was still in the middle of writing.The endpoint is a fallback resolver, so each of those returned the same template the post already had. The requests were wasted, not wrong.
How
Read the slug from
getRawEntityRecordinstead. A slug only selects a template once saved.The assigned template still comes from the edited record, so picking a template in the UI updates the canvas immediately.
Testing instructions
Open DevTools, Network tab, filter on
templates/lookup.?slug=page-h,?slug=page-he). With this branch: none.Also check:
pageorsingle-posttemplate.