diff --git a/.changeset/mean-dryers-fix.md b/.changeset/mean-dryers-fix.md new file mode 100644 index 000000000000..96037f344005 --- /dev/null +++ b/.changeset/mean-dryers-fix.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: access last safe value of prop on unmount diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js index 4d09d9293fb2..6c2171785244 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -42,6 +42,9 @@ export function CallExpression(node, context) { e.bindable_invalid_location(node); } + // We need context in case the bound prop is stale + context.state.analysis.needs_context = true; + break; case '$host': diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index a4840ce4ebd0..cdd70f6c762e 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -24,4 +24,7 @@ export const EFFECT_HAS_DERIVED = 1 << 20; export const STATE_SYMBOL = Symbol('$state'); export const STATE_SYMBOL_METADATA = Symbol('$state metadata'); export const LEGACY_PROPS = Symbol('legacy props'); +export const TEARDOWN_PROPS = Symbol('teardown props'); export const LOADING_ATTR_SYMBOL = Symbol(''); + +export const CTX_DESTROYED = 1 << 1; diff --git a/packages/svelte/src/internal/client/context.js b/packages/svelte/src/internal/client/context.js index bd94d5ad8a19..2ef5314804ba 100644 --- a/packages/svelte/src/internal/client/context.js +++ b/packages/svelte/src/internal/client/context.js @@ -11,8 +11,10 @@ import { set_active_reaction, untrack } from './runtime.js'; -import { effect } from './reactivity/effects.js'; +import { effect, teardown } from './reactivity/effects.js'; import { legacy_mode_flag } from '../flags/index.js'; +import { CTX_DESTROYED, TEARDOWN_PROPS } from './constants.js'; +import { define_property } from '../shared/utils.js'; /** @type {ComponentContext | null} */ export let component_context = null; @@ -112,15 +114,17 @@ export function getAllContexts() { * @returns {void} */ export function push(props, runes = false, fn) { - component_context = { + var ctx = (component_context = { p: component_context, c: null, e: null, + f: 0, m: false, s: props, x: null, - l: null - }; + l: null, + tp: props + }); if (legacy_mode_flag && !runes) { component_context.l = { @@ -131,6 +135,31 @@ export function push(props, runes = false, fn) { }; } + teardown(() => { + if ((ctx.f & CTX_DESTROYED) !== 0) { + return; + } + // Mark the context as destroyed, so any derived props can use + // the latest known value before teardown + ctx.f ^= CTX_DESTROYED; + + // Only apply the latest known props before teardown in legacy mode + if (!is_runes(ctx)) { + var teardown_props = ctx.tp; + if (TEARDOWN_PROPS in props) { + props[TEARDOWN_PROPS] = teardown_props; + return; + } + // Apply the latest known props before teardown over existing props + for (var key in teardown_props) { + define_property(props, key, { + value: teardown_props[key], + configurable: true + }); + } + } + }); + if (DEV) { // component function component_context.function = fn; @@ -171,6 +200,13 @@ export function pop(component) { dev_current_component_function = context_stack_item.p?.function ?? null; } context_stack_item.m = true; + + // Only apply the latest known props before teardown in legacy mode + if (!is_runes(context_stack_item)) { + effect(() => { + context_stack_item.tp = { ...context_stack_item.s }; + }); + } } // Micro-optimization: Don't set .a above to the empty object // so it can be garbage-collected when the return here is unused @@ -178,8 +214,8 @@ export function pop(component) { } /** @returns {boolean} */ -export function is_runes() { - return !legacy_mode_flag || (component_context !== null && component_context.l === null); +export function is_runes(ctx = component_context) { + return !legacy_mode_flag || (ctx !== null && ctx.l === null); } /** diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index 5a3b30281f9f..4c4c4ce39c1a 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -1,4 +1,4 @@ -/** @import { Source } from './types.js' */ +/** @import { Derived, Source } from './types.js' */ import { DEV } from 'esm-env'; import { PROPS_IS_BINDABLE, @@ -10,27 +10,20 @@ import { import { get_descriptor, is_function } from '../../shared/utils.js'; import { mutable_source, set, source, update } from './sources.js'; import { derived, derived_safe_equal } from './deriveds.js'; -import { - active_effect, - get, - captured_signals, - set_active_effect, - untrack, - active_reaction, - set_active_reaction -} from '../runtime.js'; +import { get, captured_signals, untrack } from '../runtime.js'; import { safe_equals } from './equality.js'; import * as e from '../errors.js'; import { - BRANCH_EFFECT, + CTX_DESTROYED, LEGACY_DERIVED_PROP, LEGACY_PROPS, - ROOT_EFFECT, - STATE_SYMBOL + STATE_SYMBOL, + TEARDOWN_PROPS } from '../constants.js'; import { proxy } from '../proxy.js'; import { capture_store_binding } from './store.js'; import { legacy_mode_flag } from '../../flags/index.js'; +import { is_runes } from '../context.js'; /** * @param {((value?: number) => number)} fn @@ -186,6 +179,12 @@ const spread_props_handler = { } }, set(target, key, value) { + // If the spread props have been torn down, then replace the existing props with + // the stale props from the teardown + if (key === TEARDOWN_PROPS) { + target.props = [value]; + return true; + } let i = target.props.length; while (i--) { let p = target.props[i]; @@ -216,6 +215,9 @@ const spread_props_handler = { } }, has(target, key) { + if (key === TEARDOWN_PROPS) { + return true; + } // To prevent a false positive `is_entry_props` in the `prop` function if (key === STATE_SYMBOL || key === LEGACY_PROPS) return false; @@ -249,6 +251,14 @@ export function spread_props(...props) { return new Proxy({ props }, spread_props_handler); } +/** + * @param {Derived} signal + * @returns {boolean} + */ +function in_destroyed_context(signal) { + return signal.ctx !== null && (signal.ctx.f & CTX_DESTROYED) !== 0; +} + /** * This function is responsible for synchronizing a possibly bound prop with the inner component state. * It is used whenever the compiler sees that the component writes to the prop, or when it has a default prop_value. @@ -382,6 +392,11 @@ export function prop(props, key, flags, fallback) { return (inner_current_value.v = parent_value); }); + // Read the bindable prop eagerly to ensure it has a value + if (!is_runes() && bindable) { + get(current_value); + } + if (!immutable) current_value.equals = safe_equals; return function (/** @type {any} */ value, /** @type {boolean} */ mutation) { @@ -408,11 +423,20 @@ export function prop(props, key, flags, fallback) { if (fallback_used && fallback_value !== undefined) { fallback_value = new_value; } + if (in_destroyed_context(current_value)) { + return value; + } untrack(() => get(current_value)); // force a synchronisation immediately } return value; } + + // If the prop is read, we might need to return the stale value if component ctx has been destroyed + if (in_destroyed_context(current_value)) { + return current_value.v; + } + return get(current_value); }; } diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 7208ed77837e..fad0df7a266e 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -20,12 +20,11 @@ export type ComponentContext = { effect: null | Effect; reaction: null | Reaction; }>; + /** ctx flags */ + f: number; /** mounted */ m: boolean; - /** - * props — needed for legacy mode lifecycle functions, and for `createEventDispatcher` - * @deprecated remove in 6.0 - */ + /** props — needed for legacy mode lifecycle functions, for `createEventDispatcher` and teardown */ s: Record; /** * exports (and props, if `accessors: true`) — needed for `createEventDispatcher` @@ -53,6 +52,8 @@ export type ComponentContext = { /** This tracks whether `$:` statements have run in the current cycle, to ensure they only run once */ r2: Source; }; + /** teardown props */ + tp: Record; /** * dev mode only: the component function */ diff --git a/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-2/Component.svelte b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-2/Component.svelte new file mode 100644 index 000000000000..73347c4d7ff1 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-2/Component.svelte @@ -0,0 +1,11 @@ + + +{my_prop.foo} diff --git a/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-2/_config.js b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-2/_config.js new file mode 100644 index 000000000000..81005cf73760 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-2/_config.js @@ -0,0 +1,14 @@ +import { test } from '../../test'; +import { flushSync } from 'svelte'; + +export default test({ + async test({ assert, target, logs }) { + const [btn1] = target.querySelectorAll('button'); + + flushSync(() => { + btn1.click(); + }); + + assert.deepEqual(logs, ['bar']); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-2/main.svelte b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-2/main.svelte new file mode 100644 index 000000000000..f38b37fb7f7c --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-2/main.svelte @@ -0,0 +1,15 @@ + + + + +{#if value !== undefined} + +{/if} diff --git a/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-3/Component.svelte b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-3/Component.svelte new file mode 100644 index 000000000000..5bfb7771289d --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-3/Component.svelte @@ -0,0 +1,5 @@ + + + diff --git a/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-3/_config.js b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-3/_config.js new file mode 100644 index 000000000000..03a9822cdaf2 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-3/_config.js @@ -0,0 +1,11 @@ +import { test } from '../../test'; +import { flushSync } from 'svelte'; + +export default test({ + async test({ assert, target, logs }) { + const [btn1] = target.querySelectorAll('button'); + + btn1.click(); + flushSync(); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-3/main.svelte b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-3/main.svelte new file mode 100644 index 000000000000..980717bdb678 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access-3/main.svelte @@ -0,0 +1,17 @@ + + +{#if state} + {@const attributes = { title: state.title }} + +{/if} + diff --git a/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access/Component.svelte b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access/Component.svelte new file mode 100644 index 000000000000..00e12070f42f --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access/Component.svelte @@ -0,0 +1,14 @@ + + +

{count}

+ + diff --git a/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access/_config.js b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access/_config.js new file mode 100644 index 000000000000..7d1e67e21232 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access/_config.js @@ -0,0 +1,68 @@ +import { ok, test } from '../../test'; +import { flushSync } from 'svelte'; + +export default test({ + async test({ assert, target, logs }) { + const [btn1, btn2, btn3] = target.querySelectorAll('button'); + let ps = [...target.querySelectorAll('p')]; + + for (const p of ps) { + assert.equal(p.innerHTML, '0'); + } + + flushSync(() => { + btn1.click(); + }); + + // prop update normally if we are not unmounting + for (const p of ps) { + assert.equal(p.innerHTML, '1'); + } + + flushSync(() => { + btn3.click(); + }); + + // binding still works and update the value correctly + for (const p of ps) { + assert.equal(p.innerHTML, '0'); + } + + flushSync(() => { + btn1.click(); + }); + + flushSync(() => { + btn1.click(); + }); + + console.warn(logs); + + // the five components guarded by `count < 2` unmount and log + assert.deepEqual(logs, [1, true, 1, true, 1, true, 1, true, 1, true]); + + flushSync(() => { + btn2.click(); + }); + + // the three components guarded by `show` unmount and log + assert.deepEqual(logs, [ + 1, + true, + 1, + true, + 1, + true, + 1, + true, + 1, + true, + 2, + true, + 2, + true, + 2, + true + ]); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access/main.svelte b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access/main.svelte new file mode 100644 index 000000000000..9ebac3dc37e2 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/ondestroy-prop-access/main.svelte @@ -0,0 +1,42 @@ + + + + + + +{#if count < 2} + +{/if} + + +{#if count < 2} + +{/if} + + +{#if count < 2} + +{/if} + + +{#if show} + +{/if} + + + + + + + + + + + +