sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.22k stars 4.09k forks source link

Runes: $effect in *.svelte.js files is a footgun #9265

Closed WaltzingPenguin closed 2 weeks ago

WaltzingPenguin commented 11 months ago

Describe the problem

$effect looks like $dervied, $state, and store.subscribe but has all the pitfalls of get_context. There are very tight restrictions about when and where one can call functions containing $effect.

Examples of things that look like they should work: Object instantiation Singleton patterns

The first at least errors at runtime. That second one is much worse: it will provide no errors or warning, it just will work incorrectly.

Describe the proposed solution

If $effect remains tightly coupled to the component life cycle, it should look tightly coupled to the component.

Alternatives considered

Importance

would make my life easier

jsilveira commented 11 months ago

The Object instantiation example seems like a great point!

However, I'm having trouble seeing how the Singleton example is wrong. The createObject is effectively called only once, which is the singleton logic I see in the code. Were you expecting that if you enter the same value twice it would print only once? because I don't see how that code would be supposed to do that...

WaltzingPenguin commented 11 months ago

However, I'm having trouble seeing how the Singleton example is wrong.

$effect is tied to the life cycle of the component where the singleton was originally created. Anything using that pattern will be called across multiple different components, presumably with different life cycles.

I should have included an example that would fail though, so this should demonstrate it clearer

jsilveira commented 11 months ago

Ohh, I see the problem now with that example. Thanks

aradalvand commented 10 months ago

This is in part why I strongly prefer $effect and $effect.mount as the naming; where $effect would be what $effect.pre currently represents, and $effect.mount what $effect currently is. I voiced this in the Discord server once.

$effect.mount would explicitly communicate that the effect is tied to the component lifecycle, whereas $effect would be a "pure" effect (i.e. just a function that re-runs whenever the signals that are accessed within it are updated — not tied to the concept of a component or anything like that).

Update: Apparently I had misunderstood what $effect.pre really means. The truth is disappointing.

levibassey commented 10 months ago

This is in part why I prefer $effect and $effect.mount as the naming; where $effect would be what $effect.pre currently represents, and $effect.mount what $effect currently is. I voiced this in the Discord server once.

$effect.mount would explicitly communicate that the effect is tied to the component lifecycle, whereas $effect would be a "pure" effect.

When I encounter the mount keyword, my initial assumption is that the associated function will execute once after the component is created and has been successfully mounted to the DOM. This behaviour aligns with established patterns in other frameworks, including Svelte 4. Consequently, using $effect.mount for code that executes whenever its dependencies change seems somewhat counter-intuitive from a semantic perspective.

aradalvand commented 10 months ago

When I encounter the mount keyword, my initial assumption is that the associated function will execute once

When you encounter the mount keyword alone, yes, but the prefix effect should make that clear, I think.

Rich-Harris commented 5 months ago

It's not about .svelte vs .svelte.js files — the $effect in the example's createTodo could live in the .svelte file with the same outcome.

I'm not sure what a reasonable solution to this could look like. We can't syntactically restrict $effect to component initialisation, because you need to be able to use effects outside components (see e.g. this example from https://github.com/sveltejs/svelte/issues/11030#issuecomment-2032963869, in which we use an effect to set up event listeners for as long as something cares about the result), and manual control over lifecycle isn't possible. (Sure, you could create your own $effect.root and manually unmount it when the current effect is torn down, but in order to know when the current effect is torn down... you need an effect.)

I'm open to ideas but I think this is probably just a question of documentation.