Skip to content
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

$derived(...) argument expression is never executed #13310

Closed
timephy opened this issue Sep 18, 2024 · 28 comments
Closed

$derived(...) argument expression is never executed #13310

timephy opened this issue Sep 18, 2024 · 28 comments

Comments

@timephy
Copy link

timephy commented Sep 18, 2024

Describe the bug

What happenes

In my mental model Runes try their best to behave just like "normal variables". Therefore it was my expectation that console.log("derive x") would be run, because I thought the value would/should be "initialized".

<script lang="ts">
    let x = $derived(console.log("derive x")) // is never executed ...
    let y = $derived.by(() => {
        console.log("derive y") // is never executed ...
        return 0
    })
</script>

<!-- ... when not "observed somewhere" -->
<!-- {x} -->
<!-- {y} -->

I came across this, when using the workaround described here for solving #13249 manually:

<script lang="ts">
    let _events: Events | undefined = undefined
    let events = $derived.by(() => {
        console.log("Creating new Events")
         _events?.unsubscribe()
         _events = new Events(DEPENDENCY)
        _events.setup()
        return _events
    })
    // here nothing is executed
</script>

What I think should happen

I want to argue that the inner expression/function should always be evaluated, because it goes against how variables and function in JavaScript should behave.

1. An outter function can never stop execution of it's argument expression:

// execution of `console.log(1)` can never be prevented by `anyNormalJsFunction` in any way
anyNormalJsFunction(console.log(1))

// then i argue this should NEVER stop execution either
$derived(console.log(2)) // this never executed, but should

2. This "wanted behaviour" can be created by simply adding any form of observer

<script lang="ts">
    let x: number = $derived.by(() => {
        console.log("derive x") // now this is executed
        return 0
    })

    $effect(() => {
        x // this
    })
</script>

<!-- ... or this -->
<!-- {x} -->

Subscribing provides the expected behaviour (with the exception of the execution order when having multiple $derives.

3. This is a feature!

I have been using Svelte for 3 years now and I loved it since the start.
I also think the new Runes are amazing! I love the Svelte-way of writing applications.

In Svelte 4 I have been using rxjs a lot to create reactive behaviour. I believe that Svelte 5 is missing just a tiny bit of "power" to fully replace my need for rxjs. Such as this issue (or also #13249, which can be worked around in a more intuitive/sensible way).

Conclusion

I sincerely ask for a change in behaviour.

While I believe to understand why this happens, "the value is lazy" ("because it is not observed/subscribed to, and therefore is not generated/initialized").

I want to ask you to think about the users expectation (especially as explained in section 1).
And think about it as a feature, because it allows the user to create reactive behaviour as they might want to.

I understand that the ideom is "no sideeffects", however this is distant from the reality of needs that a user often has.

Reproduction

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAE5WSTU_CQBCG_8o48dBiW6x6IkAC3PXizfVQ26msLLvN7ixKmv53s3wImF44zkfeZ-adabGWihyO3lrUxZpwhLOmwQR524TAbUgxYYLOeFuGzNiVVjY8FVrwcAh5BryUDqQD-qHSM1UQfRu7clCEVEMlUxWHbkUMpfGa72ECt44Lpqg02hlFmTKfkcB9VWAcg9AHwMMJ8PzyegbRhoEN8JLAO7JHWMHS6EteHngVWbmhqo-YB-If8LFnI6l3nIHUG7KOBmBsRRZ4WfAOfmC7C-zsDJt9bKMohskU2tAjuGeMmcB4X7TE3mrIQ9Rd7jK_UnT-X_Qw2l2v-uJK9UWv-vxcfTw8PYwe36QpPGUg6z-PFdUMxnMSHNbBfekg3PZofgJf3jEouaLwDGk6Fbrd0ztMcG0qWUuqcMTWU_fe_QJofQiY0wIAAA==

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAE22OzWqEQBCEX6XT5KDgT3IVV8hzZHJYndrdgbFHZkZXEd89DCaQQ45f81VX7XwzFoGbz53lOoIb_pgmLjhuU4KwwEZwwcHNfkiXNgzeTJHsVe4XxTEo7pSoaBFpbUjmsYenC71qeLNAV_2WZTldOtqTpuLgJDiLyrp7pvi0aFWcU12TCSRY4AkrhjlCU1VVZ84jzl7oLdGRK_kt3f4pzf6WvOe5krY-d6et0r6UZXpMzweExEVS7PoAv0BTcCOeD3goprLsfux9PRJxwaPT5maguYl-xvF1fAOg6eimRAEAAA==

Logs

No response

System Info

svelte 5 playground

Severity

blocking an upgrade

@paoloricciuti
Copy link
Member

First thing first, thanks for you thoughtful issue. Now for the bad news 😜

This was kinda proposed before in some variation or another. However we definitely don't want to lose the "lazyness" of the deriveds or lose the expressive value that comes from the ability to just write an expression without needing any function (there's $derived.by for that. Furthermore while runes definitely looks like function they are not and they definitely don't behave like one. Would you expect an expression to be run again when some variable that is declared as state is used inside it only because is the argument of a function.

Also also while I agree that if you think of runes as functions it may seem weird what's your use case to create a derived that is never accessed and don't use any state variable? Why is this blocking an upgrade? Why can't you put the same log in an effect or even better in the component script itself?

@timephy
Copy link
Author

timephy commented Sep 18, 2024

I can imagine what it would mean to change the behaviour from lazy to "sync", I can believe this might even be impossible now at this point in time.


My case

I would like to share where I am coming from, maybe you can teach me the right way to do this instead.

<script lang="ts">
    const {
        group,
    }: {
        group: Group
    } = $props()

    // Call
    let _call: Call | undefined = undefined
    const call = $derived.by(() => { // a Call is created that depends on the Group
        _call?.destroy()
        console.log("Creating new Call")
        _call = new Call(group)
        return _call
    })
    onDestroy(() => {
        call.destroy()
    })

    // Events
    $effect(() => { // a Events is created that depends on the Call
        console.log("Creating new Events")
        const events = new Events(call)
        return () => events.unsubscribe()
    })
</script>

<!-- `call` is used somewhere in the template -->
<!-- `events` is NOT used in the template -->

Context

See, I have these two classes (Call and Events), that all have dependencies, I want them recreated using Svelte-Reactivity when their dependencies change (Group <- Call <- Events).

The code above does work as expected. I have:

  • I have a constructor+destructor for all classes
  • I have to treat them these classes differently in the script-code, because call is observed (in the template below), but events is not

For the Events I seem to have to use $effect, because it is not observed anywhere else.

  • I like using $effect, because of the cleanup function I can return (see how short and clean the Events-code is)
  • I can not use $effect for Call, because I depend on its value in the template (and setting a variable from inside an $effect would require me to init the variable as let call: Call | null = null, which is terrible to keep track of)

For the Call I seem to have to use $derived, because I require its value in the template.


Thoughts

I think that I have to distinguish these two cases (observed vs. not-observed) when coding is not good. First of all knowing whether a variable is observed anywhere is easy to lose track of, and second because the difference it makes in the code I have to write is large (see Call vs Events implementation above, large difference in the code)

I hope to have illustrated the need for the behaviour I seek, maybe I am thinking about this wrong?

I love the possibility to create reactive behaviour that updates with its dependencies, but I am missing some "power" to make it as useful as I need.


Key Points

  • the same "logic" (Call construct+destruct & Events construct+destruct) has to be handled very differently, depending on whether it is observed or not
  • easy to mess up, removing a derived-variable from the template results in all it's logic never running
  • $effect and $derived are great, but I feel constrained using them

Also also while I agree that if you think of runes as functions

Right, but no, I don't - I know it's more of a "variable-decorator".

Would you expect an expression to be run again when some variable that is declared as state is used inside it only because is the argument of a function.

If it is writing inside an $effect, yes. If it is written inside a $derived, yes, yes I would expect that to re-run too.

@paoloricciuti
Copy link
Member

If it is writing inside an $effect, yes. If it is written inside a $derived, yes, yes I would expect that to re-run too.

I was thinking of normal JS functions just to prove the point that they are not normal JS functions.

Gonna take a look at your code.

@paoloricciuti
Copy link
Member

Ok took a look at your code...to me what you want is $effect.pre. Let me see if I can craft an example for you.

@paoloricciuti
Copy link
Member

Ok here's a quick example

<script lang="ts">
	import {Call, Events} from "./classes.js";
	
	const { group } = $props();

	let call: Call = $state();

	$effect.pre(() => {
		call = new Call(group);
		return ()=>{
			call.destroy();
		}
	});

	$effect(() => {
		const events = new Events(call);
		return ()=>{
			events.destroy();
		}
	});
</script>

Using call in template {call.group}

You can safely declare call as Call because $effect.pre runs before the template so you know it will always be there.

@timephy
Copy link
Author

timephy commented Sep 18, 2024

This approach makes sense.

But it creates a typing problem:

<!-- This is your quick example copied into one file -->
<script lang="ts">
    type Group = number
    class Call {
        group: Group
        constructor(group: Group) {
            this.group = group
        }
        destroy() {
            console.log("destroying call this.group=", this.group)
        }
    }
    class Events {
        call: Call
        constructor(call: Call) {
            this.call = call
        }
        destroy() {
            console.log("destroying events this.call.group=", this.call.group)
        }
    }

    /* ================================================================================================================== */

    const { group }: { group: Group } = $props()

    let call: Call = $state() // Type 'Call | undefined' is not assignable to type 'Call'. Type 'undefined' is not assignable to type 'Call'.ts(2322)

    $effect.pre(() => {
        call = new Call(group)
        return () => {
            call.destroy()
        }
    })

    $effect(() => {
        const events = new Events(call)
        return () => {
            events.destroy()
        }
    })
</script>

Using call in template {call.group}

This can be fixed by writing this instead:

let call: Call = $state()!

I ran into this problem before when having a state variable within a class that is set in the constructor:

class InnerStateKnownInConstructor {
    inner: number = $state()!
    constructor() {
        this.inner = 1
    }
}

Is there a better way to do it?


Even when asserted with the !, this then makes it "not-typesafe" to remove the $effect.pre. I mean, i can now freely remove the $effect.pre and TypeScript will never complain, because I have asserted the type with !.

Can you solve this problem too?

@timephy
Copy link
Author

timephy commented Sep 18, 2024

Btw, I believe this also does not work with SSR, because $effect.pre is run just before mounting, right?

$derived would work with SSR and CSR i believe...

@paoloricciuti
Copy link
Member

There's actually already an issue with this #12655 and as you can see by the discussion it's not that easy.

If you have any good ideas we are eager to hear them tho.

Btw, I believe this also does not work with SSR, because $effect.pre is run just before mounting, right?

$derived would work with SSR and CSR i believe...

Uh yeah you are right. I wonder if the right solution is just to create a new Call as initial state. I know you talked about registering listeners and stuff but again you should just avoiding doing that on the server

@paoloricciuti
Copy link
Member

paoloricciuti commented Sep 18, 2024

Also if you want to avoid the initial "waste" of creating two instances I would just go with

let call = $state(new Call(group));
let first = true;

$effect.pre(()=>{
    if(first){
        first = false;
        call;
        return ()=> call.destroy();
    }
    call = new Call(group);
    return ()=> call.destroy();
});

@paoloricciuti
Copy link
Member

Btw since executing deriveds is still not the solution to your problem I'm gonna close this issue. Feel free to continue here if you need any other help or if you want to share ideas.

@timephy
Copy link
Author

timephy commented Sep 18, 2024

This line in your example breaks when SSR is enabled (no TypeScript warning):

Using call in template {call.group}

-> TypeError: Cannot read properties of undefined (reading 'group') on request during runtime


Also if you want to avoid the initial "waste" of creating two instances I would just go with

Well yes... That would work... It's not pretty.


Btw since executing deriveds is still not the solution to your problem

Is it not?

What would solve my problem immediately, would be if I did this:

<script lang="ts">
    import {Call, Events} from "./classes.js";
	
    const { group }: { group: Group } = $props()

    let _call: Call | undefined = undefined
    let call = $derived.by(() => {
        _call?.destroy()
        _call = new Call(group)
        return _call
    })
    onDestroy(() => {
        call.destroy()
    })
    $effect.pre(() => { // makes sure it is always "loaded" even when not used in template
        call
    })

    // and the same for Call...
</script>

But just slapping an $effect around every $derived to make sure it is loaded feels terrible.


It is a shame one has to pick between these two:

$derived

  • Works CSR+SSR (+)
  • Manual Cleanup (-)
  • Typesafe (+)

$effect.pre

  • Works only CSR (-)
  • Built-in Cleanup (+)
  • Typing issues (-)

@paoloricciuti paoloricciuti reopened this Sep 18, 2024
@paoloricciuti
Copy link
Member

This line in your example breaks when SSR is enabled (no TypeScript warning):

This feels weird...is call initialized in the script?

@timephy
Copy link
Author

timephy commented Sep 18, 2024

This line in your example breaks when SSR is enabled (no TypeScript warning):

This feels weird...is call initialized in the script?

It breaks during SSR, because $effect.pre only runs client-side (right?), and therefore, no, call is not initialized on the server-side.

let call: Call = $state()!

$effect.pre(() => {
    call = new Call(group)
    return () => {
        call.destroy()
    }
})

console.log("call", call) // `call === undefined` on server-side

And the hack of using $state()!, lets TypeScript trust it.

@timephy
Copy link
Author

timephy commented Sep 18, 2024

What I would love to achieve is to make this more convenient:

let _call: Call | undefined = undefined // cleanup on recreate
let call = $derived.by(() => { // have a typesafe variable and value immediately, that may or may not be observed
    _call?.destroy() // cleanup on recreate
    _call = new Call(group)
    return _call
})
onDestroy(() => {
    call.destroy() // cleanup on destroy
})
$effect(() => { // have the logic in `derived` execute, independent of if the variable is observed
    call
})

I think being able to do such things would make the Svelte reactivity system "powerful" enough to solve a much larger scope of "problems"/situations than before. It would dramatically empower the user to do more with it. Not requiring other libraries to create reactive behaviour, more than "just" creating reactive UI.

@paoloricciuti
Copy link
Member

Also if you want to avoid the initial "waste" of creating two instances I would just go with

let call = $state(new Call(group));
let first = true;

$effect.pre(()=>{
    if(first){
        first = false;
        call;
        return ()=> call.destroy();
    }
    call = new Call(group);
    return ()=> call.destroy();
});

Did you miss the part where I initialize call outside? 😄

@timephy
Copy link
Author

timephy commented Sep 18, 2024

Did you miss the part where I initialize call outside? 😄

No i saw that.. If you say that is the best solution.. I'll have to go with it. Still feels to me like a missed opportunity to simplify.

@timephy
Copy link
Author

timephy commented Sep 18, 2024

Thanks for the help and openness to discuss! Really appreciate it!

Don't get me wrong, I am a huge fan of Runes!

For building UI, they rock.
These issues only appear when I try to apply them to Logic&Functionality instead of UI.

@paoloricciuti
Copy link
Member

Did you miss the part where I initialize call outside? 😄

No i saw that.. If you say that is the best solution.. I'll have to go with it. Still feels to me like a massive missed opportunity to simplify.

I get why you say that but this is basically to simplify your use case which is not the common use case. And all this new apis that would simplify your use case needs to be weighted in to avoid creating a very bad API. We might still go with the previous value for the derived (I will actually work on that tomorrow) but it's not before actually having weighted the pros and cons and possible impact on the future.

Adding a cleanup to derived is wrong when you consider that they should be side effect free. We might do it in the future if we think is the best way forward and the same reasoning apply to this proposal. Executing derived eagerly would be a huge shift and that's just because in this use case it could be useful to use them as a form of effect.

Btw I personally don't even find your initial code that bad lol

@timephy
Copy link
Author

timephy commented Sep 18, 2024

Yes I understand. I can understand to want to keep the "simplicity", and not overload the functionality...

I am interested to see how Runes develop in the future.


One more thing:

Why does this print "hello" on the server-side, but not on the client-side?

Is $derived treated differently during SSR than during CSR?

<script lang="ts">
    const test = $derived(console.log("hello"))
    // `test` is not observed anywhere, its content is evaluated during SSR, but not during CSR
</script>

@paoloricciuti
Copy link
Member

Yes I understand. I can understand to want to keep the "simplicity", and not overload the functionality...

I am interested to see how Runes develop in the future.

One more thing:

Why does this print "hello" on the server-side, but not on the client-side?

Is $derived treated differently during SSR than during CSR?

<script lang="ts">
    const test = $derived(console.log("hello"))
</script>

Yes...the server is one shot so every expression is immediately inlined so that we can get the value to SSR.

    const test = console.log("hello")

P.s. you can check the server output from the repl itself

We could probably work around this but that would just add complexity for a relatively low gain.

@timephy
Copy link
Author

timephy commented Sep 18, 2024

Thanks again!

@timephy
Copy link
Author

timephy commented Sep 19, 2024

Also if you want to avoid the initial "waste" of creating two instances I would just go with

let call = $state(new Call(group));
let first = true;

$effect.pre(()=>{
    if(first){
        first = false;
        call;
        return ()=> call.destroy();
    }
    call = new Call(group);
    return ()=> call.destroy();
});

Did you miss the part where I initialize call outside? 😄

Btw, this does not work sadly...

Can it be that $effect learns in its first run (during first === true) that it does not depend on new Call(group)?

@paoloricciuti
Copy link
Member

Can it be that $effect learns in its first run (during first === true) that it does not depend on new Call(group)?

Oh i was just tired yesterday writing on the phone lol. It should be like this

let call = $state(new Call(group));
let first = true;

$effect.pre(()=>{
    if(first){
        first = false;
        group;
        return ()=> call.destroy();
    }
    call = new Call(group);
    return ()=> call.destroy();
});

@timephy
Copy link
Author

timephy commented Sep 19, 2024

Hey, this is what I came up with, it creates an easy to use solution for the behaviour I wanted:

// # Component
let group = $state(0)

// This variable is not observed anywhere
let call = $derived.by(
    magic({
        construct: () => new Call(group),
        destruct: (curr) => curr.destruct(), // `() => call.destruct()` works as well
    }),
)

// # Library
type Opts<T> = {
    construct: () => T
    destruct: (curr: T) => void
}
/** Use this inside a `$derived.by(...)`. */
function magic<T>(opts: Opts<T>) {
    let curr: T = $state()!
    // NOTE: This constructs and recreates the value on the client-side
    $effect.pre(() => {
        curr = opts.construct()
        return () => opts.destruct(curr)
    })
    // NOTE: Since `$effect` does not run on the server, we need to construct and destruct the value here manually
    if (!browser) {
        curr = opts.construct()
        onDestroy(() => opts.destruct(curr))
    }
    // NOTE: This is the value that will be returned through `$derived.by`
    return () => curr
}

It has the following beneficial behaviour:

  • Benefits of $derived

    • call has a typesafe value that may or may not be used
    • works with SSR + CSR
  • Benefits of $effect

    • always runs, also when the derived-variable is not observed anywhere
    • automatic cleanup
  • Downsides

    • has to be used inside $derived.by(...) to work
    • not yet usable in .svelte.ts files, as it uses $effect.pre without $effect.root + uses onDestroy (i believe)

Questions

Is this safe to use?

Is there a future where the user can define something like this as a "custom rune"?
To be able to write let call = $magic({ ... }) instead of let call = $derived.by(magic({ ... })).

Is there a way to detect if code is running inside a Component or outside to address and fix the second downside?

@timephy
Copy link
Author

timephy commented Sep 19, 2024

This change makes it possible to use inside .svelte.ts files, but would be great if inComponent can be determined from inside the function:

export type Opts<T> = {
    construct: () => T
    destruct: (curr: T) => void
}
/** Use this inside a `$derived.by(...)`. */
export function magic<T>(opts: Opts<T>, inComponent = true) {
    let curr: T = $state()!

    if (browser) {
        // NOTE: This constructs and recreates the value on the client-side
        const _effect = () => {
            $effect.pre(() => {
                curr = opts.construct()
                return () => opts.destruct(curr)
            })
        }
        if (inComponent) {
            _effect()
        } else {
            $effect.root(_effect)
        }
    } else {
        curr = opts.construct()
        if (inComponent) onDestroy(() => opts.destruct(curr))
    }

    // NOTE: This is the value that will be returned through `$derived.by`
    return () => curr
}

@paoloricciuti
Copy link
Member

Is this safe to use?

I don't see why not.

Is there a way to detect if code is running inside a Component or outside to address and fix the second downside?

You can use $effect.tracking() to know if you are inside a derived.by or just inside the .svelte.js file. But pay attention: the reason why effects need to be inside other effects is because when the parent effect is destroyed it can clean up the childs dependencies. Putting an effect in $effect.root will make it work but you should take care of cleaning up the dependencies yourself.

Is there a future where the user can define something like this as a "custom rune"?
To be able to write let call = $magic({ ... }) instead of let call = $derived.by(magic({ ... })).

99% no...if we let people define their own "runes" we bound ourself to not be able to introduce new runes in a minor

@timephy
Copy link
Author

timephy commented Sep 19, 2024

Is this safe to use?

I don't see why not.

Great! 👍🏽

Is there a way to detect if code is running inside a Component or outside to address and fix the second downside?

You can use $effect.tracking() to know if you are inside a derived.by or just inside the .svelte.js file. But pay attention: the reason why effects need to be inside other effects is because when the parent effect is destroyed it can clean up the childs dependencies. Putting an effect in $effect.root will make it work but you should take care of cleaning up the dependencies yourself.

$effect.tracking() does not solve this, because an outter $derived.by is required regardless of whether we are in .svelte or .svelte.ts i believe... Any other way to determine whether code is executed from within a Component?

$effect.tracking() does not even detect being inside $derived.by, does it? So would not work either way.

Is there a future where the user can define something like this as a "custom rune"?
To be able to write let call = $magic({ ... }) instead of let call = $derived.by(magic({ ... })).

99% no...if we let people define their own "runes" we bound ourself to not be able to introduce new runes in a minor

Understood

@paoloricciuti
Copy link
Member

$effect.tracking() does not solve this, because an outter $derived.by is required regardless of whether we are in .svelte or .svelte.ts i believe... Any other way to determine whether code is executed from within a Component?

$effect.tracking() does not even detect being inside $derived.by, does it? So would not work either way.

$derived.by can help you use the thing without having to call a function but to use the thing you still need to end up in a component (how does having something reactive helps you in a module?).

If you have an example of how you plan to use this in a module maybe that would be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants