From 84ff1d41d3a61ab570ab887f6eb4e54f2be8d440 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 5 Dec 2024 20:58:44 +0100 Subject: [PATCH] fix: ensure bindings always take precedence over spreads --- .changeset/little-berries-worry.md | 5 +++ .../client/visitors/shared/component.js | 36 +++++++++++++------ .../server/visitors/shared/component.js | 35 ++++++++++++------ .../bind-and-spread-precedence/_config.js | 9 +++++ .../bind-and-spread-precedence/input.svelte | 5 +++ .../bind-and-spread-precedence/main.svelte | 11 ++++++ 6 files changed, 81 insertions(+), 20 deletions(-) create mode 100644 .changeset/little-berries-worry.md create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/input.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/main.svelte diff --git a/.changeset/little-berries-worry.md b/.changeset/little-berries-worry.md new file mode 100644 index 000000000000..aad4de715a50 --- /dev/null +++ b/.changeset/little-berries-worry.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure bindings always take precedence over spreads 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 5dde60b3b414..aa7be93cb57e 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 @@ -20,6 +20,8 @@ import { determine_slot } from '../../../../../utils/slot.js'; export function build_component(node, component_name, context, anchor = context.state.node) { /** @type {Array} */ const props_and_spreads = []; + /** @type {Array<() => void>} */ + const delayed_props = []; /** @type {ExpressionStatement[]} */ const lets = []; @@ -63,14 +65,23 @@ export function build_component(node, component_name, context, anchor = context. /** * @param {Property} prop + * @param {boolean} [delay] */ - function push_prop(prop) { - const current = props_and_spreads.at(-1); - const current_is_props = Array.isArray(current); - const props = current_is_props ? current : []; - props.push(prop); - if (!current_is_props) { - props_and_spreads.push(props); + function push_prop(prop, delay = false) { + const do_push = () => { + const current = props_and_spreads.at(-1); + const current_is_props = Array.isArray(current); + const props = current_is_props ? current : []; + props.push(prop); + if (!current_is_props) { + props_and_spreads.push(props); + } + }; + + if (delay) { + delayed_props.push(do_push); + } else { + do_push(); } } @@ -202,22 +213,27 @@ export function build_component(node, component_name, context, anchor = context. attribute.expression.type === 'Identifier' && context.state.scope.get(attribute.expression.name)?.kind === 'store_sub'; + // 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(expression)]), + true ); } else { - push_prop(b.get(attribute.name, [b.return(expression)])); + push_prop(b.get(attribute.name, [b.return(expression)]), true); } const assignment = b.assignment('=', attribute.expression, b.id('$$value')); push_prop( - b.set(attribute.name, [b.stmt(/** @type {Expression} */ (context.visit(assignment)))]) + b.set(attribute.name, [b.stmt(/** @type {Expression} */ (context.visit(assignment)))]), + true ); } } } + delayed_props.forEach((fn) => fn()); + if (slot_scope_applies_to_itself) { context.state.init.push(...lets); } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js index 79df3cdd04c6..7cabfb06c527 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js @@ -13,6 +13,8 @@ import { is_element_node } from '../../../../nodes.js'; export function build_inline_component(node, expression, context) { /** @type {Array} */ const props_and_spreads = []; + /** @type {Array<() => void>} */ + const delayed_props = []; /** @type {Property[]} */ const custom_css_props = []; @@ -49,14 +51,23 @@ export function build_inline_component(node, expression, context) { /** * @param {Property} prop + * @param {boolean} [delay] */ - function push_prop(prop) { - const current = props_and_spreads.at(-1); - const current_is_props = Array.isArray(current); - const props = current_is_props ? current : []; - props.push(prop); - if (!current_is_props) { - props_and_spreads.push(props); + function push_prop(prop, delay = false) { + const do_push = () => { + const current = props_and_spreads.at(-1); + const current_is_props = Array.isArray(current); + const props = current_is_props ? current : []; + props.push(prop); + if (!current_is_props) { + props_and_spreads.push(props); + } + }; + + if (delay) { + delayed_props.push(do_push); + } else { + do_push(); } } @@ -81,11 +92,12 @@ export function build_inline_component(node, expression, context) { const value = build_attribute_value(attribute.value, context, false, true); push_prop(b.prop('init', b.key(attribute.name), value)); } else if (attribute.type === 'BindDirective' && attribute.name !== 'this') { - // TODO this needs to turn the whole thing into a while loop because the binding could be mutated eagerly in the child + // Delay prop pushes so bindings come at the end, to avoid spreads overwriting them push_prop( b.get(attribute.name, [ b.return(/** @type {Expression} */ (context.visit(attribute.expression))) - ]) + ]), + true ); push_prop( b.set(attribute.name, [ @@ -95,11 +107,14 @@ export function build_inline_component(node, expression, context) { ) ), b.stmt(b.assignment('=', b.id('$$settled'), b.false)) - ]) + ]), + true ); } } + delayed_props.forEach((fn) => fn()); + /** @type {Statement[]} */ const snippet_declarations = []; diff --git a/packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/_config.js b/packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/_config.js new file mode 100644 index 000000000000..79e707148605 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/_config.js @@ -0,0 +1,9 @@ +import { test } from '../../test'; + +export default test({ + ssrHtml: ``, + + test({ assert, target }) { + assert.equal(target.querySelector('input')?.value, 'foo'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/input.svelte b/packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/input.svelte new file mode 100644 index 000000000000..f19f2477077d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/input.svelte @@ -0,0 +1,5 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/main.svelte b/packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/main.svelte new file mode 100644 index 000000000000..b4dea5b5885d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-and-spread-precedence/main.svelte @@ -0,0 +1,11 @@ + + +