Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Svelte 5: When spread object passed to components changes all the output of $props is updated instead of only the necessary stuff #11286

Open
HighFunctioningSociopathSH opened this issue Apr 22, 2024 · 5 comments · May be fixed by #11290
Open

Svelte 5: When spread object passed to components changes all the output of $props is updated instead of only the necessary stuff #11286

HighFunctioningSociopathSH opened this issue Apr 22, 2024 · 5 comments · May be fixed by #11290
Assignees
@trueadm

Comments

@HighFunctioningSociopathSH
Copy link

Describe the bug

When {...restProps} passed to a component changes, all the other props also elicit a reaction even though nothing is changed.
This false signal causes components to almost completely render again which is a waste of resources and can also cause bugs.

Reproduction

lets say we have the following component
// Test.svelte

<svelte:options runes />

<script lang="ts">
  let { about, ...restProps }: { about?: string; someObject?: Record<string, any> } = $props();

  $effect(() => {
    console.log(about); // This runs again when restProps is changed.
  });
</script>

And in our +page we render it like below

<script lang="ts">
  import Test from "$components/Test/Test.svelte";
  import { onMount } from "svelte";

  onMount(() => {
    setTimeout(() => {
      restProps = {};
    }, 3000);
  });

  let restProps = $state({});
</script>


<Test about="something" {...restProps}></Test>

Now changing restProps will cause the $effect that is looking at the primitive value of about to run again and this happens for all props.

If our restProps is the output of a $derived then it will be a new object every time that derived runs again which means any reactive code inside Test runs again.

One bug that this can cause is for example if you have a functionality that is based on changes to a prop then that functionality will run again even though the user didn't change that prop at all. for instance, In the following example from the documentation that uses the writable derived method which is supposed to update the value of facade whenever the incoming prop value changes but also allow for changes to facade from within the component, The false signal causes the getter to go off even though the value prop was not changed by the user which results in facade to have the wrong value.

// Test.svelte
<script lang="ts">
  let { value, ...restProps }: { value: string; someObject?: Record<string, any> } = $props();

  let facade = {
    get value() {
      return value.toUpperCase();
    },
    set value(val) {
      value = val.toLowerCase();
    }
  };
</script>

<div>
  Test input
  <input bind:value={facade.value} />
</div>

// +page.svelte

<script lang="ts">
  import Test from "$components/Test/Test.svelte";
  import { onMount } from "svelte";

  onMount(() => {
    setTimeout(() => {
      restProps = {};
    }, 5000);
  });

  let restProps = $state({});

  let value = $state("hello");
</script>

<Test {value} {...restProps}></Test>

<div>
  +page input
  <input bind:value />
</div>

Here try to change the value of Test input then wait 5 seconds to witness that even without typing in the +page input the value of Test input changes to +page input's value because restProps was changed

REPL

Logs

No response

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (16) x64 12th Gen Intel(R) Core(TM) i7-12650H
    Memory: 5.77 GB / 15.63 GB
  Binaries:
    Node: 18.14.2 - C:\Program Files\nodejs\node.EXE
    npm: 9.7.1 - C:\Program Files\nodejs\npm.CMD
    bun: 1.1.3 - ~\.bun\bin\bun.EXE
  Browsers:
    Edge: Chromium (123.0.2420.97)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    svelte: ^5.0.0-next.110 => 5.0.0-next.110

Severity

annoyance
@paoloricciuti
Copy link
Contributor

Interestingly the order of the props matters: repl

@paoloricciuti paoloricciuti linked a pull request Apr 22, 2024 that will close this issue
Open
5 tasks
@trueadm trueadm self-assigned this Apr 28, 2024
@HighFunctioningSociopathSH
Copy link
Author

@trueadm Here's another example of a problem I think has to do with the same thing and it's weird.
Lets say we have the following component.
Test.svelte

<script lang="ts">
  import { onMount } from "svelte";

  let { open = $bindable(false), about, somethingElse }: { open?: boolean; about?: string; somethingElse?: number } = $props();

  // $inspect(open);
  $effect(() => {
    console.log(open);
  });

  onMount(() => {
    setTimeout(() => {
      open = true;
    }, 3000);
  });
</script>

And this is our +page where I'm simulating the spreading of 2 different objects based on a condition.

<script lang="ts">
  import Test from "$components/Test/Test.svelte";
  import { onMount } from "svelte";

  let boolVar = $state(false);
  onMount(() => {
    setTimeout(() => {
      boolVar = true;
    }, 5000);
  });
</script>

<Test {...boolVar ? {} : { about: "hello", somethingElse: 2 }}></Test>

Now after 3 seconds, we are simulating a change from inside Test that sets open to true, then wait for the setTimeout inside page to change the boolVar variable. You will notice that the variable open which was not even in the spreaded object, retakes its default value, meaning it changes back to false.
Now if you change the $effect with an inspect then something else happens. Even though the reactivity is triggered and $inspect runs again, this time the value of open remains true.

@paoloricciuti
Copy link
Contributor

Just to clarify the behaviour: the reason effects rerun when you reassign a spread is because they need to check if the new shape of the object is overriding a prop. In my PR I limited this by listening to a derived of the keys of the object so that it's only rerunning when the keys change.

However it would still need to rerun in both cases you proposed.

@HighFunctioningSociopathSH
Copy link
Author

but aren't props like open or about primitives? shouldn't it run when the value changes? there is no mention of the parent object inside the $effect.
still, it shouldn't cause open to take its default value again.

@paoloricciuti
Copy link
Contributor

but aren't props like open or about primitives? shouldn't it run when the value changes? there is no mention of the parent object inside the $effect.

What I mean is this:

<Child open={true} {...rest} />
<button onclick={()=> rest = { open: false }}>change</button>

So if inside the effect you access open you have an implicit dependency on the object because it the object changes open could change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@trueadm trueadm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: spread as a dependency for every prop paoloricciuti/svelte
3 participants
@trueadm @paoloricciuti @HighFunctioningSociopathSH