trueadm commented 4 months ago

Rich here — after extensive discussion we decided to leave out the previous value from the signature, and to call it $derived.call(fn) rather than $derived.fn(fn). Original comment follows:

`$derived.fn`

Sometimes you need to create complex derivations which don't fit inside a short expression. In this case, you can resort to $derived.fn which accepts a function as its argument and returns its value.

<script>
    let count = $state(0);
    let complex = $derived.fn(() => {
        let tmp = count;
        if (count > 10) {
            tmp += 100;
        }
        return tmp * 2;
    });
</script>

<button on:click={() => count++}>
    {complex}
</button>

$derived.fn passes the previous derived value as a single argument to the derived function. This can be useful for when you might want to compare the latest derived value with the previous derived value. Furthermore, you might want to pluck out specific properties of derived state to use in the new derived state given a certain condition – which can be useful when dealing with more complex state machines.

<script>
    let items = $state([1, 1, 1]);

    let reversed = $derived.fn((prev) => {
        const reversed = items.toReversed();
        // Naive deep equals comparison
        if (JSON.stringify(reversed) === JSON.stringify(prev)) {
            return prev;
        }
        return reversed;
    });
</script>

<button on:click={() => items.push(1)}>
    {reversed}
</button>

changeset-bot[bot] commented 4 months ago

🦋 Changeset detected

Latest commit: 97261bf84626bc51b762681e3e3aeb602790f9a9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

| Name | Type | | ------ | ----- | | svelte | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Rich-Harris commented 4 months ago

I'm not convinced this is the right API.

Firstly, consider the case where you want to reuse the comparison logic in multiple places. You'd have to do this sort of thing...

<script>
  let one = $state([1, 1, 1]);
  let two = $state([2, 2, 2]);

  function equal(a, b) {
    return JSON.stringify(a) === JSON.stringify(b);
  }

  let one_reversed = $derived.fn((prev) => {
    const reversed = one.toReversed();
    if (equal(reversed, prev)) return prev;
    return reversed;
  });

  let two_reversed = $derived.fn((prev) => {
    const reversed = two.toReversed();
    if (equal(reversed, prev)) return prev;
    return reversed;
  });
</script>

...when this would be much easier:

<script>
  let one = $state([1, 1, 1]);
  let two = $state([2, 2, 2]);

  function equal(a, b) {
    return JSON.stringify(a) === JSON.stringify(b);
  }

  let one_reversed = $derived(one.toReversed(), { equal });
  let two_reversed = $derived(two.toReversed(), { equal });
</script>

Secondly, the fact that prev is undefined at first will often make the logic more cumbersome than it needs to be. At the very least, you'll need to use optional chaining and/or type narrowing in a lot of cases.

Thirdly, it's honestly a bit weird that you signal 'the values are equal' by returning prev. It feels like an accident, rather than deliberate API design.

Fourthly, if equality checking is useful, it's useful for $derived(...) as well as $derived.fn(...).

I'm still not wholly convinced that we need this, but if we really must then I really think we should work on the naming. Elsewhere we prefer to use full words where possible. I'm not sure what a better name would be — will have a think — but I hope we can avoid .fn.

Rich-Harris commented 4 months ago

I also think it's worth noting that useMemo doesn't take a previous value, and as far as I know this hasn't been a problem for React. Are we sure we need it?

Not-Jayden commented 4 months ago

I'm not convinced this is the right API.

Firstly, consider the case where you want to reuse the comparison logic in multiple places. You'd have to do this sort of thing...

<script>
  let one = $state([1, 1, 1]);
  let two = $state([2, 2, 2]);

  function equal(a, b) {
    return JSON.stringify(a) === JSON.stringify(b);
  }

  let one_reversed = $derived.fn((prev) => {
    const reversed = one.toReversed();
    if (equal(reversed, prev)) return prev;
    return reversed;
  });

  let two_reversed = $derived.fn((prev) => {
    const reversed = two.toReversed();
    if (equal(reversed, prev)) return prev;
    return reversed;
  });
</script>

I mean I figure you'd end up use curried functions for those cases where you would be straight duplicating the logic. i.e.

<script>
  let one = $state([1, 1, 1]);
  let two = $state([2, 2, 2]);

  function equal(a, b) {
    return JSON.stringify(a) === JSON.stringify(b);
  }

  function createReverser(state) {
    return (prev) => {
      const reversed = state.toReversed();
      if (equal(reversed, prev)) return prev;
      return reversed;
    };
  }

  let one_reversed = $derived.fn(createReverser(one));
  let two_reversed = $derived.fn(createReverser(two));
</script>

...when this would be much easier:

<script>
  let one = $state([1, 1, 1]);
  let two = $state([2, 2, 2]);

  function equal(a, b) {
    return JSON.stringify(a) === JSON.stringify(b);
  }

  let one_reversed = $derived(one.toReversed(), { equal });
  let two_reversed = $derived(two.toReversed(), { equal });
</script>

This is definitely more succinct, though it's not immediately obvious to me what is actually happening here. I take it equal a reserved property that accepts a predicate? Could be a nice addition, though it still doesn't address the first consideration of being able to define complex logic inside of $derived without IIFE's.

I do also agree all things considered, the desire to avoid IIFE's is very much a convenience/preference thing. That said there are definitely people with stronger opinions than my own, and using this syntax to provide the prev callback value does add some additional justification on top of that at least.

dummdidumm commented 4 months ago

example from discord for another potential use case.

trueadm commented 4 months ago

I also think it's worth noting that useMemo doesn't take a previous value, and as far as I know this hasn't been a problem for React. Are we sure we need it?

You'd need to use useRef to store the old value in from experience. Which is cumbersome. Either way, I do think the naming could be resolved for this PR, but having access to the previous value is super invaluable for complex cases. As for equality, we can still have that as dedicated option and change the documentation, however maybe it's not needed unless we plan on providing it for $state too (which might be harder to explain now with deep state).

Rich-Harris commented 4 months ago

I mean I figure you'd end up use curried functions for those cases

...which is a level of complexity that isn't really in keeping with Svelte. It's also broken — updating one and two doesn't create a new deriver. For that to work, you'd have to put the function inside another function... this feels like a very strong signal that the API is fatally flawed, with or without the prev value. You'd probably need to enforce that the argument to $derived.fn must be a function expression (or arrow function expression) — hello there, uncanny valley.

example from discord for another potential use case.

Not sure I understand. You can just do this?

-const prev = $derived.fn(previousState(() => value))
+const prev = $derived(previousState(() => value)())

So there's two things going on here:

using functions instead of expressions — I understand the desire for this, but as shown above there's a real footgun involved, and the mitigations for that footgun (either enforcing function expressions, which reduces reusability, or adding the runtime overhead of tracking whether signals were read while creating the deriver) have drawbacks that we probably don't want
passing prev into the deriver — the API is a non-starter in my opinion, for the reasons outlined above. I would love to understand some non-contrived use cases for this. If you really need it, it's solvable in userland (albeit clunkily), but I suspect most of the time it's actually undesirable (equality checking isn't free — it's often quite expensive — and in general you want equality checks to be as far upstream as possible rather than in downstream derivers) so we probably don't want to actively encourage people to use it.

dummdidumm commented 4 months ago

I don't think that this kind of mistake in usage hints at a fatal flaw in API design, it's just how the system works. Having access to the previous value is a nice bonus to enable reducer patterns (the equality thing is an accidental side effect of this for me)

trueadm commented 4 months ago

passing prev into the deriver — the API is a non-starter in my opinion, for the reasons outlined above. I would love to understand some non-contrived use cases for this. If you really need it, it's solvable in userland (albeit clunky), but I suspect most of the time it's actually undesirable (equality checking isn't free — it's often quite expensive — and in general you want equality checks to be as far upstream as possible rather than in downstream derivers) so we probably don't want to actively encourage people to use it.

I'm not sure why this is a non-starter TBH. Other than it initially being undefined, which means you can pass a default argument for it, I don't see the problems. It has valid usability as I was trying to point out in https://gist.github.com/trueadm/671dbe4d085cc6d313fcd8074f73a1b7. This works just like it does with createMemo from Solid and that's how I'd like it to work here. Negating the use cases for equality, we can skip that topic and have a dedicated API for it, as it does read better.

If I'm using $derived to generate new $state each time, either via a class or just deep state, then I'm hinting that some state gets updated from above and thus needs to stay in "sync". However, I might also have some state in that derived value that is not designed to be in sync. This is what I demonstrated on https://gist.github.com/trueadm/671dbe4d085cc6d313fcd8074f73a1b7 with the latest changes.

In many ways this is kind of like how you can set state in React with knowledge of the previous values (setState accepts a function that gives you the previous value, so you derive new state given the previous state) – and there's also useReducer that provides the same pattern. I guess what I'm looking for is something that's like this:

<script>
  const { markdown, submitMarkdown } = $props();

  let is_editing = $state(false);
  const initial_value = { markdown: '' };
  const state = $derived.reduce(state => {
    let new_state = $state({ markdown: is_editing ? state.markdown : markdown });

    return new_state;
  }, initial_value);
</script>

<MarkdownEditor
  bind:markdown={state.markdown}
  onEdit={() => is_editing = true}
  onReadOnly={() => is_editing = false} 
  onSave={() => submitMarkdown(state.markdown)}
/>

Imagine we don't control <MarkdownEditor>, it's some third-party library that expects a bound prop, how can we make this work without using effects today?

It's tempting to try and do something like this:

<script>
  const { markdown, submitMarkdown } = $props();

  let is_editing = $state(false);
  let local_state = $state({ markdown });
  const state = $derived(is_editing ? local_state : { markdown });
</script>

However, when we switch modes back and forth, the local_state.markdown will go back to what was there before, rather than the latest version from props. Which seems like a sync issue.

I'm eagerly trying to push people away from this pattern:

https://discord.com/channels/457912077277855764/1153350350158450758/1198567900450144326

Another approach

So I thought to myself, how can we deal with this from another angle? Taking my above example, maybe it makes more sense to position it from an another angle. What if we could derive values from within state?

<script>
  const { markdown, submitMarkdown } = $props();

  let is_editing = $state(false);

  let state = $state({ markdown }, state => ({ markdown: is_editing ? state.markdown : markdown }));
</script>

Rich-Harris commented 4 months ago

Let's at least keep this issue open for now, since the discussion is still ongoing.

I'm not sure why this is a non-starter TBH. Other than it initially being undefined, which means you can pass a default argument for it, I don't see the problems.

I outlined a few points above, but consider also the impact on types:

As soon as prev becomes a potential return value, you can no longer rely on inference. An explicit equal option, were we to add such a thing, wouldn't have that drawback.

I think the reducer idea is interesting. Initially it seemed off because the reducer pattern is (state, action) => state and there's no action here — nothing gets dispatched. But you could think of the action as being implicit. I think this idea is worth exploring further, and trying it out in different scenarios.

I'm eagerly trying to push people away from this pattern:

https://discord.com/channels/457912077277855764/1153350350158450758/1198567900450144326

For posterity:

// data comes from sveltekit load function
const { data, form } = $props()

let markdown = $state(data.markdown)
$effect(() => {
  markdown = data.markdown
})

If we were to use bind:data inside SvelteKit to enable optimistic UI (#9905), this would work. That's one option, though it's not a general solution to the 'I have some state that I want to be able to edit locally' problem.

We need to fully articulate that problem though. If it's 'I want the view of some state to be this value while some condition (e.g. is_editing) is true, and this other value when it's false' then that's one thing (and something that can be solved today).

But if it's 'I want to have some locally edited state that is replaced whenever there's new stuff from the server' — which better describes the above snippet — then it seems to me that effects are the right tool for the job, because fundamentally what you're trying to do is have a piece of state that represents the most recent of \<data from server> and \<local edits>, and effects are how you incorporate temporal concepts like recency. I don't think a reducer pattern helps us here.

trueadm commented 4 months ago

But if it's 'I want to have some locally edited state that is replaced whenever there's new stuff from the server' — which better describes the above snippet — then it seems to me that effects are the right tool for the job, because fundamentally what you're trying to do is have a piece of state that represents the most recent of and , and effects are how you incorporate temporal concepts like recency. I don't think a reducer pattern helps us here.

One of things that I'm worried about is a future in where we have the forking of state. For example, a pending state tree for async/suspenseful transitioning UI that has not yet been applied. In a fork that is still pending, we cannot run effects – as they might mutate the DOM or trigger some other irreversible side-effect. If there is some effect that is responsible for syncing the state tree then that won't happen anymore. This is important, as an incoming prop may have changed, causing the pending state to change in some part. If that no longer happens then you have a glitchy state tree. So moving as much of that to derived state is always preferable.

Rich-Harris commented 4 months ago

Ha - I was also thinking about that stuff, but from a different angle. Namely that the smaller the API surface area today, the more freedom we'll have in future. Part of the reason I'm so resistant to new API in general is that I've often been painted into a corner by convenience APIs that weren't truly necessary.

Big picture: we're on the same page, just not fully aligned on all the details

mimbrown commented 4 months ago

using functions instead of expressions — I understand the desire for this, but as shown above there's a real footgun involved, and the mitigations for that footgun (either enforcing function expressions, which reduces reusability, or adding the runtime overhead of tracking whether signals were read while creating the deriver) have drawbacks that we probably don't want

I don't really understand this "adding the runtime overhead of tracking whether signals were read while creating the deriver."

If $derived(expr) compiles to $.derived(() => expr), then $derived.fn(function) would compile to $.derived(function) (which in my experience is how most people think about it anyway), and it doesn't matter whether the function is defined inline or elsewhere. The only way I could think of to use a signal while creating a deriver would be to use an IIFE inside derived.fn, which... kinda feels like the dev probably knows what they're doing at that point.

EDIT: I take that back, I understand that there's multiple ways that signals could be read while creating a deriver:

const myDerived = $derived.fn(higherOrderFunction());
const myDerived = $derived.fn(someCondition ? functionOne : functionTwo);

But I don't think it's cumbersome to explain that "only signals read inside the passed function will be tracked." I stand by that many people already think in those terms.

The ability to pass the function itself opens up possibilities like when you want to do something with feature detection, where you want the ability to pass completely different logic based on some check that only needs to happen once, like:

const myDerived = $derived.fn(browserSupportsFeatureX ? functionOne : functionTwo);

Thiagolino8 commented 4 months ago

Not sure I understand. You can just do this?
-const prev = $derived.fn(previousState(() => value))
+const prev = $derived(previousState(() => value)())
The example given by @dummdidumm was mine The reason I didn't use $derived but rather $derived.fn for the job was that I didn't know it was possible Because honestly it is not at all intuitive that $derived has two different behaviors, most of the time re-executing the entire passed expression but re-executing only the return of a function if that return is called For me a consistent behavior would be that if const c = $derived(a * 2) compiles to const c = $.derived(() => a * 2), then const c = $derived(double(() => a)()) should compile to const c = $.derived(() => double(() => a)()) not const c = $.derived(double(() => a)) In my opinion the problem is not that $derived.fn is doing something that $derived can already do, but that $derived is doing too many things As for the name, to maintain consistency with $inspect.with I think $derived.with would be ideal

aradalvand commented 4 months ago

I second the naming $derived.with over $derived.fn

dummdidumm commented 4 months ago

$inspect.with has nothing in common with derived semantically (the former reads like "inspect this value with that callback function", which $derived.with doesn't), and as such fn as a shorthand for function makes much more sense to me.

navorite commented 4 months ago

Does it have to be a separate API though, at least if there are no additional changes other than it being a function? Can't it just be overloaded and have the type checked in the compilation step?

Rich-Harris commented 4 months ago

But I don't think it's cumbersome to explain that "only signals read inside the passed function will be tracked." I stand by that many people already think in those terms.

I was making that point in response to @Not-Jayden's comment upthread...

I mean I figure you'd end up use curried functions for those cases where you would be straight duplicating the logic

...which wouldn't work. The very first example of someone using $derived.fn (and I'm not picking on @Not-Jayden here, it's a very easy error to make!) contained a subtle bug (subtle because it might not even fail until you refactor your code that mutates one to code that reassigns one). If you need to carefully read through the documentation and deeply understand the implementation in order to suppress your intuitions about how things should work, then the API is flawed.

That's why I'd be slightly more open to a version of this where you can only pass a function expression:

const ok = $derived.whatever(() => {...});
const not_ok = $derived.whatever(somefn);

It would prevent that category of bugs, at the cost of the 'uncanny valley' phenomenon.

.with definitely isn't the right name for this. One idea: $derived.block(() => {...})?

Does it have to be a separate API though, at least if there are no additional changes other than it being a function? Can't it just be overloaded and have the type checked in the compilation step?

Would love it if that was possible, but I don't think it is. Consider:

const filter_function = $derived(filter_functions[current_filter]);

How would you type that such that filter_function was a function and not the result of calling filter_function?

Thiagolino8 commented 4 months ago

Does it have to be a separate API though, at least if there are no additional changes other than it being a function? Can't it just be overloaded and have the type checked in the compilation step?

This was my first suggestion https://github.com/sveltejs/svelte/issues/9984, which was closed in favor of https://github.com/sveltejs/svelte/issues/9968

How would you type that such that filter_function was a function and not the result of calling filter_function?

In case of overloading, the correct return should be the result of calling filter_function Just make it clear that if the value passed to $derived is a function then the value returned will be the result of this function If you want the returned value to be the function itself, then just return it from a callback

const filter_result = $derived(filter_functions[current_filter]);
const filter_function = $derived(() => filter_functions[current_filter]);

mimbrown commented 4 months ago

The very first example of someone using $derived.fn (and I'm not picking on @Not-Jayden here, it's a very easy error to make!) contained a subtle bug (subtle because it might not even fail until you refactor your code that mutates one to code that reassigns one). If you need to carefully read through the documentation and deeply understand the implementation in order to suppress your intuitions about how things should work, then the API is flawed.

Fair, and I agree the bug is subtle, but… tbh, it feels more like a product of the signal design choices than this specific API. Like with svelte 5 we’re all going to have to get more used to paying attention to reference semantics, and I don’t think this is the only place where this issue will naturally occur. $effect takes a function and reacts to changes when signals inside of that function are read, which means this exact same problem you noted already exists in $effect.

My main concern is that helpful and powerful patterns are not disallowed.

mimbrown commented 4 months ago

BTW the repl that I just posted gives me a nice warning:

State referenced in its own scope will never update. Did you mean to reference it inside a closure?

I think the rules should be consistent between $effect and $derived[fn | block | function | whatevs]. If your takeaway @Rich-Harris is that $effect should be limited to only accept inline function expressions, then I guess I'll accept that as an unintended consequence of my arguments.

Not-Jayden commented 4 months ago

Naive question having not yet spent the time to dig into the internals yet.

GIven this does work as you would expected:

- let one_reversed = $derived.fn(createReverser(one));
+ let one_reversed = $derived.fn((prev) => createReverser(one)(prev));

Is there a reason the compiler can't do the heavy lifting to determine that it has received a compatible function and transpile it as such that it would be wrapped in a higher order function to make it work?

Not-Jayden commented 4 months ago

I'm not sure why this is a non-starter TBH. Other than it initially being undefined, which means you can pass a default argument for it, I don't see the problems.

I outlined a few points above, but consider also the impact on types:

As soon as prev becomes a potential return value, you can no longer rely on inference.

Yeah that type issue is a bit unfortunate 😕 You can at least use the generic, but I could see people getting confused with it.

Could the scope of this PR maybe be reduced in the short term to not include the prev parameter to allow for just using regular functions pending further discussion of the best way to implement it in the meantime, or is it considered blocking?

Also re: method naming, compute, calling, using, and from were some alternatives I'd considered. I still think with makes sense if you think of it as "The value is derived with the result of the function it receives", but I understand it's not exactly consistent with how $inspect is implemented.

Thinking on it more, compute would be my leading choice at this point given it will be familiar/consistent with the API for people coming from Vue, Solid, and Angular land.

mimbrown commented 4 months ago

Is there a reason the compiler can't do the heavy lifting to determine that it has received a compatible function and transpile it as such that it would be wrapped in a higher order function to make it work?

Even if the compiler could handle all those edge cases, how can it be sure that this is actually what the developer intended? The more the compiler changes the semantics of what the developer wrote, the less confident the developer will be that their code will work as intended.

I would rather shoot myself in the foot and have to learn how to fix it then have the compiler shoot me in the foot and for me to have to track that down in the compiled output.

Not-Jayden commented 4 months ago

Is there a reason the compiler can't do the heavy lifting to determine that it has received a compatible function and transpile it as such that it would be wrapped in a higher order function to make it work?

Even if the compiler could handle all those edge cases, how can it be sure that this is actually what the developer intended? The more the compiler changes the semantics of what the developer wrote, the less confident the developer will be that their code will work as intended.

I would rather shoot myself in the foot and have to learn how to fix it then have the compiler shoot me in the foot and for me to have to track that down in the compiled output.

What would be the footgun in this case though?

mimbrown commented 4 months ago

What would be the footgun in this case though?

I don't think it's impossible that someone could be intentionally referencing a signal outside the derived scope and not expect it to be tracked, and if the compiler "fixes" that it could be really surprising. I can't come up with a specific case in this situation, but it feels to me like it crosses some sort of fundamental line of things the compiler shouldn't do. I think a warning would be a lot more appropriate.

Rich-Harris commented 4 months ago

$effect takes a function and reacts to changes when signals inside of that function are read, which means this exact same problem you noted already exists in $effect

good point!

If your takeaway @Rich-Harris is that $effect should be limited to only accept inline function expressions, then I guess I'll accept that as an unintended consequence of my arguments.

😈

State referenced in its own scope will never update. Did you mean to reference it inside a closure?

This warning is a hangover from before the age of proxies — at one stage, foo.bar += 1 wouldn't do anything if it didn't happen in the same scope where foo was declared as $state. Now that foo is a proxy, and assigning to bar will update a behind-the-scenes signal, that warning is quite unhelpful since it's actually quite reasonable to do this sort of thing:

const ticker = $state({ current: 'tick' });

alternate(ticker); // this causes the warning

function alternate(ticker) {
  $effect(() => {
    const interval = setInterval(() => {
      ticker.current = ticker.current === 'tick' ? 'tock' : 'tick';
    }, 1000);
    return () => clearInterval(interval);
  });
}

As such, I had been operating under the assumption that we were about to get round to removing this warning and therefore couldn't rely on it to prevent the sort of bug we're discussing.

However! It just occurred to me that we could keep the warning for state that is reassigned rather than mutated — i.e. you'd still get it if you had this somewhere...

ticker = { current: 'lolwut' };

...because that would 'break the link' between {ticker.current} in the template and ticker.current inside alternate, but otherwise the warning wouldn't appear. This might actually satisfy my footgun concerns.

GIven this does work as you would expected:

- let one_reversed = $derived.fn(createReverser(one));
+ let one_reversed = $derived.fn((prev) => createReverser(one)(prev));

What we don't want is for the reverser to be created anew when one and two are merely mutated. We also want to minimise the amount of compiler magic, just because it becomes hard to keep track of how your code is being mucked about with. Though again, if we can get the right warnings in place then maybe this is a non-issue.

Could the scope of this PR maybe be reduced in the short term to not include the prev parameter to allow for just using regular functions pending further discussion of the best way to implement it in the meantime, or is it considered blocking?

That would be my preference, I think the prev thing has derailed us a bit.

Thinking on it more, compute would be my leading choice at this point given it will be familiar/consistent with the API for people coming from Vue, Solid, and Angular land.

'derive' and 'compute' are essentially synonyms in this context, so I think that would be rather confusing. But I retract my block proposal since that only really makes sense if you enforce function expression arguments, and I'm much less convinced we need that if the footgun issue is solved.

Summary:

[ ] let's remove the prev stuff from this PR
[ ] let's fix the overzealous state reference warning
[ ] let's keep bikeshedding the name

Rich-Harris commented 4 months ago

Lest my previous comment be interpreted as a green light for this feature: I do still have reservations. The most basic one is 'having multiple ways to do the same thing is almost always bad', though if it's sufficiently inconvenient to only have one then this isn't overriding.

The thing I'm more concerned about is closing off future avenues. I wrote this above...

Ha - I was also thinking about that stuff, but from a different angle. Namely that the smaller the API surface area today, the more freedom we'll have in future. Part of the reason I'm so resistant to new API in general is that I've often been painted into a corner by convenience APIs that weren't truly necessary.

...in response to this:

One of things that I'm worried about is a future in where we have the forking of state. For example, a pending state tree for async/suspenseful transitioning UI that has not yet been applied.

Rather than continuing to talk cryptically I may as well share something concrete about our plans, since it applies to this conversation. We'd like to introduce a notion of asynchronous state and derivations, coupled with a Suspense-like mechanism for coordination. We haven't yet explored exactly what this would entail — it's a post-5.0 goal — but one design idea I very much like is that you'd mark a piece of state as async just by awaiting it:

let post = $derived(await fetch_post(page.params.id));

Maybe it wouldn't look like that, and it would be something like $derived.async instead, but await feels nicest to me and is likely to be the starting point for our explorations.

Here's the rub: if we had $derived.fn, it wouldn't be possible to determine via static analysis that something was an async derived. So if we were to introduce it, purely for the (let's me honest, fairly modest) convenience of being able to write this...

let foo = $derived.fn(() => {
  if (a) return b;
  return c;
});

...instead of this...

let foo = $derived(get_foo());

function get_foo() {
  if (a) return b;
  return c;
}

...then there's a real danger that we could force worse ergonomics for a potentially far more valuable feature.

And that's a consequence that we can actually foresee — there are always ones that we can't. That's why it's so important to be conservative about new APIs.

mimbrown commented 4 months ago

The most basic one is 'having multiple ways to do the same thing is almost always bad', though if it's sufficiently inconvenient to only have one then this isn't overriding.

I and some others from discord land would argue that the solution to this is actually to disallow the current expression-only syntax and just always pass a function:

// From
const double = $derived(count * 2);
// To
const double = $derived(() => count * 2);

Why

Every other framework passes a function (useMemo, computed). Svelte is not trying to be like every other framework, which we all really appreciate, but there is a cost to doing things differently from everyone else. We're a compiler and we can doesn't always mean we should.
To the uninitiated, the expression passed to $derived looks (according to the rules of JS) like it's going to be evaluated exactly once. But it won't be, and that could be surprising. Forcing them to pass a function appropriately signals (pun intended) that this thing could be evaluated multiple times. Svelte 5 has been favoring explicitness over magic in certain cases, and this is one where I think explicitness is actually better. I more or less know what it's doing under the hood, and I still look at certain $derived expressions and think huh that must be a bug... oh no wait, that'll work cause it gets turned into a function.
I heard (don't know if this is correct) that expressions were originally favored over functions to discourage side effects. If that's the real reason, I would argue dev runtime checks should be able to detect signal writes inside a derived computation and throw appropriate errors, which should catch the majority of bad usage, and expressions don't prevent side effects anyway.

Concrete example: there was recently a discussion on discord about whether changes to a nested object created through $derived would properly react. The answer was, no, it won't, because the derived computation fully overwrites itself every time it re-runs. I think part of the confusion is that the expression inside the $derived looks like it just runs once, whereas a function expression would make it more obvious that it's being overwritten every time the recompute happens.

Thiagolino8 commented 4 months ago

I second to disallowing the current expression-only syntax and just always pass a function This would also fix the problem of $derived currently having two different behaviors

aradalvand commented 4 months ago

Also agree with only allowing $derived(() => ...); the 6 extra characters that () => entails is worth eliminating all the potential confusion that comes with the current approach, and also eliminating the need for $derived.fn.

Rich-Harris commented 4 months ago

This is at least partly an empirical question: which, in practice, is the more common case?

Right now we don't have a ton of good data. One potentially instructive case is @dummdidumm's POC of a Svelte 5 rewrite of learn.svelte.dev. It contains 15 $derived values. Of those:

11 are straightforward expressions, that are better off without the () =>
One is a weird IIFE that would unquestionably be nicer if you could just pass a function
One is a call expression that would arguably be better off with an inline anonymous function
One is a ternary expression. Some people might prefer to write it like this — it's a matter of personal preference:
```
const actions = $derived(() => {
  if (!can_remove) return [];
  return [...];
});
```

One is another question of preference — some people might prefer this:

const actions = $derived((() => {
  const result = [];

  if (can_create.file) {
    result.push({...});
  }

  // ...

  return result;
});

So there are two cases where it would be nicer to have $derived(() => ...), two where it's borderline, and 11 where expressions are nicer. That roughly tracks with my own observations.

In Svelte 4 we had this, and no-one seemed to think it was weird even though there's no function body:

$: doubled = count * 2;

In Svelte 5 we're asking people to write a bit more code in exchange for various benefits:

let doubled = $derived(count * 2);

Some people already feel put out by that, though mostly people are okay with it. But if we add more punctuation, the difference becomes pretty stark:

// Svelte 4 
$: doubled = count * 2;

// Svelte 5
let doubled = $derived(() => count * 2);

The intent:boilerplate ratio really starts to suffer.

Every other framework passes a function (useMemo, computed)

There are lots of things that other frameworks do out of necessity, like count.value and doubled.value, or putting everything behind function calls like setCount(count() + 1), or requiring top-level sibling elements to be inside a <>, or whatever. There's only so far you can take the 'there is a cost to doing things differently from everyone else' logic.

I heard (don't know if this is correct) that expressions were originally favored over functions to discourage side effects

This is definitely part of it, yeah. We've noticed that people are often confused about what should use $derived vs what should use $effect. $derived(expression) is one way to guide people towards the right mental model; if $derived and $effect are very similar it exacerbates the confusion, because it very much looks like $derived is another place to do work.

Yes, we can forbid writes to state. But that's only a small fraction of 'side effects'.

Finally, to reiterate, this could complicate our future plans to introduce async state. One way that could look is like this:

let a = $state(await get_some_state());
let b = $derived(a + await get_some_other_value());

If we went with the function form, we would likely have to introduce a new rune for async state and deriveds (e.g. $derived.async), and instead of just adding an unnecessary () => we'd have to add an async keyword as well:

-let a = $state(await get_some_state());
-let b = $derived(a + await get_some_other_value());
+let a = $state.async(get_some_state());
+let b = $derived.async(async () => a + await get_some_other_value());

That .async(async () => is sinfully ugly.

Or — since it's a bit confusing that a inside that function is the result of awaiting get_some_state() — we could do it like this instead...

-let a = $state(await get_some_state());
-let b = $derived(a + await get_some_other_value());
+let a = await $state(get_some_state());
+let b = await $derived(async () => a + await get_some_other_value());

Both, to me, feel much worse.

So I'm still of the view that $derived(expression) is preferable, but I don't think there are any slam dunk arguments on either side here. I just want to make sure that we fully consider the ramifications of a change.

gterras commented 4 months ago

we could do it like this instead

+let a = await $state(get_some_state());
+let b = await $derived(async () => a + await get_some_other_value());

I hope get_some_state and get_some_other_value will be constants here else it's a bit scary.

Since runes are magical could they also operate on the async syntax ?

I have trouble picturing a case where you don't just want everything to be awaited inside your rune. Taking actual advantage of the promise mechanism inside a rune feels completely pointless unless I'm missing something.

So I guess what most people want when dealing with async reactivity is just pretend it's not async :

+let a = $state.await(get_some_state());
+let b = $derived.await(() => a + get_some_other_value());

At this point you could also strip the method :

+let a = $state(get_some_state());
+let b = $derived(() => a + get_some_other_value());

I would be ok with that as long as the rule is clear and simple like $state and $derived automatically await everything you throw inside it.

mimbrown commented 4 months ago

As far as the async goes, I would say it's also pretty monstrous if you wanted the body of the derived inline:

let userId = $state(1);

const userData = $derived(await (async () => {
  const response = await fetch(`/user/${userId}`);
  return response.json();
})());

For what it's worth (not much, I know), I think I actually like await $state(promise) better than $state(await promise), but that's neither here nor there. When I'm developing I would want some reassurance that waterfalls are properly avoided, which sounds hard. Good luck with that!

IMO, calculations like $derived(count * 2) are perfectly fine. What gets me is when $derived is used to "unwrap" a wrapped signal. I think the reason is because, in all other cases, passing something into a function loses its reactivity, and this is one of those fundamental rules that's really important to understand.

// some wrapper state
let foo = $state({
  value: 1
});

const bar = normalFunction(foo.value); // reactivity lost. `normalFunction` cannot response to changes to `foo.value`.

But with $derived, reactivity is actually gained, even though it looks like you're passing the signal into a function.

// some wrapper state
let foo = $state({
  value: 1
});

const bar = $derived(foo.value); // `bar` will react to changes in `foo.value`

That's the cognitive dissonance I feel that the $: label syntax didn't have (which I'm not one of the ones longing for a return to those days, btw).

Rich-Harris commented 4 months ago

At this point you could also strip the method :

Afraid this is a non-starter — the compiler would need to insert await keywords in front of everything that it couldn't statically determine wasn't a promise, which would add a ton of overhead and complexity, and in any case we certainly don't want to prevent people from doing promise = $derived(whatever()).

I think I actually like await $state(promise) better than $state(await promise)

In some ways so do I. The wrinkle is that you can't then do things like this:

let b = $derived(a + await get_some_other_value());

(The flip side is that a syntactical restriction like that would prevent errors like $derived(await x() + await y()), which — since dependencies must be read synchronously in order to be bound to the derived, unless we do some trickery with AsyncContext which is still at stage 2 and therefore off-limits for the next year or so — would produce a derived that updated when x() changed but not when y() changed. Though maybe we can prevent those cases anyway, the relevant static analysis should be pretty straightforward.)

That's the cognitive dissonance I feel that the $: label syntax didn't have (which I'm not one of the ones longing for a return to those days, btw).

To what extent do you think this could be mitigated by compiler warnings? For primitive state values we already warn:

For non-primitives the static analysis gets a little hairier (and in cross-module cases, impossible without a complete rethink of our compiler architecture) but we could probably get quite far on a best-effort basis. Or does the visual similarity between normalFunction(...) and $derived(...) mean that no amount of compiler nudging will fix it?

mimbrown commented 4 months ago

To what extent do you think this could be mitigated by compiler warnings?

Compiler warnings are definitely helpful, but in this case the compiler warning is only relevant by being absent. And while I definitely pay attention when the compiler does warn me about something, I've never once thought "that looks wrong but it's probably fine because it's not underlined."

(Feature request: compiler affirmations. Green underline with a hoverover message, "Well done, this is lookin' real clean!")

So... I would still have a slight preference for functions. Others probably don't. What we all wouldn't give for do expressions. I think I've come to the end of my perspective on it, and if expressions make other things possible in the end then that will probably win out. I personally like it when I can pretty quickly see "through" the code I'm writing to what it's roughly going to be compiled to, and I get scared when I don't understand what the compiler is going to do to it. This is not one of those cases, and with time probably it becomes more natural.

eddiemcconkie commented 4 months ago

I would be ok with that as long as the rule is clear and simple like $state and $derived automatically await everything you throw inside it.

@gterras what if you want the state to be a promise though? If it automatically awaited promises, you wouldn't be able to do something like this where you have a promise as state, and you handle the awaiting in your template

mimbrown commented 4 months ago

Ok I know I said I was done but... just got tripped up with this again. I was playing around with destructuring from a $derived because I just found out about it (pretty cool that this works).

But in the above repl, I actually originally had this:

-const counter = createCounter();
-let { count, increment } = $derived(counter);
+let { count, increment } = $derived(createCounter());

This looks like it should be semantically identical according to all the rules of JS, but it doesn't work because createCounter will get called every time. Which would have been obvious if I had actually written $derived(() => createCounter()).

So again, it's this thing where I have to suppress all my instincts about how things work when working with $derived that's making it feel like it's more confusing than helpful.

mjadobson commented 4 months ago

Ok I know I said I was done but... just got tripped up with this again. I was playing around with destructuring from a $derived because I just found out about it (pretty cool that this works).

But in the above repl, I actually originally had this:
-const counter = createCounter();
-let { count, increment } = $derived(counter);
+let { count, increment } = $derived(createCounter());
This looks like it should be semantically identical according to all the rules of JS, but it doesn't work because createCounter will get called every time. Which would have been obvious if I had actually written $derived(() => createCounter()).

So again, it's this thing where I have to suppress all my instincts about how things work when working with $derived that's making it feel like it's more confusing than helpful.

I’ve been tripped up by precisely this use case as well. It definitely reinforced to me that runes are compiler macros rather than functions, despite their appearance! I found that reactive labels were less confusing in this regard, but both obviously break JavaScript semantics.

Rich-Harris commented 4 months ago

(pretty cool that this works)

Interesting — I have almost the opposite reaction, namely that it seems vaguely surprising and weird that it works. I almost wonder if we should prevent destructuring of deriveds to prevent that sort of confusion from arising.

(Though separately, we're considering changes that would make destructuring of deriveds work better, such that if you have let { a, b } = $derived(...) and a changes, an effect referencing only b wouldn't refire.)

Short of making those changes (which might be a bit much), here's what I think we should do: if you create state inside a derived expression (such as createCounter()), we yell at you. In simple cases we could even determine that at compile time, on a best-effort basis. An error like that would prevent this category of mistake from happening.

mimbrown commented 4 months ago

if you create state inside a derived expression (such as createCounter()), we yell at you.

For me, that doesn't solve the problem at the principled level, but practically it sure is helpful. In the same vein should it be a runtime warning if no signals are read during a derived computation? (Probably not an error since there are rare cases where it's intentional.)

Rich-Harris commented 4 months ago

I'd be curious to know what those intentional cases are. It seems like it'd be unintended in 99% of cases. Perhaps we could make it an error for now, and see what happens — we have the luxury of doing that sort of thing as long as we're in pre-release

mimbrown commented 4 months ago

There's potential anytime there's something that's unknown at compile time but static at runtime, like a browser support check or feature flag or something like that:

let unsaved_changes = $state([]);

const is_save_button_visible = $derived(
  browser_supports_file_system_access // never changes, not a signal
    ? unsaved_changes.length > 0
    : false // in this path we never read from any signal, but it's valid
);

Rich-Harris commented 4 months ago

We spent a healthy chunk of time discussing this issue on Friday, and reached the following conclusions:

adding a variant that takes a function shouldn't prevent us from implementing async state, when the time comes
we want to keep $derived(expression)
therefore the variant should be a new sub-rune. we settled on $derived.call(fn) as the name
we don't want to pass the previous value into the function

Not-Jayden commented 4 months ago

I know this is probably too little too late to flag this, but I intentionally avoided suggesting call as the method name as it conflicts with the reserved Function.prototype.call() method for functions in JS.

e.g. in theory you would expect

$derived.call(null, count * 2);

to be equivalent to:

$derived(count * 2);

While I don't think its a huge deal, it is breaking JS conventions a little.

tcc-sejohnson commented 4 months ago

IMO the above is a nonissue; $derived is already breaking function semantics in that the expression inside of it is reevaluated when dependencies change, rather than being executed and “passed into” the “function”. It’s pretty clear beginning-to-end that runes are not functions and don’t hold to JavaScript function semantics.

eltigerchino commented 4 months ago

Furthermore, it’s been mentioned before that runes are meant to be similar to the import() keyword. So, the conventional function rules don’t apply:

The $derived() syntax is a function-like expression.

The $derived() call is a syntax that closely resembles a function call, but derived itself is a keyword, not a function. You cannot alias it like const myDerived = $derived, which will throw a SyntaxError.

Blatantly plagiarised from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import#syntax

lucaesposto commented 4 months ago

How about $derived.fromFunction or $derived.fromMethod, or something similar?

Rich-Harris commented 4 months ago

good point @Not-Jayden — I've opened #10334 with a suggestion

sveltejs / svelte

chore: add $derived.call rune #10240

`$derived.fn`

🦋 Changeset detected

Another approach

Why