fix: reconnected deep derived signals to graph

[trueadm]

Fixes #12330.

[changeset-bot]

🦋 Changeset detected

Latest commit: c665ddf

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]

Huh — should be happening here. It definitely doesn't need to happen in both places

svelte/packages/svelte/src/internal/client/runtime.js

Lines 775 to 792 in e8c3729

	if ((flags & DISCONNECTED) !== 0) {
	// reconnect to the graph
	deps = derived.deps;
	if (deps !== null) {
	for (var i = 0; i < deps.length; i++) {
	var dep = deps[i];
	var reactions = dep.reactions;
	if (reactions === null) {
	dep.reactions = [derived];
	} else if (!reactions.includes(derived)) {
	reactions.push(derived);
	}
	}
	}
	derived.f ^= DISCONNECTED;
	}

[trueadm]

@Rich-Harris I know it's done there, but that logic never gets applied to this signal because if there are multiple disconnected signals, then only the outer signal gets called via get (which the logic you referenced above happens). There isn't a get on the inner disconnected signal, so that inner signal is not connected to the graph again.

[Rich-Harris]

Why isn't the outer derived getting its dependencies? Aren't they all marked MAYBE_DIRTY upon disconnection?

[trueadm]

@Rich-Harris I added a test case.

[Rich-Harris]

weirds me out a bit that we need to call this in two places — it feels like there should be a single choke point where this work can happen, and it feels weird to have this sort of side-effect in check_dirtiness — but I haven't wrapped my head around what that would look like. made a few suggestions

[trueadm]

I haven't benchmarked this, but it kind of feels wasteful to do another O(n() lookup in each case of check_dirtiness. I wonder if we can avoid that like in the original commit.

[Rich-Harris]

It's not — we early-return as soon as we find a dirty dependency, so we have a choice:

1. do the separate O(n) thing
2. reconnect deriveds inside the loop in check_dirtiness, but that means we can no longer return early

Since reconnecting a disconnected derived is a fairly unusual case, number 2 turns out to be much slower — I validated this via the benchmarks in #12272.

Anyway, I figured out how to make it unnecessary to call reconnect_derived in two places — we just need to do it before the loop in check_dirtiness. Change incoming