[FIX] owl-runtime: don't notify prop signals for alike props - #2004
Draft
Gxrvish wants to merge 2 commits into
Draft
[FIX] owl-runtime: don't notify prop signals for alike props#2004Gxrvish wants to merge 2 commits into
Gxrvish wants to merge 2 commits into
Conversation
Gxrvish
force-pushed
the
master-avoid-updating-arrow-functions
branch
2 times, most recently
from
July 25, 2026 19:44
1f546d6 to
c9be902
Compare
Contributor
|
hey @Gxrvish i noticed your PR, thanks! I am still thinking about it, not sure if the cost outweigh the value. The useProps function is a function that is called many times, and i want to be careful. but i did not forget this, still not sure, that's it! |
Props coming from useProps are signals, so reading one in an effect subscribes to it. But every prop signal was updated on each props update, including the props the template does not compare: `.bind`, `.alike` and inline arrow functions. Those props are a new function on every render even though they do the same thing, which is why Owl already ignores them when it decides to re-render a child. Reactivity did not. So as soon as the child was re-rendered for some other reason, the new function landed in the signal and every effect reading that prop ran again. The compiler now passes that list to createComponent, which stores it on the node, and useProps skips the update when none of the values captured by the arrow function changed. The argument is left out when there is nothing to pass, so templates compiled with an older compiler still work. Observable consequence: a `.bind` prop keeps the function it already has instead of picking up a newly bound one when the child re-renders for another reason, which is what `.alike` already meant for rendering.
Gxrvish
force-pushed
the
master-avoid-updating-arrow-functions
branch
from
July 30, 2026 18:32
e4a94c0 to
259947b
Compare
Contributor
|
Shouldn't the real fix be to use static props for those never changing props? I might be a little extreme but I'm thinking all props should always be static eventually. And if their value must change, the static prop should contain a signal/computed to change it through reactivity, not through props change. I guess the "optimization" might not hurt in the meantime. |
This file contains hidden or 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
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.
Props returned by useProps are signals, so reading one inside an effect subscribes to it. Until now, every prop signal was updated on each props update, even for the props the template does not compare by identity:
.bindand.alikeprops, and inline arrow functions.Those props are a new function object on every render, but they mean the same thing, so Owl already ignores them when it decides whether to re-render a child. Reactivity did not. As soon as the child was re-rendered for any other reason, its props object was replaced, the new function went into the signal, and every effect reading that prop ran again.
The compiler now passes the list of those props to createComponent, which stores it on the component node, and useProps skips the signal update when nothing the arrow function captured has changed. A prop signal now notifies in the same cases where Owl decides to re-render.
Note: a
.bindprop now keeps the function it already had instead of picking up a newly bound one when the child re-renders for another reason. This is what.alikealready means for rendering.The extra argument is left out when a component has no such prop, so templates compiled with an older compiler behave as before.
Fixes: #2003