useSelect: extract async queueing to separate hook #40093
Closed
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
|
Size Change: -141 B (0%) Total Size: 1.22 MB
ℹ️ View Unchanged
|
|
Could we add a "why" section or link to the issue that triggered this refactor? It'd be nice to know a bit more about the reason this is being done. |
Yes, just added a "why" section to the PR description 👍 |
This was referenced Apr 26, 2022
|
Superseded by |
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.
This is an attempt, arguably quite ambitious, to extract the "async update" logic out of
useSelectto a separateuseAsyncCallbackhook. The hook takes care of creating the render queue when needed (async mode is enabled, registry changes, ...), flushing it when needed, and returning a callback that's called either synchronously, or delayed and debounced withrequestIdleCallbackwhen async mode is on. Let's see what tests say.The one edge case I'm aware of not being solved is this: when the
mapSelectcallback changes due todepsgetting changed,useSelectwants to cancel all updates scheduled by the previous version. It does so by settingisMountedAndNotUnsubscribingtofalseand then callingrenderQueue.flush. TheonStoreChangecallback is a noop whenisMountedAndNotUnsubscribingisfalse, so the flush will call it with no effects. I yet need to find an elegant way to extract this part to the hook.Why?
There are two reasons to do this:
useSelectis a very complex hook that does many subtle things, all at once on 150 lines of code. Separating the various concerns into sub-hooks makes the code easier to reason about and to understand.useSelect, and currently theuseSuspenseSelecthook is a copy-paste ofuseSelectcode with few crucial places modified. Extracting the common logic would help avoid the code duplication.