From fc6aedd14eda03e6f14100741f401b9614850830 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Sun, 16 Nov 2025 20:56:57 +0100 Subject: [PATCH 1/8] chore: failing test for derived + fork + block --- .../samples/fork-derived-value/_config.js | 19 +++++++++++++++ .../samples/fork-derived-value/main.svelte | 23 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/fork-derived-value/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/fork-derived-value/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/fork-derived-value/_config.js b/packages/svelte/tests/runtime-runes/samples/fork-derived-value/_config.js new file mode 100644 index 000000000000..7b0a0ab06bf7 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/fork-derived-value/_config.js @@ -0,0 +1,19 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [fork, update] = target.querySelectorAll('button'); + + flushSync(() => { + fork.click(); + }); + flushSync(() => { + update.click(); + }); + + const p = target.querySelector('p'); + + assert.equal(p?.textContent, 'one'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/fork-derived-value/main.svelte b/packages/svelte/tests/runtime-runes/samples/fork-derived-value/main.svelte new file mode 100644 index 000000000000..33946f08a189 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/fork-derived-value/main.svelte @@ -0,0 +1,23 @@ + + + + + + +{#if count === 1} +

one

+{/if} \ No newline at end of file From 4eea2b01dd395b9b696dadd36c295e8736ca08c4 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 17 Nov 2025 10:35:40 +0100 Subject: [PATCH 2/8] fix: don't set derived values during time travel --- .../src/internal/client/reactivity/deriveds.js | 14 ++++++++++---- packages/svelte/src/internal/client/runtime.js | 6 ++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 7e6f3c6f604f..e3434bfbcbb8 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -353,9 +353,15 @@ export function update_derived(derived) { var value = execute_derived(derived); if (!derived.equals(value)) { - // TODO can we avoid setting `derived.v` when `batch_values !== null`, - // without causing the value to be stale later? - derived.v = value; + // if we have batch_values we don't actually set the value of the derived + // otherwise blocks that depends on that derived will not be considered dirty + // if the derived changes in a fork AND in a subsequent fork/normal state update + // IF we are in a tracking context the derived value of the derived will be set + // in the batch_values map thus updating it for this batch otherwise it will just be updated + // again during the `get` call + if (batch_values === null) { + derived.v = value; + } derived.wv = increment_write_version(); } @@ -371,7 +377,7 @@ export function update_derived(derived) { // only cache the value if we're in a tracking context, otherwise we won't // clear the cache in `mark_reactions` when dependencies are updated if (effect_tracking()) { - batch_values.set(derived, derived.v); + batch_values.set(derived, value); } } else { var status = (derived.f & CONNECTED) === 0 ? MAYBE_DIRTY : CLEAN; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 258f6962fa81..f14e74306333 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -614,6 +614,7 @@ export function get(signal) { } else if (is_derived) { derived = /** @type {Derived} */ (signal); + // if we already have a batched value don't bother updating it if (batch_values?.has(derived)) { return batch_values.get(derived); } @@ -625,6 +626,11 @@ export function get(signal) { if (is_updating_effect && effect_tracking() && (derived.f & CONNECTED) === 0) { reconnect(derived); } + + // we need to check again because we could've just update `batch_values` inside `update_derived` + if (batch_values?.has(derived)) { + return batch_values.get(derived); + } } else if (batch_values?.has(signal)) { return batch_values.get(signal); } From abf408b4abaab7f196c6af659a0957877ec55388 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 17 Nov 2025 10:42:47 +0100 Subject: [PATCH 3/8] fix: skip no async --- .../tests/runtime-runes/samples/fork-derived-value/_config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/svelte/tests/runtime-runes/samples/fork-derived-value/_config.js b/packages/svelte/tests/runtime-runes/samples/fork-derived-value/_config.js index 7b0a0ab06bf7..0635db75014c 100644 --- a/packages/svelte/tests/runtime-runes/samples/fork-derived-value/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/fork-derived-value/_config.js @@ -2,6 +2,7 @@ import { flushSync } from 'svelte'; import { test } from '../../test'; export default test({ + skip_no_async: true, async test({ assert, target }) { const [fork, update] = target.querySelectorAll('button'); From b671b6679e8611054b27111d34f974a5665c1bbc Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 17 Nov 2025 23:23:13 +0100 Subject: [PATCH 4/8] fix: only set `derived.v` outside a fork --- packages/svelte/src/internal/client/reactivity/deriveds.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index e3434bfbcbb8..6ef2a052acf5 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -358,8 +358,9 @@ export function update_derived(derived) { // if the derived changes in a fork AND in a subsequent fork/normal state update // IF we are in a tracking context the derived value of the derived will be set // in the batch_values map thus updating it for this batch otherwise it will just be updated - // again during the `get` call - if (batch_values === null) { + // again during the `get` call we also prevent setting the value if we are in a fork + // this will lead to over executing of the derived but would lead to correct values + if (batch_values === null && current_batch?.is_fork !== true) { derived.v = value; } derived.wv = increment_write_version(); From 0610001ff1cbcc5ebca2c50251e95b7e720a08d8 Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Wed, 19 Nov 2025 21:50:10 +0100 Subject: [PATCH 5/8] chore: simplify Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- packages/svelte/src/internal/client/reactivity/deriveds.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index c2e4c3f26086..dd95e3a20969 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -363,7 +363,7 @@ export function update_derived(derived) { // in the batch_values map thus updating it for this batch otherwise it will just be updated // again during the `get` call we also prevent setting the value if we are in a fork // this will lead to over executing of the derived but would lead to correct values - if (batch_values === null && current_batch?.is_fork !== true) { + if (!current_batch?.is_fork) { derived.v = value; } derived.wv = increment_write_version(); From 263b0eacaeb76afa0901b7c60aa0323d6bc0c5e6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 19 Nov 2025 18:02:15 -0500 Subject: [PATCH 6/8] tidy up test a bit (missing text makes things confusing in the sandbox) --- .../samples/fork-derived-value/main.svelte | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/packages/svelte/tests/runtime-runes/samples/fork-derived-value/main.svelte b/packages/svelte/tests/runtime-runes/samples/fork-derived-value/main.svelte index 33946f08a189..06e0f1f264ad 100644 --- a/packages/svelte/tests/runtime-runes/samples/fork-derived-value/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/fork-derived-value/main.svelte @@ -1,23 +1,20 @@ - + }); +}}>fork - +}}>update {#if count === 1}

one

-{/if} \ No newline at end of file +{/if} From 3deb72931fa9a1d83c083b24a8cc6fdf552985d2 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 19 Nov 2025 18:04:49 -0500 Subject: [PATCH 7/8] update comment --- .../src/internal/client/reactivity/deriveds.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index dd95e3a20969..39e02be7649e 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -356,16 +356,14 @@ export function update_derived(derived) { var value = execute_derived(derived); if (!derived.equals(value)) { - // if we have batch_values we don't actually set the value of the derived - // otherwise blocks that depends on that derived will not be considered dirty - // if the derived changes in a fork AND in a subsequent fork/normal state update - // IF we are in a tracking context the derived value of the derived will be set - // in the batch_values map thus updating it for this batch otherwise it will just be updated - // again during the `get` call we also prevent setting the value if we are in a fork - // this will lead to over executing of the derived but would lead to correct values + // in a fork, we don't update the underlying value, just `batch_values`. + // the underlying value will be updated when the fork is committed. + // otherwise, the next time we get here after a 'real world' state + // change, `derived.equals` may incorrectly return `true` if (!current_batch?.is_fork) { derived.v = value; } + derived.wv = increment_write_version(); } From e0165e2584ddb26dae1f1abb005d1e69fe4cb1ec Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 19 Nov 2025 18:09:44 -0500 Subject: [PATCH 8/8] tweak --- packages/svelte/src/internal/client/runtime.js | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index f14e74306333..5ece0d79b6b8 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -611,14 +611,9 @@ export function get(signal) { return value; } - } else if (is_derived) { + } else if (is_derived && !batch_values?.has(signal)) { derived = /** @type {Derived} */ (signal); - // if we already have a batched value don't bother updating it - if (batch_values?.has(derived)) { - return batch_values.get(derived); - } - if (is_dirty(derived)) { update_derived(derived); } @@ -626,12 +621,9 @@ export function get(signal) { if (is_updating_effect && effect_tracking() && (derived.f & CONNECTED) === 0) { reconnect(derived); } + } - // we need to check again because we could've just update `batch_values` inside `update_derived` - if (batch_values?.has(derived)) { - return batch_values.get(derived); - } - } else if (batch_values?.has(signal)) { + if (batch_values?.has(signal)) { return batch_values.get(signal); }