| CARVIEW |
Select Language
HTTP/2 200
date: Fri, 10 Oct 2025 16:59:22 GMT
content-type: text/html; charset=utf-8
cache-control: no-cache
content-security-policy: default-src 'none'; base-uri 'self'; child-src github.githubassets.com github.com/assets-cdn/worker/ github.com/assets/ gist.github.com/assets-cdn/worker/; connect-src 'self' uploads.github.com www.githubstatus.com collector.github.com raw.githubusercontent.com api.github.com github-cloud.s3.amazonaws.com github-production-repository-file-5c1aeb.s3.amazonaws.com github-production-upload-manifest-file-7fdce7.s3.amazonaws.com github-production-user-asset-6210df.s3.amazonaws.com *.rel.tunnels.api.visualstudio.com wss://*.rel.tunnels.api.visualstudio.com github.githubassets.com objects-origin.githubusercontent.com copilot-proxy.githubusercontent.com proxy.individual.githubcopilot.com proxy.business.githubcopilot.com proxy.enterprise.githubcopilot.com *.actions.githubusercontent.com wss://*.actions.githubusercontent.com productionresultssa0.blob.core.windows.net/ productionresultssa1.blob.core.windows.net/ productionresultssa2.blob.core.windows.net/ productionresultssa3.blob.core.windows.net/ productionresultssa4.blob.core.windows.net/ productionresultssa5.blob.core.windows.net/ productionresultssa6.blob.core.windows.net/ productionresultssa7.blob.core.windows.net/ productionresultssa8.blob.core.windows.net/ productionresultssa9.blob.core.windows.net/ productionresultssa10.blob.core.windows.net/ productionresultssa11.blob.core.windows.net/ productionresultssa12.blob.core.windows.net/ productionresultssa13.blob.core.windows.net/ productionresultssa14.blob.core.windows.net/ productionresultssa15.blob.core.windows.net/ productionresultssa16.blob.core.windows.net/ productionresultssa17.blob.core.windows.net/ productionresultssa18.blob.core.windows.net/ productionresultssa19.blob.core.windows.net/ github-production-repository-image-32fea6.s3.amazonaws.com github-production-release-asset-2e65be.s3.amazonaws.com insights.github.com wss://alive.github.com wss://alive-staging.github.com api.githubcopilot.com api.individual.githubcopilot.com api.business.githubcopilot.com api.enterprise.githubcopilot.com; font-src github.githubassets.com; form-action 'self' github.com gist.github.com copilot-workspace.githubnext.com objects-origin.githubusercontent.com; frame-ancestors 'none'; frame-src viewscreen.githubusercontent.com notebooks.githubusercontent.com; img-src 'self' data: blob: github.githubassets.com media.githubusercontent.com camo.githubusercontent.com identicons.github.com avatars.githubusercontent.com private-avatars.githubusercontent.com github-cloud.s3.amazonaws.com objects.githubusercontent.com release-assets.githubusercontent.com secured-user-images.githubusercontent.com/ user-images.githubusercontent.com/ private-user-images.githubusercontent.com opengraph.githubassets.com marketplace-screenshots.githubusercontent.com/ copilotprodattachments.blob.core.windows.net/github-production-copilot-attachments/ github-production-user-asset-6210df.s3.amazonaws.com customer-stories-feed.github.com spotlights-feed.github.com objects-origin.githubusercontent.com *.githubusercontent.com; manifest-src 'self'; media-src github.com user-images.githubusercontent.com/ secured-user-images.githubusercontent.com/ private-user-images.githubusercontent.com github-production-user-asset-6210df.s3.amazonaws.com gist.github.com; script-src github.githubassets.com; style-src 'unsafe-inline' github.githubassets.com; upgrade-insecure-requests; worker-src github.githubassets.com github.com/assets-cdn/worker/ github.com/assets/ gist.github.com/assets-cdn/worker/
referrer-policy: no-referrer-when-downgrade
server-timing: pull_request_layout-fragment;desc="pull_request_layout fragment";dur=405.915387,conversation_content-fragment;desc="conversation_content fragment";dur=1901.432948,conversation_sidebar-fragment;desc="conversation_sidebar fragment";dur=333.599469,nginx;desc="NGINX";dur=1.297236,glb;desc="GLB";dur=134.768239
strict-transport-security: max-age=31536000; includeSubdomains; preload
vary: X-PJAX, X-PJAX-Container, Turbo-Visit, Turbo-Frame, X-Requested-With,Accept-Encoding, Accept, X-Requested-With
x-content-type-options: nosniff
x-frame-options: deny
x-voltron-version: 266d7a9
x-xss-protection: 0
server: github.com
content-encoding: gzip
accept-ranges: bytes
set-cookie: _gh_sess=hqbLfHuvqIoYCPQfHqoGx8yYCjJ%2Fbqb0shyB3LyW6emxnCZ%2F%2Fw%2FrIcubC8j6kmHpUC5wIs4oNylVal1YUeXSsYtiw6ujOBiwQIpQLR%2FK6oVMb%2Bdlgb489vRdlCijdUtBKo2oz%2FbrsKbZ4lvxcskcsNnXnHyvlSsuWlpGM6XAy5lkyj3DCCgbPnuk2uK5CsFYZmdjuT6%2BL0JaZBC8WOS%2BCE6dhbl0UZipkxS0h%2FCP3hRRNmoC38ZFp7NHDFG7FAlMvGe7jAOnM12VaI8ZavAJ9w%3D%3D--SxXqSfAv%2B%2BbbYC%2FJ--8BquKYxy77ZQFvTSNJK5lA%3D%3D; Path=/; HttpOnly; Secure; SameSite=Lax
set-cookie: _octo=GH1.1.1330785906.1760115560; Path=/; Domain=github.com; Expires=Sat, 10 Oct 2026 16:59:20 GMT; Secure; SameSite=Lax
set-cookie: logged_in=no; Path=/; Domain=github.com; Expires=Sat, 10 Oct 2026 16:59:20 GMT; HttpOnly; Secure; SameSite=Lax
x-github-request-id: DE58:FFCBC:F847B:12DB57:68E93B68
Core Data: Fix early return check for the record field-level resolutions by Mamaduka · Pull Request #71541 · WordPress/gutenberg · GitHub
Mamaduka
added
[Type] Bug
An existing feature does not function as intended
[Package] Core data
/packages/core-data
labels
Sep 8, 2025
Mamaduka
requested review from
nerrad,
ajitbohra,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
September 8, 2025 11:19
Mamaduka
force-pushed
the
fix/core-data-get-entity-record-field-resolution
branch
from
September 8, 2025 13:40
Mamaduka
deleted the
fix/core-data-get-entity-record-field-resolution
branch
September 12, 2025 10:50
Skip to content
Navigation Menu
{{ message }}
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Core Data: Fix early return check for the record field-level resolutions #71541
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Mamaduka
merged 5 commits into
trunk
from
fix/core-data-get-entity-record-field-resolution
Sep 12, 2025
Merged
Core Data: Fix early return check for the record field-level resolutions #71541
Mamaduka
merged 5 commits into
trunk
from
fix/core-data-get-entity-record-field-resolution
Sep 12, 2025
+346
−25
Conversation
This file contains hidden or 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
Mamaduka
added
[Type] Bug
|
Size Change: +73 B (0%) Total Size: 1.94 MB
ℹ️ View Unchanged
|
Mamaduka
commented
Sep 8, 2025
Comment on lines
115
to
126
| it( 'does not make a request if the record is already in store', async () => { | ||
| const select = { | ||
| hasEntityRecord: jest.fn( () => true ), | ||
| }; | ||
|
|
||
| await getEntityRecord( 'root', 'postType', 'post', { | ||
| context: 'edit', | ||
| _fields: 'id,title,slug', | ||
| } )( { dispatch, select, registry, resolveSelect } ); | ||
|
|
||
| // No request should have been made. | ||
| expect( triggerFetch ).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a better candidate for integration tests from #71474.
The current test gives us a false sense of safety; even if the hasEntityRecord behavior changes, the test will still pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced with integration tests in 31cdbbb.
Mamaduka
requested review from
nerrad,
ajitbohra,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
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. |
|
Flaky tests detected in 611c852. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/17548578611
|
Mamaduka
force-pushed
the
fix/core-data-get-entity-record-field-resolution
branch
from
611c852 to
1294c4a
Compare
This was referenced Sep 8, 2025
priethor
reviewed
Sep 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was looking forward to this 🚀
I've taken some time to properly understand it and test it, and it looks good to me. Probably the biggest advice is to document a bit more when to use hasEntityRecord vs hasEntityRecords and how their lookup logic differs, because with the field-level resolutions, it is not just "the same but for collections" as the name suggests.
| return !! queriedState.itemIsComplete[ context ]?.[ key ]; | ||
| } | ||
|
|
||
| const item = queriedState.items[ context ]?.[ key ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking out loud: if we are checking for an item with specific fields and they are all cached, is it relevant that the context is not the same if we have the data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data is stored by context, so looking it up by context is important. When there's no data for the same context, we'll have to make a request.
|
Thanks, @priethor! I'll try to document the differences for these selectors. I think Cc @aduth, I would appreciate your feedback here as well, if you've time. |
Does this mean it's still an issue if someone were to use those selectors with a query including |
aduth
approved these changes
Sep 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me 👍
I'd be curious if there's opportunities for more code reuse between query field resolution of the "records" vs. "record" flows, but perhaps it's appropriate that this be simpler in how it's evaluating existence of fields within a single item (vs. building up items matching in an array).
Comment on lines
+464
to
+474
| for ( let i = 0; i < fields.length; i++ ) { | ||
| const path = fields[ i ].split( '.' ); | ||
| let value = item; | ||
| for ( let p = 0; p < path.length; p++ ) { | ||
| const part = path[ p ]; | ||
| if ( ! value || ! Object.hasOwn( value, part ) ) { | ||
| return false; | ||
| } | ||
| value = value[ part ]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a micro-optimization, I was curious if for ... of would be better than iterating index and assigning a variable, but alas the simple for loop continues to prevail (as of Node v22.16.0)
for loop x 12,295,933 ops/sec ±0.46% (99 runs sampled)
for...of x 12,141,949 ops/sec ±0.59% (97 runs sampled)
Benchmarking code
import Benchmark from "benchmark";
const fields = ["a", "b.c", "d"];
const item = { a: 1, b: { c: 2 }, e: 3 };
const suite = new Benchmark.Suite();
suite.add("before", () => {
for (let i = 0; i < fields.length; i++) {
const path = fields[i].split(".");
let value = item;
for (let p = 0; p < path.length; p++) {
const part = path[p];
if (!value || !Object.hasOwn(value, part)) {
return false;
}
value = value[part];
}
}
});
suite.add("after", () => {
for (const field of fields) {
const path = field.split(".");
let value = item;
for (const part of path) {
if (!value || !Object.hasOwn(value, part)) {
return false;
}
value = value[part];
}
}
});
suite.on("cycle", (event) => {
console.log(String(event.target));
});
suite.run({ async: true });There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for checking. That's also my experience that simple for loops perform better for now.
I've not seen any related issues with The |
priethor
approved these changes
Sep 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we talked over Slack, my note about the docs should not be a blocker
Mamaduka
deleted the
fix/core-data-get-entity-record-field-resolution
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
[Package] Core data
/packages/core-data
[Type] Bug
An existing feature does not function as intended
3 participants
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.
You can’t perform that action at this time.
What?
Part of #26629.
PR fixes the early return check for the record field-level resolutions, inside the
getEntityRecord. This restores optimization logic and avoids additional HTTP requests when entity record data already exists in the store.Why?
It looks like the
hasEntityRecordsselector has regressed sometime ago, probably when we had to addincludeto thestableCacheKeyto avoid incorrect results.How?
PR introduces a new
hasEntityRecordselector, which is now used to perform checks.Testing Instructions
Test Snippets.
Testing Instructions for Keyboard
Same.