From a73a6197146e04b4bcae4f52bd8635f2eb438dac Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 19 Feb 2025 22:24:51 +0100 Subject: [PATCH 1/4] fix: access last safe value of prop on unmount --- .changeset/tasty-pears-travel.md | 5 +++ .../phases/3-transform/client/types.d.ts | 2 + .../3-transform/client/visitors/Component.js | 32 +++++++++++++++- .../3-transform/client/visitors/Identifier.js | 36 ++++++++++++++++-- .../3-transform/client/visitors/IfBlock.js | 24 +++++++++++- .../client/visitors/shared/element.js | 2 +- packages/svelte/src/internal/client/index.js | 3 +- .../src/internal/client/reactivity/props.js | 37 +++++++++++++++++++ 8 files changed, 133 insertions(+), 8 deletions(-) create mode 100644 .changeset/tasty-pears-travel.md 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/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index 63fe3223cf7d..c61ccaa98862 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 @@ -23,6 +23,8 @@ export interface ClientTransformState extends TransformState { * us to rewrite `this.foo` as `this.#foo.value` */ readonly in_constructor: boolean; + readonly safe_props_ids?: Map; + readonly safe_props_name?: string; readonly transform: Record< string, 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..99eae046b015 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 @@ -10,8 +10,24 @@ import { build_component } from './shared/component.js'; */ export function Component(node, context) { if (node.metadata.dynamic) { + let safe_props_ids = new Map(); + + const safe_props_name = context.state.scope.generate('$$safe_props'); + // 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, + safe_props_ids, + safe_props_name + } + }, + b.id('$$anchor') + ); context.state.init.push( b.stmt( b.call( @@ -20,7 +36,19 @@ export function Component(node, context) { // TODO use untrack here to not update when binding changes? // Would align with Svelte 4 behavior, but it's arguably nicer/expected to update this b.thunk(/** @type {Expression} */ (context.visit(b.member_id(node.name)))), - b.arrow([b.id('$$anchor'), b.id('$$component')], b.block([component])) + b.arrow( + [b.id('$$anchor'), b.id('$$component')], + b.block([ + b.const( + safe_props_name, + b.call( + '$.safe_props', + b.object([...safe_props_ids].map(([name, id]) => b.get(name, [b.return(id)]))) + ) + ), + component + ]) + ) ) ) ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js index ae62909eff8a..40144859a4b6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js @@ -9,9 +9,9 @@ import { build_getter } from '../utils.js'; * @param {Context} context */ export function Identifier(node, context) { - const parent = /** @type {Node} */ (context.path.at(-1)); + let parent = context.path.at(-1); - if (is_reference(node, parent)) { + if (is_reference(node, /** @type {Node} */ (parent))) { if (node.name === '$$props') { return b.id('$$sanitized_props'); } @@ -36,6 +36,36 @@ export function Identifier(node, context) { } } - return build_getter(node, context.state); + const getter = build_getter(node, context.state); + + if ( + // this means we are inside an if or as an attribute of a dynamic component + // and we want to access `$$safe_props` to allow for the component to access them + // after destructuring + context.state.safe_props_name != null && + context.state.safe_props_ids != null && + // the parent can either be a component/svelte component in that case we + // check if this identifier is one of the attributes + (((parent?.type === 'Component' || parent?.type === 'SvelteComponent') && + parent.attributes.some( + (el) => + (el.type === 'Attribute' && + typeof el.value !== 'boolean' && + !Array.isArray(el.value) && + el.value.expression === node) || + (el.type === 'BindDirective' && el.expression === node) + )) || + // or a spread and we check the expression + (parent?.type === 'SpreadAttribute' && parent.expression === node)) && + // we also don't want to transform bindings that are defined withing the if block + // itself (for example an each local variable) + !binding?.references[0].path.some((node) => node.type === 'IfBlock') + ) { + // we store the getter in the safe props id and return an access to `$$safe_props.name` + context.state.safe_props_ids.set(node.name, getter); + return b.member(b.id(context.state.safe_props_name), b.id(node.name)); + } + + return getter; } } 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..2aec6e0156dd 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,29 @@ export function IfBlock(node, context) { context.state.template.push(''); const statements = []; - const consequent = /** @type {BlockStatement} */ (context.visit(node.consequent)); + let safe_props_ids = new Map(); + + const safe_props_id = context.state.scope.generate('$$safe_props'); + + const consequent = /** @type {BlockStatement} */ ( + context.visit(node.consequent, { + ...context.state, + safe_props_ids, + safe_props_name: safe_props_id + }) + ); + + if (consequent.body.length > 0 && safe_props_ids) { + consequent.body.unshift( + b.const( + safe_props_id, + b.call( + '$.safe_props', + b.object([...safe_props_ids].map(([name, id]) => b.get(name, [b.return(id)]))) + ) + ) + ); + } const consequent_id = context.state.scope.generate('consequent'); statements.push(b.var(b.id(consequent_id), b.arrow([b.id('$$anchor')], consequent))); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js index abffad0ff7a4..f75e9dd93c49 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js @@ -185,7 +185,7 @@ export function build_attribute_value(value, context, memoize = (value) => value return { value: b.literal(chunk.data), has_state: false }; } - let expression = /** @type {Expression} */ (context.visit(chunk.expression)); + let expression = /** @type {Expression} */ (context.visit(chunk.expression, context.state)); return { value: memoize(expression, chunk.metadata.expression), 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..be4b11dec2d7 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -31,6 +31,7 @@ import { import { proxy } from '../proxy.js'; import { capture_store_binding } from './store.js'; import { legacy_mode_flag } from '../../flags/index.js'; +import { teardown } from './effects.js'; /** * @param {((value?: number) => number)} fn @@ -416,3 +417,39 @@ 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(); + /** + * @type {Map} + */ + const olds = new Map(untrack(() => 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)); + } + } + ); +} From 48adf81220bf83a7c1539263643620b33d76ade3 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Thu, 20 Feb 2025 16:28:07 +0100 Subject: [PATCH 2/4] fix: add test and change strategy --- .../phases/3-transform/client/types.d.ts | 3 +- .../3-transform/client/visitors/Component.js | 21 +----- .../3-transform/client/visitors/Identifier.js | 36 +--------- .../3-transform/client/visitors/IfBlock.js | 25 ++----- .../client/visitors/shared/component.js | 62 +++++++++++++++-- .../client/visitors/shared/element.js | 2 +- .../ondestroy-prop-access/Component.svelte | 11 +++ .../samples/ondestroy-prop-access/_config.js | 68 +++++++++++++++++++ .../samples/ondestroy-prop-access/main.svelte | 45 ++++++++++++ 9 files changed, 194 insertions(+), 79 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/ondestroy-prop-access/Component.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/ondestroy-prop-access/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/ondestroy-prop-access/main.svelte 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 c61ccaa98862..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 @@ -23,8 +23,6 @@ export interface ClientTransformState extends TransformState { * us to rewrite `this.foo` as `this.#foo.value` */ readonly in_constructor: boolean; - readonly safe_props_ids?: Map; - readonly safe_props_name?: string; readonly transform: Record< string, @@ -47,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/Component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Component.js index 99eae046b015..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 @@ -10,10 +10,6 @@ import { build_component } from './shared/component.js'; */ export function Component(node, context) { if (node.metadata.dynamic) { - let safe_props_ids = new Map(); - - const safe_props_name = context.state.scope.generate('$$safe_props'); - // Handle dynamic references to what seems like static inline components const component = build_component( node, @@ -22,8 +18,7 @@ export function Component(node, context) { ...context, state: { ...context.state, - safe_props_ids, - safe_props_name + needs_safe_props: true } }, b.id('$$anchor') @@ -36,19 +31,7 @@ export function Component(node, context) { // TODO use untrack here to not update when binding changes? // Would align with Svelte 4 behavior, but it's arguably nicer/expected to update this b.thunk(/** @type {Expression} */ (context.visit(b.member_id(node.name)))), - b.arrow( - [b.id('$$anchor'), b.id('$$component')], - b.block([ - b.const( - safe_props_name, - b.call( - '$.safe_props', - b.object([...safe_props_ids].map(([name, id]) => b.get(name, [b.return(id)]))) - ) - ), - component - ]) - ) + b.arrow([b.id('$$anchor'), b.id('$$component')], b.block([component])) ) ) ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js index 40144859a4b6..ae62909eff8a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js @@ -9,9 +9,9 @@ import { build_getter } from '../utils.js'; * @param {Context} context */ export function Identifier(node, context) { - let parent = context.path.at(-1); + const parent = /** @type {Node} */ (context.path.at(-1)); - if (is_reference(node, /** @type {Node} */ (parent))) { + if (is_reference(node, parent)) { if (node.name === '$$props') { return b.id('$$sanitized_props'); } @@ -36,36 +36,6 @@ export function Identifier(node, context) { } } - const getter = build_getter(node, context.state); - - if ( - // this means we are inside an if or as an attribute of a dynamic component - // and we want to access `$$safe_props` to allow for the component to access them - // after destructuring - context.state.safe_props_name != null && - context.state.safe_props_ids != null && - // the parent can either be a component/svelte component in that case we - // check if this identifier is one of the attributes - (((parent?.type === 'Component' || parent?.type === 'SvelteComponent') && - parent.attributes.some( - (el) => - (el.type === 'Attribute' && - typeof el.value !== 'boolean' && - !Array.isArray(el.value) && - el.value.expression === node) || - (el.type === 'BindDirective' && el.expression === node) - )) || - // or a spread and we check the expression - (parent?.type === 'SpreadAttribute' && parent.expression === node)) && - // we also don't want to transform bindings that are defined withing the if block - // itself (for example an each local variable) - !binding?.references[0].path.some((node) => node.type === 'IfBlock') - ) { - // we store the getter in the safe props id and return an access to `$$safe_props.name` - context.state.safe_props_ids.set(node.name, getter); - return b.member(b.id(context.state.safe_props_name), b.id(node.name)); - } - - return getter; + return build_getter(node, context.state); } } 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 2aec6e0156dd..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,29 +11,13 @@ export function IfBlock(node, context) { context.state.template.push(''); const statements = []; - let safe_props_ids = new Map(); - - const safe_props_id = context.state.scope.generate('$$safe_props'); - const consequent = /** @type {BlockStatement} */ ( context.visit(node.consequent, { ...context.state, - safe_props_ids, - safe_props_name: safe_props_id + needs_safe_props: true }) ); - if (consequent.body.length > 0 && safe_props_ids) { - consequent.body.unshift( - b.const( - safe_props_id, - b.call( - '$.safe_props', - b.object([...safe_props_ids].map(([name, id]) => b.get(name, [b.return(id)]))) - ) - ) - ); - } const consequent_id = context.state.scope.generate('consequent'); statements.push(b.var(b.id(consequent_id), b.arrow([b.id('$$anchor')], consequent))); @@ -41,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/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/compiler/phases/3-transform/client/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js index f75e9dd93c49..abffad0ff7a4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js @@ -185,7 +185,7 @@ export function build_attribute_value(value, context, memoize = (value) => value return { value: b.literal(chunk.data), has_state: false }; } - let expression = /** @type {Expression} */ (context.visit(chunk.expression, context.state)); + let expression = /** @type {Expression} */ (context.visit(chunk.expression)); return { value: memoize(expression, chunk.metadata.expression), 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} + + + + + + + + + + + + From 8bd65ca738bfd916d473a91a470a9d0d4900d2cf Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Thu, 20 Feb 2025 16:49:20 +0100 Subject: [PATCH 3/4] fix: typecheck --- .../src/compiler/phases/3-transform/client/transform-client.js | 1 + 1 file changed, 1 insertion(+) 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), From c2c2eddb02efc51449c0f7044dad2b88ffeeef35 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Fri, 21 Feb 2025 12:34:42 +0100 Subject: [PATCH 4/4] fix: other blocks --- .../3-transform/client/visitors/AwaitBlock.js | 11 ++- .../3-transform/client/visitors/EachBlock.js | 13 +++- .../3-transform/client/visitors/KeyBlock.js | 4 +- .../client/visitors/SnippetBlock.js | 2 +- .../src/internal/client/reactivity/props.js | 75 ++++++++----------- 5 files changed, 55 insertions(+), 50 deletions(-) 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/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/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/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index be4b11dec2d7..877eccff6528 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -7,31 +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 { capture_store_binding } from './store.js'; -import { legacy_mode_flag } from '../../flags/index.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'; /** * @param {((value?: number) => number)} fn @@ -428,28 +414,31 @@ export function safe_props(props) { unmounting = true; }); const deriveds = new Map(); - /** - * @type {Map} - */ - const olds = new Map(untrack(() => 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 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)); } - return get(deriveds.get(key)); } - } - ); + ); + }); }