Inserter: Try deferring search value instead of debouncing #58058
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
Mamaduka
added
[Feature] Inserter
|
Size Change: +13 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
I am getting mixed reports on CI; locally results are the same, or deferred search performance is slightly better. I would appreciate another set of tests as a control. |
|
What do you expect from this change? Input debouncing is usually used to prevent doing expensive search (REST requests) on each keystroke. If you type reasonably quickly, there won't be multiple requests like |
|
It is just an experiment to see if we can avoid a fixed delay. I think the only HTTP request the inserter is making is for the block directory. Using deferred value worked out well in #56139, and I wanted to see if it is a viable replacement for the inserter. |
|
I'm not sure I agree with this change when it's about limiting network requests. FWIW, we intentionally wanted to reduce the number of requests here, and if you read further in the docs, it says:
IMHO, we don't want this as it's not guaranteed to minimize how often it triggers, and we intentionally want to limit the number of requests per given time limit, as on a faster device it can theoretically still issue too many (unnecessary) network requests. You can think of this experience as searching in settings on an iPhone, where it intentionally adds a small delay before starting to search and it feels very natural, not to mention the performance improvement caused by the fewer rerenders and the reduced server load that's a direct consequence of fewer performed requests to the API. |
|
Makes sense. I just monitored network requests while searching - it's a lot; using debouncing makes more sense. I appreciate your feedback on my experimental PR 🙇 |
|
I played with this and it works well 🙂 There are two searches that are triggered by typing in the search field:
I can't type fast enough to prevent the installed blocks happening on every keystroke. But apparently my computer is fast enough for the browser to always have enough idle time to do the search, so it's OK. And it turns out that the REST requests for block directory have their own 400ms debouncing! So, even with this patch applied, the search happens only once, after I'm finished typing my search query. |
Oh, what specifically were you testing and what were the requests? I had a different, much better experience in my testing. |
|
Using the TT4 theme, try typing |
Oh yes, I forgot about patterns. And was looking only at REST requests (fetch/XHR), so I missed all the requests that the preview iframes are making. |
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?
PR tries to replace
useDebouncedInputwithuseDeferredValuefor global inserter search.Why?
Testing Instructions
imageorparaTesting Instructions for Keyboard
Same