Stabilize useEntityRecord and useEntityRecords #40162
Merged
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
adamziel
added
the
Developer Experience
adamziel
requested review from
draganescu,
talldan,
tellthemachines,
kevin940726,
getdave,
noisysocks,
ajitbohra and
nerrad
as code owners
|
Size Change: +31 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
gziolo
added
the
Needs Dev Note
dmsnell
reviewed
Apr 8, 2022
| * @template RecordType | ||
| */ | ||
| export default function __experimentalUseEntityRecord< RecordType >( | ||
| export default function useEntityRecord< RecordType >( |
There was a problem hiding this comment.
should we not leave the __experimentalUseEntityRecord in place for a bit but add a deprecation() to it so people get some warning to switch over if they are using it?
There was a problem hiding this comment.
Good question, but isn't the purpose of __experimental to not require deprecation?
There was a problem hiding this comment.
Is that? I know part of the intention is to lead people to avoid using those but I think the reality is that many do.
It's not my call and I don't really know our polity (maybe a good update to the docs is in store) but personally I think we can avoid a lot of harm and loss of trust if we do give a due period of deprecation for the core APIs. If after six months we remove something that's been deprecated then at least we gave notice and opportunity to update code. Things move slow even when things move fast.
There was a problem hiding this comment.
The idea with the __experimental APIs is to not have people using them outside of the Gutenberg repository until they stabilize:
An experimental or unstable API is named as such to communicate instability of a function whose interface is not yet finalized. Aside from references within the code, these APIs should neither be documented nor mentioned in any CHANGELOG. They should effectively be considered to not exist from an external perspective. In most cases, they should only be exposed to satisfy requirements between packages maintained in this repository.
There's a related discussion going on in #40316
Technically it wouldn't be too hard to add an additional __experimental export that calls the underlying function with a deprecation notice, but it is not how other __experimental APIs are handled right now. I'd move forward here as is, and then have the deprecation policy discussion separately.
There was a problem hiding this comment.
I guess it won't harm to add the deprecation notice for the old method name so it continues to work if someone uses it and remove it with WordPress 6.1 or 6.2.
…ds hook is disabled
adamziel
force-pushed
the
stabilize/use-entity-record
branch
from
ec99caf
to
dd71452
Compare
|
I addressed all the feedback. I think we're good now @gziolo @dmsnell @ZebulanStanphill @draganescu |
gziolo
reviewed
May 5, 2022
adamziel
force-pushed
the
stabilize/use-entity-record
branch
from
6b7449d
to
2f6b479
Compare
gziolo
reviewed
May 5, 2022
|
One thing made me wonder. Why those 2 new methods aren't included in any documentation? Does it mean, Let's included it also in https://github.com/WordPress/gutenberg/blob/trunk/packages/core-data/CHANGELOG.md in the "New Features" section. |
Closed
|
Dev note proposed at https://make.wordpress.org/core/?p=99829&preview=1&_ppp=2b1db975ea |
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.
Description
Part of #38135, related to #40162
Stabilizes the
__experimentalUseEntityRecordand__experimentalUseEntityRecordshooks. They have been used in other core packages without any problems.This Pull Request does the following:
enum Statustoconst enum Statusto reduce the footprint of generated code at no additional cost, as @dmsnell proposed in Propose useEntityRecords (experimental) #38782Test plan
Confirm all the checks are green – no runtime logic is expected to change as a result of this PR aside of the EMPTY_ARRAY adjustment.