-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Allow opt-in explicit dependency tracking for $effect
#9248
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
Comments
You can implement this in user land: function explicitEffect(fn, depsFn) {
$effect(() => {
depsFn();
untrack(fn);
});
}
// usage
explicitEffect(
() => {
// do stuff
},
() => [dep1, dep2, dep3]
); |
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 |
Good approach. Don't love the syntax, but I suppose it's workable. |
I think you want const { data } = $props();
const a = $derived(data.x.filter(...));
const b = $derived(data.y.toSorted()); |
Thanks @ottomated, that seems reasonable. The inverse of $track([dep1, dep2], () => {
// do stuff
}); But let's see what happens 🙂 |
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 $watch((a,b) => {
c = a + b
}); |
@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". |
It could cause issues/confusion when intentionally using recursive effects that assign one of the dependencies. A side note on the names: I am in favor of having explicit tracking, maybe even as the recommended/intended default; as of now the potential for errors with |
I'd opt for using a config object over an array if this were to be implemented. $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 |
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 |
The API shapes I'd propose: $effect.on(() => {
// do stuff...
}, dep1, dep2, dep3); Or $watch([dep1, dep2, dep3], () => {
// do stuff
}); I actually prefer With Plus, the word |
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 |
I'd be in favor of a 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 $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 |
@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 |
@opensas would that still track dependencies based on what's referenced in the callback? I think the difference with the |
no, in case dependencies are explicitly passed, references in callback should be ignored. you are telling the compiler "let me handle dependencies". |
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. |
I want to reiterate that it's really easy to create this yourself: #9248 (comment) |
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. |
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 <script>
let num = $state(0);
let timesChanged = $state(0);
$watch([num], incrementTimesChanged);
function incrementTimesChanged() {
timesChanged++
}
</script> I'm actually a fan of the Imagine a junior developer wrote 50 lines instead a |
Actually, the point is you can't do it the same way. Try it—that code will cause an infinite loop as it detects |
Wow, you're right. I really need to change my vision of reactivity with Svelte 5. This also means "Side-effects" are bad design in programming, and 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 There is another caveat with the <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 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 <script>
let num = $state(0);
$watch(num, () => {
if (shouldLog()) {
console.log({ num })
}
});
function shouldLog() {
return Math.random() > 0.3
}
</script> |
@Gin-Quin well I agree with almost everything you're saying. Especially the lack of transparency of when an
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
But I do fully support adding the feature to run an effect based on an explicitly defined list of dependencies. Since an |
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)
}
}) |
This is neither a bug, nor how
|
I understand, but it is not intuitive imo. let num = $state(0)
$effect(() => {
console.log('Hello')
if (false) {
console.log(num)
}
}) I would assume that the first |
For what it's worth, SolidJS 2.0 will likely separate dependency tracking from effect execution to support their work on async reactivity. |
See also #12908 |
Perhaps, since there's an let a = $state(0);
let b = $state(0);
$effect(()=>{
track(b); //or track(()=>b) for consistency with untrack
console.log(a);
}) It would only be for readability, and would do pretty much nothing under the hood. |
Just found this issue because I was looking for a good solution to exactly the case of marking something as unsaved/dirty that was already mentioned. In my case, I have an object with a deep structure. It's holding the entire state that the user is working with so it can be saved to a file through (using spreads like the other solution isn't straightforward. There are lots of sub-properties, with some of them being arrays that are passed to multiple components in The fact that I can get fine-grained reactivity with
Based on what @Gin-Quin said, I think instead of a I don't know the specifics of how For reference, this is my current code:
|
Inspect is deleted in production code so please don't use it for such things. |
this is very comfortable to use, if only it was rune like $effect.by() or $effect.on(). it makes reading effects very easy since seeing directly what its reacting to rather reading whole function body (especially when its complex). Also avoids infinite loops. |
Yes, we should make it safer to use because it’s a powerful tool. Saying we shouldn’t make it safer is like saying we shouldn’t use any tool that could hurt us—like avoiding going outside because we might get hit by a bus! |
|
derived values are not mutable which limits usage very much. This makes me to go back to effect but I think it should be better practice to control what its reacting to if body is somewhat complex. |
Based on the (slightly tweaked) example in the very first response by @dummdidumm, I exported the explicitEffect(
() => [dep1, dep2, ...],
() => {
// do stuff
},
); Does it have any potential performance issues? And should I be worried that it will stop working after some Svelte update? |
For me, it works best just to implement a small watch function: const watch = (...deps: unknown[]): void => {
deps.forEach(dep => {
if (typeof dep === "object" && dep !== null) {
// Deep access for objects/arrays
JSON.stringify(dep) // Lightweight way to touch all properties
} else {
dep?.toString()
}
})
} And then just call it with all dependencies in the effect rune: $effect(() => {
watch(obj.arr, foo, baz)
sideEffectFn()
}); |
@caboe in this example, wouldn't |
Damn i wasn't even aware of that. Just tested it with an example. Idk...im dont really like the fact that effects do this deep tracking. This means i would have to recursively follow all the function calls in an effect to make sure it doesn't trigger without intension. |
@Blackwidow-sudo that sounds like you are always still thinking in terms of procedural programming. All modern frontend frameworks work best with the mental model of declarative programming. And in fact, once you're able to think in that way, your code suddenly becomes way cleaner than it could ever be in the procedural style. So when you think of an Of course there can be exceptions to this rule, but this rule should work for like 95% of use-cases. |
@Evertt Youre right, i was not really thinking in a declarative way. But still, i think this could be made clearer in the docs. |
Yes, it would. This could lead to infinitive loops. But this could also happen under other condition like two effects. |
Here's type defined utility function for Vue-like effect, with specific dependency tracking and previous values: import { untrack } from "svelte";
export const effectBy = <T extends readonly unknown[]>(depsFn: () => [...T], fn: (prevDepValues: [...T] | null) => void) => {
let prevDepValues: [...T] | null = null;
$effect(() => {
const curDepValues = depsFn();
untrack(() => fn(prevDepValues));
prevDepValues = curDepValues;
});
}; Usage: effectBy (
() => [dep1, dep2, ...],
(prevDepValues) => {
// do stuff
},
); Hope someone will find it useful. 😉 |
@IcyFoxe Unfortunately, this will not work with, if you do not make an assignment:
Will not log out something, if you call addNum... |
Ah, you're absolutely right @caboe Anyone has a better idea? Edit: import { untrack } from "svelte";
export const effectBy = <T extends readonly unknown[]>(deps: () => [...T], fn: (prevDepValues: $state.Snapshot<[...T]> | undefined[]) => void) => {
let prevDepValues: $state.Snapshot<[...T]> | undefined[] = [];
$effect(() => {
untrack(() => fn(prevDepValues));
prevDepValues = $state.snapshot(deps());
});
}; It certainly works now, even with arrays, but again, can someone from the Svelte team inform us, whether this is a good approach performance-wise? Or does it have any unwanted side effects? |
A common — and arguably less esoteric — way of implementing your example in Svelte 3/4 is this pattern: let num = 0
let times_changed = 0
$: inc(num)
function inc () {
times_changed++
} Could a translation of this just be: let num = $state(0)
let times_changed = $derived.by(() => pre_inc(num))
function pre_inc () {
return times_changed + 1
} Or have I missed the point? EDIT: confirmed this only works where the side-effect is an evaluation that doesn't interrogate the value eventually assigned to. Sorry! |
I think the point of this issue has come across. Is there an rfc proposing this? |
No because this is an anti-feature #9248 (comment) |
I'm usually on your side Rich, but not with this one. I come from Vue, and explicitly specifying tracking dependencies has so many advantages over Svelte automatically deducing them.
Here one example how I use my // Store video player timestamp
effectByPre(
() => [file],
([fileOld]) => {
// File has updated, but the video player is still the previous one
if (fileOld && previewVideoElement) {
savedVideoTimes[fileOld.id] = previewVideoElement.currentTime;
}
},
); // Restore video player timestamp
effectBy(
() => [file],
() => {
if (file && previewVideoElement) {
previewVideoElement.currentTime = savedVideoTimes[file.id] || 0;
}
},
); Of course as with everything, maybe there's a better way to go about it, but to me this is already very readable and does what I want it to do. :) I like Svelte, because I could write Vue-like watch utility that behaves exactly as I want, and as long as it will work, I'm fine with Svelte not having a dedicated $watch rune. Anyways, I'll paste again my Typescript decorated utility functions, so that others can use, and maybe even improve them. import { untrack } from "svelte";
export const effectBy = <T extends readonly unknown[]>(deps: () => [...T], fn: (prevDepValues: $state.Snapshot<[...T]> | undefined[]) => void) => {
let prevDepValues: $state.Snapshot<[...T]> | undefined[] = [];
$effect(() => {
untrack(() => fn(prevDepValues));
prevDepValues = $state.snapshot(deps());
});
};
export const effectByPre = <T extends readonly unknown[]>(deps: () => [...T], fn: (prevDepValues: $state.Snapshot<[...T]> | undefined[]) => void) => {
let prevDepValues: $state.Snapshot<[...T]> | undefined[] = [];
$effect.pre(() => {
untrack(() => fn(prevDepValues));
prevDepValues = $state.snapshot(deps());
});
}; |
After discussion with other maintainers, we're closing this as
Again: effects are not about responding to state changing in a certain way, they are about to responding to state being a certain way. The place to respond to changes is in event handlers and subscription callbacks and function bindings. If you internalise this idea, I promise you will be able to write code that is more robust, more readable and more concise. But if you think you truly have a use for explicit dependencies — and I cannot emphasise enough that you probably don't! — then as #9248 (comment) shows you can trivially do this yourself:
If you need to react to changes to properties of those dependencies — and again, this is a bad idea and you really shouldn't do this! — then you can use explicitEffect(
() => {
// do stuff
},
- () => [dep1, dep2, dep3]
+ () => [$state.snapshot(dep1), $state.snapshot(dep2), $state.snapshot(dep3)]
); "But React has explicit dependencies"Yes — so that React knows about them, not so you can control them. A lesson that React users learn the hard way over and over again is that if you don't specify the right dependencies, effects become buggy. That's why React ships with a lint rule that yells at you if you specify dependencies incorrectly (where 'incorrectly' means 'the explicit dependencies don't perfectly match the implicit ones'). "But my effect is running too often"Let's say you have a contrived scenario like this, where both $effect(() => {
send(data, timestamp);
}); You might be tempted to make the dependency on watch(
() => send(data, timestamp),
() => [data]
); ...but this is flawed: it won't fire if only It also creates a maintenance burden and space for bugs. Imagine one day it becomes this, and it 'works' because send({ ...data, ...stuff }, timestamp); Unless you were careful enough to add Instead, you should $effect(() => {
send(data, untrack(() => timestamp));
}); (Realistically, in most cases "But my effect isn't running often enough"Another scenario is this, where $effect(() => {
if (condition) {
do_something_with(count);
}
}); This effect is just buggy: "But I can't work out why my effect is re-running"If you want explicit dependencies because you're just not sure why an effect is re-running, then first of all your effect is honestly probably too complex and you should try to simplify it. Avoid putting complex logic in effects! But in the cases where the complexity is unavoidable, it's far better to diagnose the issue than to try and avoid it. The "But Vue has
|
Uh oh!
There was an error while loading. Please reload this page.
Describe the problem
In Svelte 4, you can use this pattern to trigger side-effects:
However, this becomes unwieldy with runes:
Describe the proposed solution
Allow an opt-in manual tracking of dependencies:
(note: this could be a different rune, like
$effect.explicit
)Alternatives considered
but this compiles to
Importance
would make my life easier
The text was updated successfully, but these errors were encountered: