svelte icon indicating copy to clipboard operation
svelte copied to clipboard

Binding to an object fires erroneous reactive update

Open intelcentre opened this issue 5 years ago • 14 comments

Describe the bug When binding a complex value such as an object to a component <Component bind:value />, additional reactive updates are fired for the bound value even when no data is changed.

May be related to https://github.com/sveltejs/svelte/issues/4430 ?

To Reproduce Simply bind a complex value to a component property.

Example: https://svelte.dev/repl/5e14759de70d4d39b6f3833f91db4542?version=3.29.7

Expected behavior Reactive updates should be consistent between simple and complex types

Severity I find it irritating, but not a blocker. This issue is most likely to cause redundant calculations without formally breaking anything. This issue will likely snowball if components are nested, though I have not confirmed this. The additional updates are likely to confuse people causing unnecessary work.

Additional context Add any other context about the problem here.

intelcentre avatar Nov 18 '20 12:11 intelcentre

likely duplicate #4447

pushkine avatar Nov 18 '20 13:11 pushkine

similar problem reported here #5555, but this is much cleaner example.

xpuu avatar Nov 21 '20 04:11 xpuu

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jun 26 '21 22:06 stale[bot]

it is still actual!

gyurielf avatar Jan 18 '22 16:01 gyurielf

I ran into this as well.

Conditionally rendering the binding will refire the reactive statement.

Here is a reproduction: https://svelte.dev/repl/ed4f4a3d1d694e92a03cd08a3b5ce0a0?version=3.48.0

SarcevicAntonio avatar May 04 '22 07:05 SarcevicAntonio

I went around the issue and found Kevin Bridges awesome explanation about the entire reactivity. So every non primitives are counts as "dirty". That's why we ran into this.

gyurielf avatar May 04 '22 07:05 gyurielf

I run into this as well. Thanks @gyurielf for the link to Kevin's explanation, now it's clear why that happens.

What isn't clear to me yet, is if that is desired... Should Svelte work like that with objects or is it a bug? I don't see any use case that requires the object to be invalidated entirely as soon as one single property is updated. I wouldn't expect that.

For example: I want reload something when the user name is updated

<script lang="ts">
  export let user: {name: string, age: number};// ← comes from the main component through "bind:user"

  $: nameChanged(user.name);
  function nameChanged(newName) {
    reload(); // ← my logic
  }

</script>

The code needs the following changes to work as expected:

<script lang="ts">
  export let user: {name: string, age: number};
  
  let oldName: string; // ← variable to store the old value
  
  $: nameChanged(user.name);
  function nameChanged(newName) {
    if(oldName == undefined || oldName !== newName) { // ← check value changed
      oldName = newName; // ← update the old value
      reload(); // ← my logic
    }
  }

</script>

I think that is a strange workaround to make it all work.

kevinleto-informaticon avatar Jun 14 '22 09:06 kevinleto-informaticon

<script lang="ts">
  export let user: {name: string, age: number};// ← comes from the main component through "bind:user"

  $: nameChanged(user.name);
  function nameChanged(newName) {
    reload(); // ← my logic
  }

</script>

This will never work, even without this bug. nameChanged will be called at least once on initialization.

PatrickG avatar Jun 14 '22 10:06 PatrickG

<script lang="ts">
  export let user: {name: string, age: number};// ← comes from the main component through "bind:user"

  $: nameChanged(user.name);
  function nameChanged(newName) {
    reload(); // ← my logic
  }

</script>

This will never work, even without this bug. nameChanged will be called at least once on initialization.

I know. I've simplified the code to make it as simple as possible. A simple if(user) { ... } makes the trick. The focus was on the workaround 😄

kevinleto-informaticon avatar Jun 14 '22 10:06 kevinleto-informaticon

Should Svelte work like that with objects or is it a bug?

IMO, in an ideal world, Svelte would be as smart and lazy as possible, only doing the work needed if something (even object properties) actually changed.

I'm sure there is a good reason it's this way right now, but maybe that could change in the future?

SarcevicAntonio avatar Jun 14 '22 10:06 SarcevicAntonio

What isn't clear to me yet, is if that is desired... Should Svelte work like that with objects or is it a bug? I don't see any use case that requires the object to be invalidated entirely as soon as one single property is updated. I wouldn't expect that.

IMHO a reasonable decision for the deeply nested objects. It's too expensive to get around all of the deeply nested props. So I guess that's why.

gyurielf avatar Jun 14 '22 13:06 gyurielf

I am facing exactly the same unexpected behavior. I made a a very simple REPL to show the issue with the minimal setup.

Does anybody found a workaround to this ? (In my produsction case, this behavior leads to a double fetch...)

infuzz avatar Jul 30 '22 21:07 infuzz

You probably need to work with boolean variables like:

<script>
  let alreadyLoaded = false;
  if(!alreadyLoaded) {
    data = fetch(...);
    alreadyLoaded = true;
  }
</script>

That is the only workaround I've found till now.

kevinleto-informaticon avatar Aug 02 '22 06:08 kevinleto-informaticon

I went around the issue and found Kevin Bridges awesome explanation about the entire reactivity. So every non primitives are counts as "dirty". That's why we ran into this.

Svelte's reactivity is based on assignments. This issue is about the object being marked dirty for no reason, without any assignment. So I don't think this is relevant.

What isn't clear to me yet, is if that is desired... Should Svelte work like that with objects or is it a bug?

What you are suggesting is that changing an object property should not invalidate the entire object and that's a fair point to make when first encountering this behavior. But think about this: the object might have getters or methods that depend on multiple properties, it's impossible for Svelte to know. If you know, then look into the immutable option. Also what about obj[key]? Svelte cannot know what keys you use at runtime. By default the correct way is to invalidate the entire object. However, this is not related to this issue at all. This issue is about the object being invalidated for no reason at all.

I don't see any use case that requires the object to be invalidated entirely as soon as one single property is updated.

In addition to what I said above, there are countless of use-cases. Such as sending the object to a server, syncing it via BroadcastChannel or caching it in sessionStorage. All if which I do (abstracted via stores). For me this behavior is desired and absolutely vital. Usually you don't have to worry about it, because all the places the other properties of the object are used and passed around will eventually compare with a primitive and result in a NOOP (that's partially what "surgically updates the DOM" is about). But again, not related to this issue. Any discussion about it would probably be better of in a different place to not water down this bug.


Again, here's a minimal REPL that this issue is about: https://svelte.dev/repl/5f436e05809346a38baf906184321761?version=3.49.0 This will log obj twice. For absolutely no reason. In complex applications this causes a lot of undesired and expensive side-effects such as #6298

I re-iterate:

  1. Svelte's reactivity is based on assignment
  2. There is no assignment happening at all, the obj is not touched whatsoever
  3. Simply using bind with an object will cause the object to be invalidated for no reason
  4. This breaks assumptions about Svelte and is a bug causing performance issues and side-effects in real applications

Prinzhorn avatar Aug 05 '22 18:08 Prinzhorn

There is another related problem that I have. Reproduce here https://svelte.dev/repl/c6c7a555d8e24572ae00461bb989fee9?version=3.49.0

Basically I want:

  1. Init form state from some input data
  2. Re-computed the form state if the input data change
  3. Bind the form state to the element value

If there is a line of code that reassign the input data (line 6 in the repl), even if that line isn't executed, the form state still gets re-computed every time. Commented out that line, then it's work.

trungtin avatar Aug 16 '22 09:08 trungtin

In my use case, this is actually a pretty inconvenient problem. I have a big complex object that I subscribe to, and when it's updated I show an "Unsaved changes" indicator. Because of this issue, the "Unsaved changes" indicator can show up simply by mounting a component.

probablykasper avatar Oct 22 '22 00:10 probablykasper

Fixed by https://github.com/sveltejs/svelte/pull/7981

baseballyama avatar Dec 05 '22 14:12 baseballyama

This should be fixed in 3.54.0 - https://svelte.dev/repl/5e14759de70d4d39b6f3833f91db4542?version=3.54.0

Conduitry avatar Dec 06 '22 19:12 Conduitry

I reopen this issue and I removed the "bug" label. I described the reason at https://github.com/sveltejs/svelte/issues/6298#issuecomment-1368674996.

baseballyama avatar Jan 02 '23 06:01 baseballyama

I'm confused. Why re-open it then? Either it's a bug or not. Can you explain the purpose of the "conservative reactivity" label? Is this relevant for v4?

A reactive statement is always executed whenever there is a possibility of a change in the object's properties.

There is no possibility, because there is no assignment. I pass a prop down and nothing should come back up.

If I understand #8114 correctly the changes were reverted because you couldn't find a solution (that doesn't break other things) and not because you concluded it's not a bug.

I was luckily able to workaround #6298 by using clone + deep-equal, but that's not something that would work without stores. And you also first need to realize that something is going bad. In my case simply rendering a specific component would trigger dozens (depending on the number of items in the store) of network requests for no reason at all. And every change of a single item would again trigger a request for all items because the binding keeps invalidating everything for no reason.

Prinzhorn avatar Jan 02 '23 09:01 Prinzhorn

Is it an answer to your question? https://github.com/sveltejs/svelte/pull/8114#issuecomment-1368788459

Can you explain the purpose of the "conservative reactivity" label? Is this relevant for v4?

Of cause, such unnecessary processes should not be executed. But this is an intentional trade-off between performance and this issue. For example, if we use a Proxy, we can prevent executing such a process, but we need to pay for Proxy overhead. Our current consensus is that unnecessary event firing is better than such overhead because reactive things should have idempotence and we have some workaround for this.

"conservative reactivity" means the downside of this trade-off. In Svelte4, I'm not sure that we will improve this point.

baseballyama avatar Jan 02 '23 10:01 baseballyama

But this is an intentional trade-off between performance and this issue.

this is not a trade-off ====> this is insane... behaviour of component should be predictive, not random, I was not aware of this issue, and wasted many hours trying to understand why I have these extra extra components updates

I am using Svelte with Symfony + Inertia, and when props are passed to top level component, I already have double rendering, just by passing non primitive prop like object or array.

aniolekx avatar Mar 01 '23 15:03 aniolekx

This is so shame to know about this issue. and way we bind prop and also assign value to it as reactive prop is widely useful. but bcoz of this issue it can easily produce tons of bugs and performance issues. And in this many years, it still not solved. Hardly we convinced to use svelte, and such basic stuff issue causes lot of negativity in team. bcoz we need to use dirty workarounds like _oldblahblah and checks.

jaydeep987 avatar Mar 11 '23 15:03 jaydeep987

I would love a github reaction with the semantic "I also wasted a lot of time because of this unexpected behaviour, give it one more dev higher priority when rethinking reactivity". Meanwhile, I'll just leave this comment. Thanks

jsilveira avatar Apr 30 '23 15:04 jsilveira

@Rich-Harris, we just had another bad discovery in a project that we got from others and that we can't fix rapidly (we can't think of any work-arounds to this problem other than rewriting everything differently).

As @Prinzhorn well explained we are all a bit desperate at this point because we love Svelte a lot but when you start creating more complex projects you come across this issue and other issues that have been stuck for years now!

PLEASE, ONE QUESTION ONLY: Could you tell us where in the future roadmap is the resolution of this issue?

I'm not asking for an ETA, just where in the future program it is.

I'm trying to keep the client and my team from switching to Vue! :pray:


@Prinzhorn did you found an alternative way?

frederikhors avatar May 09 '23 23:05 frederikhors

I was wondering why is my reactive function firing so many times and just discovered this... :-(

gauel avatar Jul 24 '23 09:07 gauel

This will be fixed in Svelte 5, only one update happening now. Related to #4265

dummdidumm avatar Nov 15 '23 12:11 dummdidumm