Skip to content

Commit

Permalink
fix: avoid disconnecting deriveds that are still active (#13292)
Browse files Browse the repository at this point in the history
* fix: ensure the if block condition is wrapped in a derived

* add test

* tidy

* fix key block

* fix key block test

* alternative approach

* changeset

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

Co-authored-by: Rich Harris <rich.harris@vercel.com>

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
  • Loading branch information
trueadm and Rich-Harris committed Sep 17, 2024
1 parent 9d1394f commit 09527fd
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/dirty-worms-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: avoid disconnecting deriveds that are still active
9 changes: 8 additions & 1 deletion packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,14 @@ function remove_reaction(signal, dependency) {
}
// If the derived has no reactions, then we can disconnect it from the graph,
// allowing it to either reconnect in the future, or be GC'd by the VM.
if (reactions === null && (dependency.f & DERIVED) !== 0) {
if (
reactions === null &&
(dependency.f & DERIVED) !== 0 &&
// Destroying a child effect while updating a parent effect can cause a dependency to appear
// to be unused, when in fact it is used by the currently-updating parent. Checking `new_deps`
// allows us to skip the expensive work of disconnecting and immediately reconnecting it
(new_deps === null || !new_deps.includes(dependency))
) {
set_signal_status(dependency, MAYBE_DIRTY);
// If we are working with a derived that is owned by an effect, then mark it as being
// disconnected.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { flushSync } from 'svelte';
import { test } from '../../test';

export default test({
async test({ assert, target }) {
await Promise.resolve();

let [btn1] = target.querySelectorAll('button');

flushSync(() => {
btn1?.click();
});

assert.htmlEqual(
target.innerHTML,
`false\ntrue\n<button>Toggle</button>\nfirst:\nfalse\n<br>\nsecond:\ntrue`
);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<script>
let first = $state(true)
let second = $state(false)
let derivedSecond = $derived(second)
queueMicrotask(() => {
first = false
});
</script>

{first} {second}

<button onclick={() => {
second = true
}}>Toggle</button>

{#if first || derivedSecond}
first: {first}
<br />
second: {derivedSecond}
{/if}

0 comments on commit 09527fd

Please sign in to comment.