Core data: getEditedEntityRecord: do not return empty object #60988
Conversation
This file contains 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
|
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. |
ellatrix
added
[Package] Core data
ellatrix
commented
Apr 23, 2024
| @@ -846,10 +846,18 @@ export const getEditedEntityRecord = createSelector( | |||
| kind: string, | |||
| name: string, | |||
| recordId: EntityRecordKey | |||
| ): ET.Updatable< EntityRecord > | undefined => ( { | |||
There was a problem hiding this comment.
It's interesting how the type and docs already accounted for undefined.
|
Size Change: -954 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
ellatrix
commented
Apr 23, 2024
| } ), | ||
| ): ET.Updatable< EntityRecord > | undefined => { | ||
| const raw = getRawEntityRecord( state, kind, name, recordId ); | ||
| const edited = getEntityRecordEdits( state, kind, name, recordId ); |
There was a problem hiding this comment.
We could probably avoid the getEntityRecordEdits call if the raw entity is not defined yet?
|
Flaky tests detected in 4a4ce64. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8798096810
|
|
The proposal makes sense, but I'm afraid it will break any third-party consumer code that relied on previous behavior. |
|
@Mamaduka The documentation says it can be |
|
@ellatrix, true, but some of the updates in this PR highlight that sometimes people refer to returned values instead of the docs :( A couple more examples from a GitHub search:
P.S. Sorry for popping up in your PRs like a "ghost of Gutenberg's past" 😅 |
ellatrix
requested review from
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
@Mamaduka The new changes will fix that. |
Mamaduka
approved these changes
Apr 23, 2024
|
I think that the backward compatibility concerns are much smaller than it looks. First, the contract for Sure, we don't want to be legalistic, we just want to be nice and not break people's code. But the two practical examples that @Mamaduka shared also don't pass closer scrutiny:
I propose that we simply return By the way, there is also the |
|
I'd be ok with that, if we get some reports of breakage with the plugin, we could revert? |
jsnajdr
reviewed
Apr 25, 2024
| @@ -178,7 +178,7 @@ _Parameters_ | |||
|
|
|||
| _Returns_ | |||
|
|
|||
| - `undefined< EntityRecord > | undefined`: The entity record, merged with its edits. | |||
There was a problem hiding this comment.
This undefined< EntityRecord > type exposes a bug in the docgen tool. The type in the source is ET.Updatable<EntityRecord>, and is imported with import type * as ET from './types';. Apparently the tool is unable to follow the wildcard references.
After I change the import to be non-wildcard (import { Updatable, ... } from './types') then docgen can process it correctly.
|
@jsnajdr, there's always a private code that can't be found on GitHub or via wpdirectory.net. Maybe we could share an early dev note to give people time to update the code if needed. |
Yes, in this case I'd rather help plugins fix their code while the next WordPress/Gutenberg is still in alpha/beta. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
There's a bug in
getEditedEntityRecord: when neither its dependencies are defined, it's returning an empty object, which causes the editor to think there's an entity while there is not. This causes a bunch of code to the executed while there's no data, and wrong requests to be made (for example a template lookup forpagebecause there's no slug yet, while later another is made forpage-slug).Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast