derived value is not updating correctly

[days-later]

Describe the bug

the derived value depends on a class field of a newly created class.
when this derived value has a fallback value (like "v || '-'") it doesnt update correctly. removing the fallback seems to fix the issue.

Reproduction

https://svelte.dev/playground/dec1b43d47eb4c3aa729291f91ab8a39?version=5.28.2

Logs

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (12) x64 AMD Ryzen 5 5600X 6-Core Processor
    Memory: 21.19 GB / 31.91 GB
  Binaries:
    Node: 22.12.0 - ~\AppData\Local\fnm_multishells\15692_1745479128899\node.EXE
    Yarn: 1.22.22 - ~\AppData\Local\fnm_multishells\15692_1745479128899\yarn.CMD
    npm: 11.3.0 - ~\AppData\Local\fnm_multishells\15692_1745479128899\npm.CMD
    pnpm: 10.7.0 - ~\AppData\Local\fnm_multishells\15692_1745479128899\pnpm.CMD
  Browsers:
    Edge: Chromium (133.0.3065.82)
    Internet Explorer: 11.0.22621.3527

Severity

annoyance

[thes01]

Weird that it seems to work when you replace || with ??

[paoloricciuti]

Weird that it seems to work when you replace || with ??

The reason why this works is because this.c1?.v || '-' actually returns twice the same value when v is 0 and that's the root issue.

Basically: the value of the derived is - before c1 is defined in the set function and it's also - after c1 is defined...this means that the template_effect that updates the text in the dom is not executed and it never register a dependency on v so when it changes it doesn't re-execute.

Gonna explore this a bit more but it's hard to wrap your head around.

[brunnerh]

The ultimate value of the derived should not matter because the logic has to go through evaluating the c1/v source either way, so it should be registered as a dependency. ?? behaving correctly also points to something weird going on.

[7nik]

It works on 7958eb7 which is right before #15348

[7nik]

I guess this is the same bug. Though it doesn't work on 7958eb7 too (but the output is slightly different).

[paoloricciuti]

The ultimate value of the derived should not matter because the logic has to go through evaluating the c1/v source either way, so it should be registered as a dependency. ?? behaving correctly also points to something weird going on.

Yeah the underlying problem is that v doesn't have the derived in the list of reactions but I'm still trying to figure out why. 😄

[paoloricciuti]

So a bit more exploration and the problem is that skip_reaction is true when running the derived because it's UNOWNED there's no active_reaction before and this line to check there was no parent was removed in #15232

Adding the line back obviously is a no-go as it make that test fail. I'll try to figure out if there's another solution but it's getting late lol

[paoloricciuti]

Mmm i can check if the parent is null directly when checking if we should add the reaction and that make those tests pass (not sure if it's correct tho)...however this test still fails #12075 but it doesn't really make sense to me: why should we avoid adding an unowned derived in the reaction array? That derived still needs to be updated no?

cc. @Rich-Harris (I know you are probably super busy right now...whenever you have time)

[paoloricciuti]

In-fact this tests fails

test.only('unowned deriveds are not added as reactions but trigger effects', () => {
    var count = state(0);

    function create_derived() {
        return derived(() => $.get(count) * 2);
    }

    return () => {
        let d = create_derived();

        const log: number[] = [];
        assert.equal($.get(d), 0);

        let destroy = effect_root(() => {
            render_effect(() => {
                log.push($.get(d));
            });
        });

        set(count, 1);
        assert.equal($.get(d), 2);
        assert.equal(log, [0, 2]);
        destroy();
    };
});

[dummdidumm]

When initializating the inner class' value with 1 instead of 0 it does work. I suspect the problem is that the template effect's dependencies don't get updated because the derived has no different value, and because of the "don't add unowned dependencies but flush effects" logic it breaks. The solution is to somehow make the effect flush / pick up the dependencies.

[paoloricciuti]

When initializating the inner class' value with 1 instead of 0 it does work. I suspect the problem is that the template effect's dependencies don't get updated because the derived has no different value, and because of the "don't add unowned dependencies but flush effects" logic it breaks. The solution is to somehow make the effect flush / pick up the dependencies.

Yeah it's exactly that problem...but I honestly don't get why we shouldn't add unowned dependencies

[dummdidumm]

Likely because of memory leaks - you have no way of knowing when the unowned derived is "gone" and ripe for GC otherwise

[paoloricciuti]

Likely because of memory leaks - you have no way of knowing when the unowned derived is "gone" and ripe for GC otherwise

Mmm i have the feeling the constraint here is too heavy, maybe we should warn about it but still add the dependency? Have unowned derived not trigger effects seems like a very bad thing no?

[timephy]

Please just ignore if irrelevant, just wanted to leave these findings.

Here || is problematic, in #15853 ?? is problematic.
Both seem to be fixed by using a getter instead of using $derived.