-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spread props cause unnecessary changes #3289
Comments
Looking at that code again, I'm wondering if there's a good reason not to do this instead: if (changed.x || changed.a || changed.b) {
nested1.$set(get_spread_update(nested1_spread_levels, [
(changed.x) && ctx.x,
(changed.a) && { a: ctx.a },
(changed.b) && { b: ctx.b },
nested1_spread_levels[3],
nested1_spread_levels[4]
]));
} |
Hmm kinda makes sense that it changes, aren't you creating a new array ever time the component is called? I'm kinda thinking that if c should be const shouldn't it be declared in the script at the top as a const? In react pretty sure c would change on each render and it kinda makes sense |
No, this is a philosophical difference between React and Svelte. The React ideal is to recreate the universe from scratch on every state change (in practice, it falls short of that for performance reasons), whereas the Svelte ideal is to only ever update things that are known to have changed (in practice, it falls short of that for correctness/simplicity reasons). There's no reason to recreate |
I see what your saying at compile time you see there is no reason to change, what if some one spread a value that contains b as a value tho? |
@Rich-Harris Is it related with this bug REPL? |
Describe the bug
Inline expressions are re-evaluated when setting spread props on a component.
To Reproduce
https://svelte.dev/repl/38ab6eb9233a4d8c97e67d576e063109?version=3.6.8
Expected behavior
Waggling the slider should only cause
a
to show up as having changed. With the spread version,c
also appears to have changed.Severity
Mildly annoying. Could result in incorrect behaviour (as opposed to mere redundant work)
Additional context
This could be fixed like so:
The text was updated successfully, but these errors were encountered: