diff --git a/.changeset/light-rivers-jump.md b/.changeset/light-rivers-jump.md new file mode 100644 index 000000000000..9258401e8a86 --- /dev/null +++ b/.changeset/light-rivers-jump.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +Store deriveds old value before updating them for consistency with directly assigned sources when reading in teardown functions diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index e9cea0df3e64..5e4e08cc6537 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -15,7 +15,7 @@ import { import { equals, safe_equals } from './equality.js'; import * as e from '../errors.js'; import { destroy_effect } from './effects.js'; -import { inspect_effects, set_inspect_effects } from './sources.js'; +import { inspect_effects, old_values, set_inspect_effects } from './sources.js'; import { get_stack } from '../dev/tracing.js'; import { tracing_mode_flag } from '../../flags/index.js'; import { component_context } from '../context.js'; @@ -175,6 +175,27 @@ export function update_derived(derived) { var value = execute_derived(derived); if (!derived.equals(value)) { + // store old value before updating + // so that user_effect teardown functions + // can access the previous value. + // this is needed because derived updates happen early during + // effects dependency resolution (before cleanup), + // unlike direct state/derived updates, and this + // can happen also during template_effect execution + // that also happens before user_effect teardown. + // + // store old value only if not inside a teardown function + // because we only need to save the old values before + // the cleanup is triggered othewise accessing + // a derived during cleanup will return the incorrect + // value in case the derived wasn't in the deps of the effect, + // or the teardown was executed because the component was + // destroyed. + if (!is_destroying_effect) { + var old_value = derived.v; + old_values.set(derived, old_value); + } + derived.v = value; derived.wv = increment_write_version(); } diff --git a/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-no-setter/Component.svelte b/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-no-setter/Component.svelte new file mode 100644 index 000000000000..40d6b1c2fbb4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-no-setter/Component.svelte @@ -0,0 +1,11 @@ + + +
Count: {count}
diff --git a/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-no-setter/_config.js b/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-no-setter/_config.js new file mode 100644 index 000000000000..07ea5af153a6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-no-setter/_config.js @@ -0,0 +1,63 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, logs, target }) { + /** @type {HTMLButtonElement | null} */ + const increment_btn = target.querySelector('#increment'); + /** @type {HTMLButtonElement | null} */ + const toggle_btn = target.querySelector('#toggle'); + + for (const p of target.querySelectorAll('p')) { + assert.equal(p.innerHTML, 'Count: 0'); + } + + // Click increment: count=1 + flushSync(() => { + increment_btn?.click(); + }); + + for (const p of target.querySelectorAll('p')) { + assert.equal(p.innerHTML, 'Count: 1'); + } + + // Click increment again: count=2, components with count < 2 should unmount and log old values + flushSync(() => { + increment_btn?.click(); + }); + + for (const p of target.querySelectorAll('p')) { + assert.equal(p.innerHTML, 'Count: 2'); + } + + // Toggle show to hide components that depend on show + flushSync(() => { + toggle_btn?.click(); + }); + + // Should log old values during cleanup from the six components guarded by `count < 2`: + // 1. Component with bind: "1 true" + // 2. Component with spread: "1 true" + // 3. Component with normal props: "1 true" + // 4. Runes dynamic component with bind: "1 true" + // 5. Runes dynamic component with spread: "1 true" + // 6. Runes dynamic component with normal props: "1 true" + // Then from the four components guarded by `show`: + // 7. Component with bind (show): "2 true" (old value of checked) + // 8. Runes dynamic component with bind (show): "2 true" + // 9. Runes dynamic component with spread (show): "2 true" + // 10. Runes dynamic component with normal props (show): "2 true" + assert.deepEqual(logs, [ + '1 true', + '1 true', + '1 true', + '1 true', + '1 true', + '1 true', + '2 true', + '2 true', + '2 true', + '2 true' + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-no-setter/main.svelte b/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-no-setter/main.svelte new file mode 100644 index 000000000000..592c46ff429f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-no-setter/main.svelte @@ -0,0 +1,52 @@ + + + + + + +{#if count < 2} +Count: {count}
+ + + diff --git a/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-with-setter/_config.js b/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-with-setter/_config.js new file mode 100644 index 000000000000..07ea5af153a6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-with-setter/_config.js @@ -0,0 +1,63 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, logs, target }) { + /** @type {HTMLButtonElement | null} */ + const increment_btn = target.querySelector('#increment'); + /** @type {HTMLButtonElement | null} */ + const toggle_btn = target.querySelector('#toggle'); + + for (const p of target.querySelectorAll('p')) { + assert.equal(p.innerHTML, 'Count: 0'); + } + + // Click increment: count=1 + flushSync(() => { + increment_btn?.click(); + }); + + for (const p of target.querySelectorAll('p')) { + assert.equal(p.innerHTML, 'Count: 1'); + } + + // Click increment again: count=2, components with count < 2 should unmount and log old values + flushSync(() => { + increment_btn?.click(); + }); + + for (const p of target.querySelectorAll('p')) { + assert.equal(p.innerHTML, 'Count: 2'); + } + + // Toggle show to hide components that depend on show + flushSync(() => { + toggle_btn?.click(); + }); + + // Should log old values during cleanup from the six components guarded by `count < 2`: + // 1. Component with bind: "1 true" + // 2. Component with spread: "1 true" + // 3. Component with normal props: "1 true" + // 4. Runes dynamic component with bind: "1 true" + // 5. Runes dynamic component with spread: "1 true" + // 6. Runes dynamic component with normal props: "1 true" + // Then from the four components guarded by `show`: + // 7. Component with bind (show): "2 true" (old value of checked) + // 8. Runes dynamic component with bind (show): "2 true" + // 9. Runes dynamic component with spread (show): "2 true" + // 10. Runes dynamic component with normal props (show): "2 true" + assert.deepEqual(logs, [ + '1 true', + '1 true', + '1 true', + '1 true', + '1 true', + '1 true', + '2 true', + '2 true', + '2 true', + '2 true' + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-with-setter/main.svelte b/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-with-setter/main.svelte new file mode 100644 index 000000000000..592c46ff429f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bindable-props-cleanup-with-setter/main.svelte @@ -0,0 +1,52 @@ + + + + + + +{#if count < 2} +count: {count}
+derived_value: {derived_value}
diff --git a/packages/svelte/tests/runtime-runes/samples/regular-props-cleanup-old-value/Component.svelte b/packages/svelte/tests/runtime-runes/samples/regular-props-cleanup-old-value/Component.svelte new file mode 100644 index 000000000000..1a95f1edf55c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/regular-props-cleanup-old-value/Component.svelte @@ -0,0 +1,11 @@ + + +Count: {count}
diff --git a/packages/svelte/tests/runtime-runes/samples/regular-props-cleanup-old-value/_config.js b/packages/svelte/tests/runtime-runes/samples/regular-props-cleanup-old-value/_config.js new file mode 100644 index 000000000000..3d73bfd1a90c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/regular-props-cleanup-old-value/_config.js @@ -0,0 +1,49 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, logs, target }) { + /** @type {HTMLButtonElement | null} */ + const increment_btn = target.querySelector('#increment'); + /** @type {HTMLButtonElement | null} */ + const toggle_btn = target.querySelector('#toggle'); + + for (const p of target.querySelectorAll('p')) { + assert.equal(p.innerHTML, 'Count: 0'); + } + + // Click increment: count=1 + flushSync(() => { + increment_btn?.click(); + }); + + for (const p of target.querySelectorAll('p')) { + assert.equal(p.innerHTML, 'Count: 1'); + } + + // Click increment again: count=2, components with count < 2 should unmount and log old values + flushSync(() => { + increment_btn?.click(); + }); + + for (const p of target.querySelectorAll('p')) { + assert.equal(p.innerHTML, 'Count: 2'); + } + + // Toggle show to hide components that depend on show + flushSync(() => { + toggle_btn?.click(); + }); + + // Should log old values during cleanup from the four components guarded by `count < 2`: + // 1. Component with normal props: "1 true" + // 2. Component with spread: "1 true" + // 3. Runes dynamic component with normal props: "1 true" + // 4. Runes dynamic component with spread: "1 true" + // Then from the three components guarded by `show`: + // 5. Component with normal props (show): "2 true" (old value of checked) + // 6. Runes dynamic component with normal props (show): "2 true" + // 7. Runes dynamic component with spread (show): "2 true" + assert.deepEqual(logs, ['1 true', '1 true', '1 true', '1 true', '2 true', '2 true', '2 true']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/regular-props-cleanup-old-value/main.svelte b/packages/svelte/tests/runtime-runes/samples/regular-props-cleanup-old-value/main.svelte new file mode 100644 index 000000000000..8bc6abfa4d6b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/regular-props-cleanup-old-value/main.svelte @@ -0,0 +1,41 @@ + + + + + + +{#if count < 2} +