svelte icon indicating copy to clipboard operation
svelte copied to clipboard

Allow opt-in explicit dependency tracking for `$effect`

Open ottomated opened this issue 2 years ago • 64 comments
trafficstars

Describe the problem

In Svelte 4, you can use this pattern to trigger side-effects:

<script>
  let num = 0;
  let timesChanged = 0;

  $: num, timesChanged++;
</script>

<h1>Num: {num}</h1>
<h2>Times changed: {timesChanged}</h2>
<button on:click={() => (num++)}>Increment</button>

However, this becomes unwieldy with runes:

<script>
  import { untrack } from 'svelte';
  let num = $state(0);
  let timesChanged = $state(0);
	
  $effect(() => {
    // I don't like this syntax - it's unclear what it's doing
    num;
    // If you forget the untrack(), you get an infinite loop
    untrack(() => {
      timesChanged++;
    });
  });
</script>

<h1>Num: {num}</h1>
<h2>Times changed: {timesChanged}</h2>
<button on:click={() => (num++)}>Increment</button>

Describe the proposed solution

Allow an opt-in manual tracking of dependencies:

(note: this could be a different rune, like $effect.explicit)

<script>
  let num = $state(0);
  let timesChanged = $state(0);
	
  $effect(() => {
    timesChanged++;
  }, [num]);
</script>

<h1>Num: {num}</h1>
<h2>Times changed: {timesChanged}</h2>
<button on:click={() => (num++)}>Increment</button>

Alternatives considered

  • Don't migrate to runes
  • Use the clunky solution above
  • This can almost be implemented by the user:
export function explicitEffect(fn, deps) {
  $effect(() => {
    deps;
    untrack(fn);
  });
}

but this compiles to

explicitEffect(
  () => {
    $.increment(timesChanged);
  },
  [$.get(num)]
);

Importance

would make my life easier

ottomated avatar Sep 23 '23 01:09 ottomated

You can implement this in user land:

function explicitEffect(fn, depsFn) {
  $effect(() => {
    depsFn();
    untrack(fn);
  });
}

// usage
explicitEffect(
  () => {
     // do stuff 
  },
  () => [dep1, dep2, dep3]
);

dummdidumm avatar Sep 23 '23 07:09 dummdidumm

I use a similar pattern when e.g. a prop change, this would also benifit from being able to opt in on what to track instead of remembering to untrack everything.

export let data;
let a;
let b;

$: init(data);

function init(data) {
   a = data.x.filter(...);
   b = data.y.toSorted();
} 

I'm only interrested in calling init() when data change, not when a or b does, but a and b is used in the markup so they you still update the DOM. Furthermore with $effect() this would not work for ssr if I understand correctly?

pbvahlst avatar Sep 23 '23 13:09 pbvahlst

You can implement this in user land:

function explicitEffect(fn, depsFn) {
  $effect(() => {
    depsFn();
    untrack(fn);
  });
}

// usage
explicitEffect(
  () => {
     // do stuff 
  },
  () => [dep1, dep2, dep3]
);

Good approach. Don't love the syntax, but I suppose it's workable.

ottomated avatar Sep 23 '23 16:09 ottomated

@pbvahlst

I think you want

const { data } = $props();
const a = $derived(data.x.filter(...));
const b = $derived(data.y.toSorted());

ottomated avatar Sep 23 '23 16:09 ottomated

Thanks @ottomated, that seems reasonable. The inverse of $untrack() would still make sense though as you initially suggested. Maybe something like

$track([dep1, dep2], () => {
    // do stuff
});

But let's see what happens 🙂

pbvahlst avatar Sep 23 '23 17:09 pbvahlst

FWIW I currently make stuff update by passing in parameter into a reactive function call eg

<script>
let c = 0, a = 0, b = 0

function update(){
  c = a+b
}

S: update(a,b)
</sctipt>

Perhaps effect could take parameters on the things to watch, defaulting to all

$effect((a,b) => {
    c = a + b
});

That's got to be better that the 'untrack' example here https://svelte-5-preview.vercel.app/docs/functions#untrack

Also I think the $effect name is a very "computer sciencie", I'd personally call it watch, because that's what it feels like its doing. I think the below code says what it does.

$watch((a,b) => {
    c = a + b
});

crisward avatar Sep 24 '23 20:09 crisward

@crisward I personally love the syntax you came up with, but I'm sure there's gonna be a lot of complaints that "that's not how javascript actually / normally works".

Evertt avatar Sep 25 '23 15:09 Evertt

It could cause issues/confusion when intentionally using recursive effects that assign one of the dependencies. It would not make much sense to re-assign a function argument as that change would commonly be gone after the function scope.

A side note on the names: $effect keeps the association with side effects which is lost with $watch. This rune is coupled to the component life cycle and will only execute in the browser; "watch" to me sounds more general than it actually is.

I am in favor of having explicit tracking, maybe even as the recommended/intended default; as of now the potential for errors with $effect seems huge, given that it can trigger on "invisible" signals if any functions are called that contain them (related issue).

brunnerh avatar Sep 25 '23 16:09 brunnerh

How about adding a syntax like $effect.last, which only runs once at the end in a microtask, like the existing $ label syntax, and does not track changes caused by it? It seems like $effect may need some fine-grained behaviors, like $effect.pre, which behaves like beforeMount.

Artxe2 avatar Sep 29 '23 03:09 Artxe2

I'd opt for using a config object over an array if this were to be implemented. e.g.

 $effect(() => {
    timesChanged++;
  }, {
  	track: [num],
  });

More explicit, and makes it more future-proof to other config that might potentially be wanted. e.g. there could be a desire for an untrack option as well to explicitly opt out of specific dependencies that might be nested in the effect, or you could define a pre boolean instead of needing $effect.pre

Not-Jayden avatar Oct 02 '23 17:10 Not-Jayden

Perhaps effect could take parameters on the things to watch, defaulting to all

$effect((a,b) => {
    c = a + b
});

But that would mean that we cant pass callbacks like it was intended before.

function recalc() {
  c = a + b
}

$effect(recalc)

Im not really in favor of untrack but i dont see a better solution. Passing an array of things to track is also very annoying, isn't that what react does?

Blackwidow-sudo avatar Oct 11 '23 16:10 Blackwidow-sudo

The API shapes I'd propose:

$effect.on(() => {
    // do stuff...
}, dep1, dep2, dep3);

Or

$watch([dep1, dep2, dep3], () => {
    // do stuff
});

I actually prefer $watch, primarily because we'll also need to have a .pre variation for this, which would make it look a little weird: $effect, $effect.pre, $effect.on, $effect.on.pre.

With $watch, on the other hand, we'll have: $effect, $effect.pre <=> $watch, $watch.pre. Nice and symmetrical.

Plus, the word watch makes a lot of sense for this, I feel.

aradalvand avatar Nov 06 '23 12:11 aradalvand

I think something like this would be pretty useful. Usually when I needed to manually control which variables triggered reactivity it was easier for me to think about which variables to track, and not which ones to untrack. To avoid adding new runes (and new stuff to teach and learn) I guess it would be easier to let $effect track every referenced variable by default, but allowing to explicitly specify dependencies, either with an extra deps array or with a config object.

opensas avatar Feb 15 '24 20:02 opensas

I'd be in favor of a $watch rune like @aradalvand proposed, as I ran into a use case for it today. I have several inputs on a page (id, name, description, location) and some input groups (tags and contents) which are kept in an array of states. I have the following code to mark the save state as "dirty" when any of the inputs change:

let { id, name, description, location, tags, contents } = $state(data.container);
let saveState = $state<'saved' | 'saving' | 'dirty'>('saved');
$effect(() => {
	[id, name, description, location, ...tags, ...contents];
	untrack(() => {
		saveState = 'dirty';
	});
});

I'm using the array to reference all the dependencies. The tags and contents both need to be spread so that the effect triggers when the individual array items change. Then I need to wrap saveState = 'dirty' in untrack so that it doesn't cause an infinite loop. With a $watch rune, it could look like this:

$watch([id, name, description, location, ...tags, ...contents], () => {
	saveState = 'dirty';
})

I think it makes it more obvious why the array is being used, and it simplifies the callback function since untrack is no longer needed.

eddiemcconkie avatar Feb 23 '24 21:02 eddiemcconkie

@eddiemcconkie that's a great example of a good opportunity to explicitly define dependencies

we could also add an options parameter like this

$effect(() => saveState = 'dirty', { track: [id, name, description, location, ...tags, ...contents] })

which I think has the advantage of reflecting that it really does the same as a regular $effect with explicit dependecies

opensas avatar Feb 23 '24 23:02 opensas

@opensas would that still track dependencies based on what's referenced in the callback? I think the difference with the $watch proposal is that it wouldn't automatically track dependencies

eddiemcconkie avatar Feb 23 '24 23:02 eddiemcconkie

@opensas would that still track dependencies based on what's referenced in the callback? I think the difference with the $watch proposal is that it wouldn't automatically track dependencies

no, in case dependencies are explicitly passed, references in callback should be ignored. you are telling the compiler "let me handle dependencies".

opensas avatar Feb 24 '24 08:02 opensas

Just to add my 2 cents. I find an extra $watch rune less confusing in this case. I'd rather have two runes that serve a similar purpose, but use two very different ways of achieving that purpose. Than to have a single rune whose behavior you can drastically change when you add a second parameter to it.

Evertt avatar Feb 24 '24 11:02 Evertt

I want to reiterate that it's really easy to create this yourself: https://github.com/sveltejs/svelte/issues/9248#issuecomment-1732246774 Given that it's so easy I'm not convinced this warrants a new (sub) rune

dummdidumm avatar Feb 24 '24 11:02 dummdidumm

I want to reiterate that it's really easy to create this yourself: #9248 (comment) Given that it's so easy I'm not convinced this warrants a new (sub) rune

On the other hand, if every project starts to define similar helpers, possibly with different argument orders and names, then this will harm the readability of code for anyone not familiar with the helpers used in that specific project and will add mental overhead to switching between projects. I feel that having an effect with a fixed set of dependencies tracked is common enough that it really should be part of the core library.

FeldrinH avatar Mar 07 '24 18:03 FeldrinH

I want to reiterate that it's really easy to create this yourself: #9248 (comment) Given that it's so easy I'm not convinced this warrants a new (sub) rune

On the other hand, if every project starts to define similar helpers, possibly with different argument orders and names, then this will harm the readability of code for anyone not familiar with the helpers used in that specific project and will add mental overhead to switching between projects. I feel that having an effect with a fixed set of dependencies tracked is common enough that it really should be part of the core library.

I wholeheartedly agree with @FeldrinH, it's not just whether it's easy or difficult to achieve such outcome, but providing an idiomatic and standard way to perform something that is common enough that it's worth having it included in the official API, instead of expecting everyone to develop it's own very-similar-but-not-quite-identical solution.

Anyway, providing in the docs the example provided by @dummdidumm would really encourage every one to adopt the same solution.

opensas avatar Mar 07 '24 20:03 opensas

Actually I would do it like this in Svelte 4:

<script>
  let num = 0;
  let timesChanged = 0;

  $: incrementTimesChanged(num);

  function incrementTimesChanged() {
    timesChanged++
  }
</script>

Which you can do the same way in Svelte 5 (EDIT: you can't, see comment below):

<script>
  let num = $state(0);
  let timesChanged = $state(0);

  $effect(() => incrementTimesChanged(num));

  function incrementTimesChanged() {
     timesChanged++
  }
</script>

But I agree that a watch function / rune would be awesome and add much readability for this use case:

<script>
  let num = $state(0);
  let timesChanged = $state(0);

  $watch([num], incrementTimesChanged);

  function incrementTimesChanged() {
     timesChanged++
  }
</script>

I'm actually a fan of the $watch rune idea. In opposition to $effect, it would allow fine-grained reactivity. $effect can be messy because you have to read all the code inside the function to guess wha have triggered it.

Imagine a junior developer wrote 50 lines instead a $effect rune and it's your job to debug it...

Gin-Quin avatar Mar 14 '24 17:03 Gin-Quin

@Gin-Quin

Which you can do the same way in Svelte 5:

Actually, the point is you can't do it the same way. Try it—that code will cause an infinite loop as it detects timesChanged being accessed even inside the function. That's why I still think that a separate rune for this is useful, for clarity even if it can be implemented in userspace.

ottomated avatar Mar 14 '24 23:03 ottomated

Wow, you're right. I really need to change my vision of reactivity with Svelte 5.

This also means $effect is even more dangerous than I thought, and would re-run in many undesired cases.

"Side-effects" are bad design in programming, and $effect is promoting "reactivity side-effects" by its design.

The previous example is actually a very good minimal example of a reactivity side-effect:

<script>
  let num = $state(0);
  let otherNum = $state(0);

  $effect(() => logWithSideEffect(num));

  function logWithSideEffect(value) {
    console.log(value)
    console.log(otherNum) // side-effect, which triggers unwanted "side-effect reactivity"
  }
</script>

When reading the line with $effect, you expect it to run when num changes, but not when otherNum changes. This means you have to look at the code of every function called to check what triggers the whole effect function.

There is another caveat with the $effect rune. Let's say you want to log a value reactively, but only in some conditions:

<script>
  let num = $state(0);

  $effect(() => {
    if (shouldLog()) {  
      console.log({ num }) 
    } 
  });  

  function shouldLog() {
    return Math.random() > 0.3 // this is just an example
  }
</script>

You would expect the $effect callback to run every time numchanges, right?

But that's not what will happen. It will log at first, until it will randomly (once chance out of three) stop logging forever.

A $watch rune would solve it:

<script>
  let num = $state(0);

  $watch(num, () => {
    if (shouldLog()) {  
      console.log({ num }) 
    } 
  });  

  function shouldLog() {
    return Math.random() > 0.3
  }
</script>

Gin-Quin avatar Mar 15 '24 10:03 Gin-Quin

@Gin-Quin well I agree with almost everything you're saying. Especially the lack of transparency of when an $effect() would re-run could be a serious problem I think.

This also means $effect is even more dangerous than I thought, and would re-run in many undesired cases.

However, this is simply a bug that I believe could be fixed. I believe some other frameworks / libraries have fixed this already. Jotai for example, say this in their docs about their atomEffect:

  • Resistent To Infinite Loops: atomEffect does not rerun when it changes a value with set that it is watching.

But I do fully support adding the feature to run an effect based on an explicitly defined list of dependencies. Since an $effect() that watches an explicitly defined list of dependencies would need a significantly different implementation, I also think it makes sense to make it a different rune with a different name. And $watch makes a hell of a lot of sense to me.

Evertt avatar Mar 15 '24 13:03 Evertt

There is another caveat with the $effect rune. Let's say you want to log a value reactively, but only in some conditions:

<script>
  let num = $state(0);

  $effect(() => {
    if (shouldLog()) {  
      console.log({ num }) 
    } 
  });  

  function shouldLog() {
    return Math.random() > 0.3 // this is just an example
  }
</script>

You would expect the $effect callback to run every time numchanges, right?

But that's not what will happen. It will log at first, until it will randomly (once chance out of three) stop logging forever.

Oh wow, this is actually very weird behaviour:

let count = $state(0)

// Whole $effect callback wont run anymore
$effect(() => {
  console.log('Effect runs')

  if (false) {
    console.log(count)
  }
})

Blackwidow-sudo avatar Mar 15 '24 16:03 Blackwidow-sudo

This is neither a bug, nor how $effect is supposed to be used.

$effect tracks only the dependencies it actually needs; they can change with every execution of the effect. Having randomness like this in an effect is also not very representative of actual use cases.

brunnerh avatar Mar 15 '24 16:03 brunnerh

This is neither a bug, nor how $effect is supposed to be used.

$effect tracks only the dependencies it actually needs; they can change with every execution of the effect. Having randomness like this in an effect is also not very representative of actual use cases.

I understand, but it is not intuitive imo. When i write:

let num = $state(0)

$effect(() => {
  console.log('Hello')

  if (false) {
    console.log(num)
  }
})

I would assume that the first console.log would always get executed when num changes.

Blackwidow-sudo avatar Mar 15 '24 17:03 Blackwidow-sudo

Depends on your point of view, my expectation is that the dead-code elimination of the bundler leads to that bit of code never making it to production in the first place (example).

brunnerh avatar Mar 15 '24 17:03 brunnerh

Sure but with a condition that changes on runtime, we have no dead code and we would still have the issue that the effect stops running once the condition is false for one time.

Blackwidow-sudo avatar Mar 15 '24 17:03 Blackwidow-sudo

Normally the condition itself is based on another signal and therefor the effect will reevaluate the condition whenever that signal changes.

So in normal use-cases this should never lead to dead code.

Evertt avatar Mar 16 '24 10:03 Evertt