-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Migrate PostTypeSupportCheck usages from withSelect to hooks #53330
Conversation
Size Change: -19 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
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, great cleanup, thanks @jsnajdr 🚀
} | ||
}; | ||
const value = orderInput === null ? order : orderInput; | ||
|
||
const value = orderInput ?? order; |
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.
👍
const { lastRevisionId, revisionsCount } = useSelect( ( select ) => { | ||
const { getCurrentPostLastRevisionId, getCurrentPostRevisionsCount } = | ||
select( editorStore ); | ||
return { | ||
lastRevisionId: getCurrentPostLastRevisionId(), | ||
revisionsCount: getCurrentPostRevisionsCount(), | ||
}; | ||
}, [] ); |
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 is duplicated between this component and the underlying PostLastRevisionCheck
component, could be a good opportunity to extract to a hook.
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.
That's true, and I think there are multiple opportunities like this. There's also the PageAttributesCheck
component, which duplicates code from the generic PostTypeSupportCheck
. It could easily be converted into a simple <PostTypeSupportCheck supportsKeys="page-attributes'>
instance.
But I didn't want to make too many diverse changes at once in this PR.
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.
Agreed on not making this PR too large. Definitely material for a follow-up if you want to pursuit it.
Inspired by @Mamaduka's #53235, this PR removes a few more usages of
withSelect
from components that usePostTypeSupportCheck
.withSelect
usages are converted touseSelect
hooks, updating also some unit tests that now needuseSelect
mockingPostTypeSupportCheck
. It accepts onlychildren
(in addition tosupportKey
), so we always usechildren
explicitly instead of generic...props
passthroughs.