Svelte 5: $effect somewhat broken between v247 and v248

[brunnerh]

Describe the bug

A dependency seems to not be tracked correctly in certain scenarios.
The example uses a $derived.by to destructure props from a stateful object updated via $effect.

Reproduction

<script>
	import { useData } from './data.svelte.js';

	const { data, next } = $derived.by(useData);
</script>

<button onclick={() => next()}>
	Next
</button>
 
<pre>{JSON.stringify(data, null, 2)}</pre>

// data.svelte.js
export function useData() {
  let data = $state(null);
  let i = $state(1);

  $effect(() => {
    fetchData(i);

    async function fetchData(index) {
      const res = await fetch(`https://dummyjson.com/posts/${index}`);
      const json = await res.json();

      data = json;
    }
  });

  return {
    get data() {
      return data;
    },
    next() { i++; },
  }
}

REPL

The data updates initially, but changing the index does not re-trigger past v247.

Logs

No response

System Info

REPL

Severity

annoyance

[paoloricciuti]

So it works in 247 but it's broken in 248?

[brunnerh]

Correct

[paoloricciuti]

So the likely culprit is #13274 ...unless is something funky with #13269 (i had some problem when i've implemented #13250 because somewhere we are using arguments.length and passing an argument broke that...but i'm pretty sure is #13274)

[paoloricciuti]

Yup, reverting #13274 locally fixes the repl...my guess is that the effect in the derived.by gets the wrong parent because it's lazily evaluated.

But i think @trueadm will fix this easily so i will not try to fix it 😄

EDIT: forcing the read in the script by loggin data "fixes" the issue so i guess i'm kinda right repl

[on1force]

literally wanna ask this on my issue, but having second thoughts because I haven't test it yet @brunnerh

[trueadm]

This is a complicated one. It’s technically working correctly now, whilst incorrect before. The derived is lazy, so where you access it matters, especially if it creates an effect. So using it inside the template will associate the inner effect with whatever effect the derived was used in. However, this isn’t ideal in this use case. I need to think about this more

[paoloricciuti]

This is a complicated one. It’s technically working correctly now, whilst incorrect before. The derived is lazy, so where you access it matters, especially if it creates an effect. So using it inside the template will associate the inner effect with whatever effect the derived was used in. However, this isn’t ideal in this use case. I need to think about this more

I was thinking: deriveds likely needs to be attached to the component effect, so what if we store the effect to the derived itself on the declaration and we set the active effect when we call it? Could this work?

[trueadm]

This is a complicated one. It’s technically working correctly now, whilst incorrect before. The derived is lazy, so where you access it matters, especially if it creates an effect. So using it inside the template will associate the inner effect with whatever effect the derived was used in. However, this isn’t ideal in this use case. I need to think about this more

I was thinking: deriveds likely needs to be attached to the component effect, so what if we store the effect to the derived itself on the declaration and we set the active effect when we call it? Could this work?

They used to be, but that's actually wrong. If you read a derived inside another $effect.pre or template effect then if those effects update, the nested effect in the derieved needs to be destroyed. Ideally, if you have an effect inside a derieved, then the derived needs to eager rather than lazy. However, it's not possible to really determine this at runtime or compile time because the derived with the effect might be read inside another derived.

[trueadm]

Actually I think there might be another issue at play here, digging into it now.

[Rich-Harris]

I was thinking: deriveds likely needs to be attached to the component effect, so what if we store the effect to the derived itself on the declaration and we set the active effect when we call it? Could this work?

Missed this comment when I was noodling on this problem earlier, but yes, this is the exact right solution