-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Data: deprecate the getIsResolving selector #59679
Conversation
Size Change: +64 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
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. |
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.
LGTM, thanks!
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.
Tests well and looks good. Good find! ✅
const resolutionState = getResolutionState( state, selectorName, args ); | ||
deprecated( 'wp.data.select( store ).getIsResolving', { | ||
since: '6.6', | ||
alternative: 'wp.data.select( store ).getResolutionState', |
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.
Should we also add a version: 6.8
or similar to the deprecation warning to suggest removing in a future WP version?
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.
Yes, it seems that getIsResolved
could indeed be removed in future. A wpdirectory.net search shows that the selector is not used by plugins -- all the search results that I checked are coming from bundled instances of @wordpress/data
itself.
4001883
to
eba3c20
Compare
eba3c20
to
e55182b
Compare
This PR is inspired in discussion in #59437 (comment)
The
getIsResolving
meta-selector should be deprecated. It returns a weirdundefined | boolean
value and it was introduced in #6600, six years ago, when the "raw" value in the store was of this type. At that time it made sense as the "raw getter". Nowadays the raw value is a more complex object returned bygetResolutionStatus
.Users should use the
isResolving
selector instead, which returns a cleantrue
/false
boolean.I'm migrating all Gutenberg usages of
getIsResolving
(there are not that many) toisResolving
.I'm also updating how the resolution status values are computed in
enrichSelectors
used byuseQuerySelect
anduseEntityRecords
. The existing code "reverse engineers" the status value from theisResolving
andhasResolved
values, but it can just straightforwardly calculate it fromgetResolutionStatus
, simply mapping strings to an enum.Last, the PR is adding
hasStarted
field to theuseQuerySelect
return value. Curiously, that value is declared in the type,QuerySelectResponse
, but not actually returned.