diff --git a/.changeset/tasty-pears-travel.md b/.changeset/tasty-pears-travel.md new file mode 100644 index 000000000000..96037f344005 --- /dev/null +++ b/.changeset/tasty-pears-travel.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: access last safe value of prop on unmount diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 2e6307a4b7a6..256da54478cd 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -167,6 +167,7 @@ export function client_component(analysis, options) { in_constructor: false, instance_level_snippets: [], module_level_snippets: [], + needs_safe_props: false, // these are set inside the `Fragment` visitor, and cannot be used until then init: /** @type {any} */ (null), diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index 63fe3223cf7d..1f201e531be3 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -45,6 +45,7 @@ export interface ComponentClientTransformState extends ClientTransformState { readonly hoisted: Array; readonly events: Set; readonly is_instance: boolean; + readonly needs_safe_props: boolean; readonly store_to_invalidate?: string; /** Stuff that happens before the render effect(s) */ diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js index e0aef2d316a7..238d0d217425 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js @@ -22,7 +22,7 @@ export function AwaitBlock(node, context) { if (node.then) { const then_context = { ...context, - state: { ...context.state, transform: { ...context.state.transform } } + state: { ...context.state, transform: { ...context.state.transform }, needs_safe_props: true } }; const argument = node.value && create_derived_block_argument(node.value, then_context); @@ -37,7 +37,7 @@ export function AwaitBlock(node, context) { } if (node.catch) { - const catch_context = { ...context, state: { ...context.state } }; + const catch_context = { ...context, state: { ...context.state, needs_safe_props: true } }; const argument = node.error && create_derived_block_argument(node.error, catch_context); /** @type {Pattern[]} */ @@ -59,7 +59,12 @@ export function AwaitBlock(node, context) { context.state.node, expression, node.pending - ? b.arrow([b.id('$$anchor')], /** @type {BlockStatement} */ (context.visit(node.pending))) + ? b.arrow( + [b.id('$$anchor')], + /** @type {BlockStatement} */ ( + context.visit(node.pending, { ...context.state, needs_safe_props: true }) + ) + ) : b.literal(null), then_block, catch_block diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Component.js index a10a3da6e3ec..ada1978f084b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Component.js @@ -11,7 +11,18 @@ import { build_component } from './shared/component.js'; export function Component(node, context) { if (node.metadata.dynamic) { // Handle dynamic references to what seems like static inline components - const component = build_component(node, '$$component', context, b.id('$$anchor')); + const component = build_component( + node, + '$$component', + { + ...context, + state: { + ...context.state, + needs_safe_props: true + } + }, + b.id('$$anchor') + ); context.state.init.push( b.stmt( b.call( diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js index 629cacda0148..eb4b83be6837 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js @@ -144,7 +144,8 @@ export function EachBlock(node, context) { const child_state = { ...context.state, transform: { ...context.state.transform }, - store_to_invalidate + store_to_invalidate, + needs_safe_props: true }; /** The state used when generating the key function, if necessary */ @@ -308,7 +309,15 @@ export function EachBlock(node, context) { if (node.fallback) { args.push( - b.arrow([b.id('$$anchor')], /** @type {BlockStatement} */ (context.visit(node.fallback))) + b.arrow( + [b.id('$$anchor')], + /** @type {BlockStatement} */ ( + context.visit(node.fallback, { + ...context.state, + needs_safe_props: true + }) + ) + ) ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js index d658f9eaf819..78cc26644327 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js @@ -11,7 +11,13 @@ export function IfBlock(node, context) { context.state.template.push(''); const statements = []; - const consequent = /** @type {BlockStatement} */ (context.visit(node.consequent)); + const consequent = /** @type {BlockStatement} */ ( + context.visit(node.consequent, { + ...context.state, + needs_safe_props: true + }) + ); + const consequent_id = context.state.scope.generate('consequent'); statements.push(b.var(b.id(consequent_id), b.arrow([b.id('$$anchor')], consequent))); @@ -19,7 +25,12 @@ export function IfBlock(node, context) { let alternate_id; if (node.alternate) { - const alternate = /** @type {BlockStatement} */ (context.visit(node.alternate)); + const alternate = /** @type {BlockStatement} */ ( + context.visit(node.alternate, { + ...context.state, + needs_safe_props: true + }) + ); alternate_id = context.state.scope.generate('alternate'); statements.push(b.var(b.id(alternate_id), b.arrow([b.id('$$anchor')], alternate))); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/KeyBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/KeyBlock.js index a013827f60bd..4fcc9062f659 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/KeyBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/KeyBlock.js @@ -11,7 +11,9 @@ export function KeyBlock(node, context) { context.state.template.push(''); const key = /** @type {Expression} */ (context.visit(node.expression)); - const body = /** @type {Expression} */ (context.visit(node.fragment)); + const body = /** @type {Expression} */ ( + context.visit(node.fragment, { ...context.state, needs_safe_props: true }) + ); context.state.init.push( b.stmt(b.call('$.key', context.state.node, b.thunk(key), b.arrow([b.id('$$anchor')], body))) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js index 7a0d6981b52a..d963adeade3e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js @@ -22,7 +22,7 @@ export function SnippetBlock(node, context) { const declarations = []; const transform = { ...context.state.transform }; - const child_state = { ...context.state, transform }; + const child_state = { ...context.state, transform, needs_safe_props: true }; for (let i = 0; i < node.parameters.length; i++) { const argument = node.parameters[i]; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index 2bae4486dc58..4801015c9e71 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -92,6 +92,21 @@ export function build_component(node, component_name, context, anchor = context. } } + let safe_props_ids = new Map(); + let safe_props_name = context.state.scope.generate('$$safe_props'); + + /** + * @param {string} name + * @param {Expression} expression + */ + function safe_propify(name, expression) { + if (context.state.needs_safe_props) { + safe_props_ids.set(name, expression); + return b.member(b.id(safe_props_name), b.id(name)); + } + return expression; + } + for (const attribute of node.attributes) { if (attribute.type === 'LetDirective') { if (!slot_scope_applies_to_itself) { @@ -118,13 +133,14 @@ export function build_component(node, component_name, context, anchor = context. if (attribute.metadata.expression.has_state) { let value = expression; + const name = context.state.scope.generate('spread_element'); if (attribute.metadata.expression.has_call) { - const id = b.id(context.state.scope.generate('spread_element')); + const id = b.id(name); context.state.init.push(b.var(id, b.call('$.derived', b.thunk(value)))); value = b.call('$.get', id); } - props_and_spreads.push(b.thunk(value)); + props_and_spreads.push(b.thunk(safe_propify(name, value))); } else { props_and_spreads.push(expression); } @@ -172,7 +188,7 @@ export function build_component(node, component_name, context, anchor = context. ); if (has_state) { - push_prop(b.get(attribute.name, [b.return(value)])); + push_prop(b.get(attribute.name, [b.return(safe_propify(attribute.name, value))])); } else { push_prop(b.init(attribute.name, value)); } @@ -205,7 +221,9 @@ export function build_component(node, component_name, context, anchor = context. context.state.init.push(b.var(get_id, get)); context.state.init.push(b.var(set_id, set)); - push_prop(b.get(attribute.name, [b.return(b.call(get_id))])); + push_prop( + b.get(attribute.name, [b.return(safe_propify(attribute.name, b.call(get_id)))]) + ); push_prop(b.set(attribute.name, [b.stmt(b.call(set_id, b.id('$$value')))])); } } else { @@ -228,11 +246,17 @@ export function build_component(node, component_name, context, anchor = context. // Delay prop pushes so bindings come at the end, to avoid spreads overwriting them if (is_store_sub) { push_prop( - b.get(attribute.name, [b.stmt(b.call('$.mark_store_binding')), b.return(expression)]), + b.get(attribute.name, [ + b.stmt(b.call('$.mark_store_binding')), + b.return(safe_propify(attribute.name, expression)) + ]), true ); } else { - push_prop(b.get(attribute.name, [b.return(expression)]), true); + push_prop( + b.get(attribute.name, [b.return(safe_propify(attribute.name, expression))]), + true + ); } const assignment = b.assignment( @@ -403,6 +427,32 @@ export function build_component(node, component_name, context, anchor = context. const statements = [...snippet_declarations]; + if (safe_props_ids.size > 0) { + // if it is a dynamic component we need to include the safe props call inside the component + // function otherwise in the init (which in case of the if will be in the consequent/alternate function) + if (component_name === '$$component') { + statements.push( + b.const( + safe_props_name, + b.call( + '$.safe_props', + b.object([...safe_props_ids].map(([name, id]) => b.get(name, [b.return(id)]))) + ) + ) + ); + } else { + context.state.init.push( + b.const( + safe_props_name, + b.call( + '$.safe_props', + b.object([...safe_props_ids].map(([name, id]) => b.get(name, [b.return(id)]))) + ) + ) + ); + } + } + if (node.type === 'SvelteComponent') { const prev = fn; diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index d78f6d452e84..95b9465113e4 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -118,7 +118,8 @@ export { legacy_rest_props, spread_props, update_pre_prop, - update_prop + update_prop, + safe_props } from './reactivity/props.js'; export { invalidate_store, diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index 5a3b30281f9f..877eccff6528 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -7,30 +7,17 @@ import { PROPS_IS_RUNES, PROPS_IS_UPDATED } from '../../../constants.js'; +import { legacy_mode_flag } from '../../flags/index.js'; 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 { safe_equals } from './equality.js'; +import { LEGACY_DERIVED_PROP, LEGACY_PROPS, STATE_SYMBOL } from '../constants.js'; import * as e from '../errors.js'; -import { - BRANCH_EFFECT, - LEGACY_DERIVED_PROP, - LEGACY_PROPS, - ROOT_EFFECT, - STATE_SYMBOL -} from '../constants.js'; import { proxy } from '../proxy.js'; +import { captured_signals, get, is_flushing_effect, untrack } from '../runtime.js'; +import { derived, derived_safe_equal } from './deriveds.js'; +import { teardown } from './effects.js'; +import { safe_equals } from './equality.js'; +import { inspect_effects, mutable_source, set, source, update } from './sources.js'; import { capture_store_binding } from './store.js'; -import { legacy_mode_flag } from '../../flags/index.js'; /** * @param {((value?: number) => number)} fn @@ -416,3 +403,42 @@ export function prop(props, key, flags, fallback) { return get(current_value); }; } + +/** + * + * @param {Record} props + */ +export function safe_props(props) { + let unmounting = false; + teardown(() => { + unmounting = true; + }); + const deriveds = new Map(); + return untrack(() => { + /** + * @type {Map} + */ + const olds = new Map(Object.entries(props)); + + return new Proxy( + {}, + { + get(_, key) { + if (!deriveds.has(key)) { + deriveds.set( + key, + derived(() => { + if (unmounting) { + return olds.get(key); + } + olds.set(key, props[key]); + return props[key]; + }) + ); + } + return get(deriveds.get(key)); + } + } + ); + }); +} diff --git a/packages/svelte/tests/runtime-runes/samples/ondestroy-prop-access/Component.svelte b/packages/svelte/tests/runtime-runes/samples/ondestroy-prop-access/Component.svelte new file mode 100644 index 000000000000..a1626ce276ad --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ondestroy-prop-access/Component.svelte @@ -0,0 +1,11 @@ + + +

{count}

+ + \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/ondestroy-prop-access/_config.js b/packages/svelte/tests/runtime-runes/samples/ondestroy-prop-access/_config.js new file mode 100644 index 000000000000..7d1e67e21232 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/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-runes/samples/ondestroy-prop-access/main.svelte b/packages/svelte/tests/runtime-runes/samples/ondestroy-prop-access/main.svelte new file mode 100644 index 000000000000..b7d4a0ecc505 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ondestroy-prop-access/main.svelte @@ -0,0 +1,45 @@ + + + + + + +{#if count < 2} + +{/if} + + +{#if count < 2} + +{/if} + + +{#if count < 2} + +{/if} + + +{#if show} + +{/if} + + + + + + + + + + + +