-
Notifications
You must be signed in to change notification settings - Fork 286
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
Optimize useInViewSelect
#8789
Labels
Comments
aaemnnosttv
added
P1
Medium priority
Type: Enhancement
Improvement of an existing feature
labels
Jun 3, 2024
IB ✔️ |
binnieshah
added
Team S
Issues for Squad 1
and removed
Next Up
Issues to prioritize for definition
labels
Jun 24, 2024
5 tasks
QA Update ✅
|
18 tasks
I've opened a very small follow up here to address one detail missed which resulted in a larger change here. |
Update: The follow-up PR has been merged. |
Thanks @nfmohit! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Feature Description
In #4096 we added
useInViewSelect
, a decorated version of theuseSelect
hook we use liberally throughout our components.There are a few details which were missed in the initial implementation which should be revisited to avoid unnecessary performance hits.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
mapSelect
should be removed from the dependency array ofmapSelectCallback
. This function is expected to be defined inline (i.e. new every render) so including it undoes the optimization provided byuseCallback
anduseSelect
internally (see next point)site-kit-wp/assets/js/hooks/useInViewSelect.js
Line 44 in 0ea003e
mapSelectCallback
should be extracted to a stable definition outside of the hook (as originally defined). This avoids calling it unnecessarily as this function will be new on every render, thus will always be called here https://github.com/WordPress/gutenberg/blob/3e867dd7c6560e4b2cb9a59cc02856e055be6142/packages/data/src/components/use-select/index.js#L93C8-L93C23Implementation Brief
mapSelectCallback
's dependency array to use the provideddeps
. The necessaryeslint-disable-next-line
is explained in the latest version ofuseSelect
which we could copy here as well() => undefined
function to a stable named function outside of the componentTest Coverage
mapSelect
function across renders in and out of viewQA Brief
Changelog entry
useInViewSelect
hook to use dependencies.The text was updated successfully, but these errors were encountered: