From 334adc0897a7463c02819d91f574eae008e611cd Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 26 Jun 2025 18:40:17 -0400 Subject: [PATCH 1/7] initialize option values before initing select values --- .../client/visitors/RegularElement.js | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index b0f285eb413d..18214c83429a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -199,16 +199,16 @@ export function RegularElement(node, context) { const node_id = context.state.node; + /** If true, needs `__value` for inputs */ + const needs_special_value_handling = + node.name === 'option' || + node.name === 'select' || + bindings.has('group') || + bindings.has('checked'); + if (has_spread) { build_attribute_effect(attributes, class_directives, style_directives, context, node, node_id); } else { - /** If true, needs `__value` for inputs */ - const needs_special_value_handling = - node.name === 'option' || - node.name === 'select' || - bindings.has('group') || - bindings.has('checked'); - for (const attribute of /** @type {AST.Attribute[]} */ (attributes)) { if (is_event_attribute(attribute)) { visit_event_attribute(attribute, context); @@ -216,7 +216,6 @@ export function RegularElement(node, context) { } if (needs_special_value_handling && attribute.name === 'value') { - build_element_special_value_attribute(node.name, node_id, attribute, context); continue; } @@ -391,6 +390,21 @@ export function RegularElement(node, context) { context.state.update.push(b.stmt(b.assignment('=', dir, dir))); } + if (!has_spread && needs_special_value_handling) { + for (const attribute of /** @type {AST.Attribute[]} */ (attributes)) { + if (attribute.name === 'value') { + build_element_special_value_attribute( + node.name, + node_id, + attribute, + context, + context.state + ); + break; + } + } + } + context.state.template.pop_element(); } @@ -612,9 +626,9 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co * @param {Identifier} node_id * @param {AST.Attribute} attribute * @param {ComponentContext} context + * @param {ComponentClientTransformState} state */ -function build_element_special_value_attribute(element, node_id, attribute, context) { - const state = context.state; +function build_element_special_value_attribute(element, node_id, attribute, context, state) { const is_select_with_value = // attribute.metadata.dynamic would give false negatives because even if the value does not change, // the inner options could still change, so we need to always treat it as reactive From d89f3184ebb58257306796454bba2880429e1941 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 26 Jun 2025 18:51:13 -0400 Subject: [PATCH 2/7] simplify init_select --- .../client/visitors/RegularElement.js | 2 +- .../internal/client/dom/elements/attributes.js | 17 +++++++++++------ .../client/dom/elements/bindings/select.js | 9 +-------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 18214c83429a..b13284c35458 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -666,7 +666,7 @@ function build_element_special_value_attribute(element, node_id, attribute, cont ); if (is_select_with_value) { - state.init.push(b.stmt(b.call('$.init_select', node_id, b.thunk(value)))); + state.init.push(b.stmt(b.call('$.init_select', node_id))); } if (has_state) { diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 2d3d6a921dc1..cc44171e21c3 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -6,7 +6,7 @@ import { create_event, delegate } from './events.js'; import { add_form_reset_listener, autofocus } from './misc.js'; import * as w from '../../warnings.js'; import { LOADING_ATTR_SYMBOL } from '#client/constants'; -import { queue_idle_task } from '../task.js'; +import { queue_idle_task, queue_micro_task } from '../task.js'; import { is_capture_event, is_delegated, normalize_attribute } from '../../../../utils.js'; import { active_effect, @@ -20,7 +20,7 @@ import { clsx } from '../../../shared/attributes.js'; import { set_class } from './class.js'; import { set_style } from './style.js'; import { ATTACHMENT_KEY, NAMESPACE_HTML } from '../../../../constants.js'; -import { block, branch, destroy_effect } from '../../reactivity/effects.js'; +import { block, branch, destroy_effect, effect } from '../../reactivity/effects.js'; import { derived } from '../../reactivity/deriveds.js'; import { init_select, select_option } from './bindings/select.js'; @@ -513,10 +513,15 @@ export function attribute_effect( }); if (is_select) { - init_select( - /** @type {HTMLSelectElement} */ (element), - () => /** @type {Record} */ (prev).value - ); + var select = /** @type {HTMLSelectElement} */ (element); + + if (!inited) { + effect(() => { + select_option(select, /** @type {Record} */ (prev).value); + }); + } + + init_select(select); } inited = true; diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/select.js b/packages/svelte/src/internal/client/dom/elements/bindings/select.js index e3263c65afae..5363df0d44c2 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/select.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/select.js @@ -53,16 +53,9 @@ export function select_option(select, value, mounting) { * inside an `#each` block. * @template V * @param {HTMLSelectElement} select - * @param {() => V} [get_value] */ -export function init_select(select, get_value) { - let mounting = true; +export function init_select(select) { effect(() => { - if (get_value) { - select_option(select, untrack(get_value), mounting); - } - mounting = false; - var observer = new MutationObserver(() => { // @ts-ignore var value = select.__value; From 58918e38e997a5f0f7b33120ab7bdb0d0543f6ef Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 26 Jun 2025 19:03:35 -0400 Subject: [PATCH 3/7] simplify --- .../phases/3-transform/client/visitors/RegularElement.js | 8 ++++---- .../svelte/src/internal/client/dom/elements/attributes.js | 8 +++----- .../src/internal/client/dom/elements/bindings/select.js | 5 ++--- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index b13284c35458..aec8e9ef312a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -665,10 +665,6 @@ function build_element_special_value_attribute(element, node_id, attribute, cont : inner_assignment ); - if (is_select_with_value) { - state.init.push(b.stmt(b.call('$.init_select', node_id))); - } - if (has_state) { const id = b.id(state.scope.generate(`${node_id.name}_value`)); @@ -682,4 +678,8 @@ function build_element_special_value_attribute(element, node_id, attribute, cont } else { state.init.push(update); } + + if (is_select_with_value) { + state.init.push(b.stmt(b.call('$.init_select', node_id))); + } } diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index cc44171e21c3..1296d1d536cb 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -515,11 +515,9 @@ export function attribute_effect( if (is_select) { var select = /** @type {HTMLSelectElement} */ (element); - if (!inited) { - effect(() => { - select_option(select, /** @type {Record} */ (prev).value); - }); - } + queue_micro_task(() => { + select_option(select, /** @type {Record} */ (prev).value); + }); init_select(select); } diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/select.js b/packages/svelte/src/internal/client/dom/elements/bindings/select.js index 5363df0d44c2..c4f425533084 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/select.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/select.js @@ -1,9 +1,9 @@ import { effect } from '../../../reactivity/effects.js'; import { listen_to_event_and_reset_event } from './shared.js'; -import { untrack } from '../../../runtime.js'; import { is } from '../../../proxy.js'; import { is_array } from '../../../../shared/utils.js'; import * as w from '../../../warnings.js'; +import { queue_micro_task } from '../../task.js'; /** * Selects the correct option(s) (depending on whether this is a multiple select) @@ -51,11 +51,10 @@ export function select_option(select, value, mounting) { * current selection to the dom when it changes. Such * changes could for example occur when options are * inside an `#each` block. - * @template V * @param {HTMLSelectElement} select */ export function init_select(select) { - effect(() => { + queue_micro_task(() => { var observer = new MutationObserver(() => { // @ts-ignore var value = select.__value; From 38d458a01ba31e01d6d68e566eadc40ce9d989bb Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 26 Jun 2025 19:13:51 -0400 Subject: [PATCH 4/7] tweak --- .../client/dom/elements/attributes.js | 5 +- .../client/dom/elements/bindings/select.js | 46 +++++++++---------- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 1296d1d536cb..b14ecb42a1ae 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -515,11 +515,10 @@ export function attribute_effect( if (is_select) { var select = /** @type {HTMLSelectElement} */ (element); - queue_micro_task(() => { + effect(() => { select_option(select, /** @type {Record} */ (prev).value); + init_select(select); }); - - init_select(select); } inited = true; diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/select.js b/packages/svelte/src/internal/client/dom/elements/bindings/select.js index c4f425533084..ff7c59f36ce7 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/select.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/select.js @@ -1,4 +1,4 @@ -import { effect } from '../../../reactivity/effects.js'; +import { effect, teardown } from '../../../reactivity/effects.js'; import { listen_to_event_and_reset_event } from './shared.js'; import { is } from '../../../proxy.js'; import { is_array } from '../../../../shared/utils.js'; @@ -54,29 +54,26 @@ export function select_option(select, value, mounting) { * @param {HTMLSelectElement} select */ export function init_select(select) { - queue_micro_task(() => { - var observer = new MutationObserver(() => { - // @ts-ignore - var value = select.__value; - select_option(select, value); - // Deliberately don't update the potential binding value, - // the model should be preserved unless explicitly changed - }); - - observer.observe(select, { - // Listen to option element changes - childList: true, - subtree: true, // because of - // Listen to option element value attribute changes - // (doesn't get notified of select value changes, - // because that property is not reflected as an attribute) - attributes: true, - attributeFilter: ['value'] - }); - - return () => { - observer.disconnect(); - }; + var observer = new MutationObserver(() => { + // @ts-ignore + select_option(select, select.__value); + // Deliberately don't update the potential binding value, + // the model should be preserved unless explicitly changed + }); + + observer.observe(select, { + // Listen to option element changes + childList: true, + subtree: true, // because of + // Listen to option element value attribute changes + // (doesn't get notified of select value changes, + // because that property is not reflected as an attribute) + attributes: true, + attributeFilter: ['value'] + }); + + teardown(() => { + observer.disconnect(); }); } @@ -128,7 +125,6 @@ export function bind_select_value(select, get, set = get) { mounting = false; }); - // don't pass get_value, we already initialize it in the effect above init_select(select); } From 39b7f416719bf160272767d7b0dbd7ec725fc135 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 26 Jun 2025 19:14:54 -0400 Subject: [PATCH 5/7] tidy up --- packages/svelte/src/internal/client/dom/elements/attributes.js | 2 +- .../svelte/src/internal/client/dom/elements/bindings/select.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index b14ecb42a1ae..5db685cf3e90 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -6,7 +6,7 @@ import { create_event, delegate } from './events.js'; import { add_form_reset_listener, autofocus } from './misc.js'; import * as w from '../../warnings.js'; import { LOADING_ATTR_SYMBOL } from '#client/constants'; -import { queue_idle_task, queue_micro_task } from '../task.js'; +import { queue_idle_task } from '../task.js'; import { is_capture_event, is_delegated, normalize_attribute } from '../../../../utils.js'; import { active_effect, diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/select.js b/packages/svelte/src/internal/client/dom/elements/bindings/select.js index ff7c59f36ce7..5e89686d8654 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/select.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/select.js @@ -3,7 +3,6 @@ import { listen_to_event_and_reset_event } from './shared.js'; import { is } from '../../../proxy.js'; import { is_array } from '../../../../shared/utils.js'; import * as w from '../../../warnings.js'; -import { queue_micro_task } from '../../task.js'; /** * Selects the correct option(s) (depending on whether this is a multiple select) From 5615fd34e88979c427e81665c8a317ec63d73448 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 26 Jun 2025 19:17:11 -0400 Subject: [PATCH 6/7] tweak --- .../3-transform/client/visitors/RegularElement.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index aec8e9ef312a..9a9bd80975af 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -393,13 +393,7 @@ export function RegularElement(node, context) { if (!has_spread && needs_special_value_handling) { for (const attribute of /** @type {AST.Attribute[]} */ (attributes)) { if (attribute.name === 'value') { - build_element_special_value_attribute( - node.name, - node_id, - attribute, - context, - context.state - ); + build_element_special_value_attribute(node.name, node_id, attribute, context); break; } } @@ -626,9 +620,9 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co * @param {Identifier} node_id * @param {AST.Attribute} attribute * @param {ComponentContext} context - * @param {ComponentClientTransformState} state */ -function build_element_special_value_attribute(element, node_id, attribute, context, state) { +function build_element_special_value_attribute(element, node_id, attribute, context) { + const state = context.state; const is_select_with_value = // attribute.metadata.dynamic would give false negatives because even if the value does not change, // the inner options could still change, so we need to always treat it as reactive From b459bb093592a0a6a346ad44fb5295eeccfa3fc9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 26 Jun 2025 19:26:10 -0400 Subject: [PATCH 7/7] on second thoughts just simplify it here --- .../3-transform/client/visitors/RegularElement.js | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 9a9bd80975af..ae8680f5940c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -22,12 +22,7 @@ import { build_set_style } from './shared/element.js'; import { process_children } from './shared/fragment.js'; -import { - build_render_statement, - build_template_chunk, - get_expression_id, - memoize_expression -} from './shared/utils.js'; +import { build_render_statement, build_template_chunk, get_expression_id } from './shared/utils.js'; import { visit_event_attribute } from './shared/events.js'; /** @@ -629,12 +624,7 @@ function build_element_special_value_attribute(element, node_id, attribute, cont element === 'select' && attribute.value !== true && !is_text_attribute(attribute); const { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) => - metadata.has_call - ? // if is a select with value we will also invoke `init_select` which need a reference before the template effect so we memoize separately - is_select_with_value - ? memoize_expression(state, value) - : get_expression_id(state.expressions, value) - : value + metadata.has_call ? get_expression_id(state.expressions, value) : value ); const evaluated = context.state.scope.evaluate(value);