Svelte 5: Incorrect Server Output for $effect.root

[abdel-17]

Describe the bug

On the server, $effect.root should return a noop function, but it returns undefined because the function is immediately invoked.

Reproduction

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAEz2N0QrDIAxFf0XCHiyM9d21hX3HuodiIwhWRWNhiP_eyGBcyOWE3NwKxjrMoN4V_HYgKHjFCHegb-yQT3SEzDmUpPtmyjrZSMvqV9LBZxLa4eZLFLO4oTGo6ZFCICkHMS-i9jsWzzY82XrsF5Cdp_H_j1uOsFtjcQdFqWD7tAuiqNgToAAAAA==

Logs

No response

System Info

Safari 17.4 iOS

Severity

annoyance

[Kapsonfire-DE]

can confirm it, behaviour changed in 175. 174 still returns a function

[dummdidumm]

The question is whether effect.root should just return a noop function, or if it should immediately invoke the function to execute stuff outside of effects inside, and then return the provided cleanup function and as backup a noop function.

[abdel-17]

The question is whether effect.root should just return a noop function, or if it should immediately invoke the function to execute stuff outside of effects inside, and then return the provided cleanup function and as backup a noop function.

My intuition says keep anything related to $effect client only. It would be strange for $effect.root to be the only fn that executes on the server.

[paoloricciuti]

The question is whether effect.root should just return a noop function, or if it should immediately invoke the function to execute stuff outside of effects inside, and then return the provided cleanup function and as backup a noop function.

I wanted to work on this but i actually have the same question. Can it make sense to have effect.root run on the server even if the effects inside it doesn't? Not running the function is currently a breaking change. Should i open the pr and then we can think about it?

[abdel-17]

Can it make sense to have effect.root run on the server even if the effects inside it doesn't? Not running the function is currently a breaking change.

Are you sure about this? It seems $effect.root is currently striped entirely from the server output.

[paoloricciuti]

Can it make sense to have effect.root run on the server even if the effects inside it doesn't? Not running the function is currently a breaking change.

Are you sure about this? It seems $effect.root is currently striped entirely from the server output.

<script>
	const cleanup = $effect.root(() => {
		console.log("this should run")
	});

	cleanup();
</script>

this generates the following code

import * as $ from "svelte/internal/server";

export default function App($$payload) {
	const cleanup = (() => {
		console.log("this should run");
	})();

	cleanup();
}

so it should be executed on the server.

[abdel-17]

It's actually inconsistent. It gets stripped when you don't capture the return value.

$effect.root(() => {
	console.log("1");
});

const cleanup = $effect.root(() => {
	console.log("2");
});

// generates
const cleanup = (() => {
	console.log("2");
})();

[paoloricciuti]

Uh then I guess the best thing is probably just change effect.root with a noop if it gets captured. The fact that is stripped shows that it's not meant to be executed

[Kapsonfire-DE]

the docs saying, $effects run on mount (pre before update), therefor it should never run on serverside

To run side-effects when the component is mounted to the DOM, and when values change, we can use the $effect rune (demo):

IMO
therefore $effect.root should never run on SSR only on hydration

The fix for return should be a no-brainer

 if (rune === '$effect.root') {
            const args = /** @type {import('estree').Expression[]} */ (
                node.arguments.map((arg) => context.visit(arg))
            );
            // Just call the function directly
            return b.call(args[0])??(()=>{});
        }

[paoloricciuti]

the docs saying, $effects run on mount (pre before update), therefor it should never run on serverside

To run side-effects when the component is mounted to the DOM, and when values change, we can use the $effect rune (demo):

IMO therefore $effect.root should never run on SSR only on hydration

The fix for return should be a no-brainer

 if (rune === '$effect.root') {
            const args = /** @type {import('estree').Expression[]} */ (
                node.arguments.map((arg) => context.visit(arg))
            );
            // Just call the function directly
            return b.call(args[0])??(()=>{});
        }

effects and effect.root are two different things but I do agree that it's probably not meant to run on the client.

The fix is a tad different because you want to return the expression that represents a noop and not the noop itself, also at this point is not even worth visiting the node on the server, you just want to completely remove effect.root if the return value is not assigned or substitute it with a noop of it is.

[trueadm]

We could make $effect.root not run on the server, but technically it always has, it's just the effects themselves that don't. The problem here is that if you don't return your own cleanup function, then maybe we should provide an empty function for that case.

[paoloricciuti]

We could make $effect.root not run on the server, but technically it always has, it's just the effects themselves that don't. The problem here is that if you don't return your own cleanup function, then maybe we should provide an empty function for that case.

Yeah the idea was to just provide a noop for that. Also #12322 (comment) suggest it was not run if the user did not assigned the return value.

Also will you work on this? I was planning to work on this in 20-ish minutes but if you are planning to work on it now up to you.

[paoloricciuti]

After talking with Dominic he confirmed that effect root should actually run on the server and the fact that it's eliminated is actually a bug...will fix both here

[Kapsonfire-DE]

can you make clear in the preview docs, that root behaves different and also meant to be run on server side?

[Antonio-Bennett]

@Kapsonfire-DE I think they ended up going with the PR that ignores it on the server so it behaves like the other runes? Correct me if I’m wrong