Compose: Introduce an in-house debounce() utility, deprecate Lodash version
#43943
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
tyxla
added
[Type] Performance
tyxla
requested review from
sgomes,
aristath,
gziolo,
swissspidy,
geriux,
jorgefilipecosta,
chad1008,
fluiddot,
ntsekouras and
fabiankaegy
tyxla
requested review from
ajitbohra,
noisysocks and
ellatrix
as code owners
|
Size Change: +375 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
sgomes
approved these changes
Sep 7, 2022
There was a problem hiding this comment.
LGTM, other than the small comment issue below 👍 Love to see this moved to a @wordpress lib 🙂 I'll leave it to more seasoned Gutenberg developers to comment on whether this is the right package for it, though.
Thank you for going to the trouble of assigning me co-authorship of the first commit, @tyxla! That was very kind of you 🙂
Thanks, that has been addressed in 737875a
I believe it should be, because
Of course! It only makes sense :) |
|
I've fixed a couple more issues - a missing internal dependency one and one where we weren't passing a wait time to a Looking forward to a few more reviews 🙌 |
|
I'm planning to land this one this week - is anyone willing to give it another look? |
gziolo
reviewed
Sep 13, 2022
gziolo
reviewed
Sep 15, 2022
tyxla
force-pushed
the
add/debounce-helper
branch
from
965a6bf
to
bb48334
Compare
|
Thanks for the feedback y'all! Added a changelog entry in bb48334 as well. Ready for another last look 👀 |
|
All CI checks pass. There is a set of unit tests included. I don't see any negative impact on the performance metrics reported in https://github.com/WordPress/gutenberg/actions/runs/3059825067/jobs/4937649466. I don't know the internals of |
gziolo
approved these changes
Sep 15, 2022
Co-authored-by: Sérgio Gomes <[email protected]>
tyxla
force-pushed
the
add/debounce-helper
branch
from
bb48334
to
d0984f4
Compare
tyxla
added
[Type] New API
This was referenced Sep 15, 2022
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?
This PR introduces an in-house version of the
_.debounce()function, along with some tests. I've borrowed most of the prior art by @sgomes (kudos for that), as indicated by his co-authorship of the first commit. We also update alldebounceusages in Gutenberg to use the new version. Finally, we deprecate the original_.debounce()function so it will never be seen again in the repo.Why?
Lodash is known to unnecessarily inflate the bundle size of packages, and in most cases, it can be replaced with native language functionality. See these for more information and rationale:
@wordpress/api-fetchpackage haslodashas a dependency #39495How?
We're introducing
debounceas another utility function in the@wordpress/composepackage. That nicely gives us the opportunity to type it, which we're essentially doing. It comes with a few tests as well. We're exposing it externally, since it will be used by other packages.Furthermore, we update all
_.debounce()calls to use the new utility function, including theuseDebounce()hook. IMHO, that closes the loop of having an in-house hook foruseDebounce()but using an external function for the actual debouncing, giving us control on the types and functionality.You might ask why we're adding all the custom features (the
optionsthird argument) when they're not used internally. Well, becauseuseDebounce()is exposed externally, and it was supporting all those options, so we have to continue supporting them for backwards compatibility.Testing Instructions
debounce()that it continues to work well, for example: