fix: avoid disconnecting deriveds that are still active

[trueadm]

Fixes #13284.

This was an interesting bug. This was historically dealt with by having multiple effects for this – we'd have one for the condition and one for the actual handling of the branching. That's because the dependencies for effects are only realised after we execute the inner function. Normally this is fine, but if you synchronously add/remove effects during this, it can cause the current effect to not correctly attach its current dependencies.

One way of tackling this is to instead wrap the condition in a derived, which are more lightweight than effects. I originally did that in this PR but the instead adjusted the logic for determining how we disconnect a derived reaction – and instead we avoid disconnection if the derived is in our active reaction's dependencies.

[changeset-bot]

🦋 Changeset detected

Latest commit: f63e493

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

[trueadm]

I also applied it to key block as it looks to work exactly like the if block logic does.

[Rich-Harris]

Not sure I understand how pausing a branch affects the block's dependencies — isn't that a bug?

[trueadm]

Not sure I understand how pausing a branch affects the block's dependencies — isn't that a bug?

Nope. It's by design. Let's take:

State A: true
State B: false

Derived C = B

If Effect: A || C
Inner Effect: {A} {C}

The If Effect starts by not having a dependency on C, only the Inner Effect does. When we switch A off, the If Effect then reads A and C, but its not store in its dependencies till the end, in the mean time it also destroys Inner Effect which is depending on A and C. Now given the fact that we haven't yet associated the If Effect with C, the removal of the Inner Effect will detach Derived C from State B. When we associate If Effect with C, it will be too late, we will already have removed it from B.

I can see if there's an alternative solution for this.

[trueadm]

Okay, so I found an alternative angle – we avoid disconnecting derived signals that are in the the current new_deps. This seems like a less expensive change.