Core Data: Avoid extraneous when creating a new record#72666
Conversation
| return; | ||
| } | ||
| const entityIdKey = entityConfig.key || DEFAULT_ENTITY_KEY; | ||
| const entityIdKey = entityConfig.key ?? DEFAULT_ENTITY_KEY; |
There was a problem hiding this comment.
I think it makes sense to update similar assignments to use the nullish coalescing operator. The entity either has a key defined or will be using the fallback.
I don't expect this to be a breaking change.
There was a problem hiding this comment.
@youknowriad, what do you think about this? Should I make similar changes in other places? Mostly for consistency.
| { | ||
| label: __( 'Base' ), | ||
| kind: 'root', | ||
| key: false, |
There was a problem hiding this comment.
The root/base is ready only, but adding this here just in case. Should I include an inline comment on why the key is intentionally false?
| label: __( 'Site' ), | ||
| name: 'site', | ||
| kind: 'root', | ||
| key: false, |
There was a problem hiding this comment.
Same here, should I add a comment?
|
Size Change: +26 B (0%) Total Size: 2.27 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 1352cf5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18781705070
|
| const entityIdKey = entityConfig.key || DEFAULT_ENTITY_KEY; | ||
| const entityIdKey = entityConfig.key ?? DEFAULT_ENTITY_KEY; | ||
| const recordId = record[ entityIdKey ]; | ||
| const isNewRecord = !! entityIdKey && ! recordId; |
There was a problem hiding this comment.
Why are we concerned about entityIdKey here?
There was a problem hiding this comment.
For the root/site, it will be intentionally false - #72666 (comment). It's more like a safeguard.
There was a problem hiding this comment.
It's a smallish breaking change I guess but I don't really expect impact, WDYT.
There was a problem hiding this comment.
I don't expect this to break any consumer code.
|
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. |
|
I think it's okay, since it affects performance 👍 |
Co-authored-by: Mamaduka <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: priethor <[email protected]> Co-authored-by: adamsilverstein <[email protected]>
|
I just cherry-picked this PR to the wp/6.9 branch to get it included in the next release: 6db6a7b |
What?
Closes #71927.
🚧 Work in Progress 🚧
PR prevents extraneous HTTP requests for entity collections when creating a new entity record.
Why?
When creating a new record,
select.getRawEntityRecordis called withoutid, which results in an unwanted collections query.How?
entityConfig.keytofalsefor entities that represent a single object. For example, the Site settings.saveEntityRecordis updating an existing record or creating a new one, and callgetRawEntityRecordconditionally.Testing Instructions
GETrequestTesting Instructions for Keyboard
Same.
Screenshots or screencast