Delay executing state updates #4580
Draft
+188
−97
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
📊 Tachometer Benchmark ResultsSummaryduration
usedJSHeapSize
Resultscreate10kduration
usedJSHeapSize
filter-listduration
usedJSHeapSize
hydrate1kduration
usedJSHeapSize
many-updatesduration
usedJSHeapSize
replace1kduration
usedJSHeapSize
run-warmup-0
run-warmup-1
run-warmup-2
run-warmup-3
run-warmup-4
run-final
text-updateduration
usedJSHeapSize
tododuration
usedJSHeapSize
update10th1kduration
usedJSHeapSize
|
|
Size Change: -76 B (-0.12%) Total Size: 62.3 kB
ℹ️ View Unchanged
|
JoviDeCroock
force-pushed
the
delay-executing-state
branch
3 times, most recently
from
374f151 to
340bd1a
Compare
JoviDeCroock
force-pushed
the
delay-executing-state
branch
4 times, most recently
from
1b6b6e2 to
a8832fe
Compare
JoviDeCroock
commented
Dec 1, 2024
JoviDeCroock
force-pushed
the
delay-executing-state
branch
from
a4f6d02 to
7a72b6a
Compare
JoviDeCroock
force-pushed
the
delay-executing-state
branch
3 times, most recently
from
3525847 to
a2d6574
Compare
JoviDeCroock
commented
Dec 2, 2024
JoviDeCroock
force-pushed
the
delay-executing-state
branch
from
5597c1e to
17960ec
Compare
|
I think this could cause issues for signals, when we combine signals with hooks we won't be able to rerender if the hooks have executed but ended up at the same value. |
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.
Resolves #2913
This puts our implementation more in line with React which could resolve a few issues as well as reduce the byte size impact of our code. Another benefit this brings would be that we stop mutating the
Componentinstance and hence can't mess with other implementations like signals.What React does is when you call
reduceis that it enqueues an update but does not execute the action yet. It first renders the component so the outer scope to the state setter can be recaptured. This way it ensures that it deals with up-to-date props/...Essentially what this comes down to is that we store every reducer invocation and only execute it as the component is rendered. This way it has access to the outer component scope, .... This needs us to add a new
SKIP_CHILDRENbit-flag on theVNodeso we can bail out when the state settles to equal.For this to properly work we'd need to introduce
options._afterRenderwhich would check all of the hooks and setSKIP_CHILDRENwhen all hooks are equal.To get the last test to pass we'd need to find a way to deterministically see whether this component started the diff as we should not bail if the parent instructed the change.
I think this should generally be a performance improvement as well as we aren't touching the
Componentinstance anymore on every pass.