chore: simplify <select> initialization (#16251)

* initialize option values before initing select values

* simplify init_select

* simplify

* tweak

* tidy up

* tweak

* on second thoughts just simplify it here

Author: Rich-Harris

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';
 
 /**
@@ -199,24 +194,23 @@ 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);
 				continue;
 			}
 
 			if (needs_special_value_handling && attribute.name === 'value') {
-				build_element_special_value_attribute(node.name, node_id, attribute, context);
 				continue;
 			}
 
@@ -391,6 +385,15 @@ 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);
+				break;
+			}
+		}
+	}
+
 	context.state.template.pop_element();
 }
 
@@ -621,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);
@@ -651,10 +649,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, b.thunk(value))));
-	}
-
 	if (has_state) {
 		const id = b.id(state.scope.generate(`${node_id.name}_value`));
 
@@ -668,4 +662,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)));
+	}
 }

packages/svelte/src/internal/client/dom/elements/attributes.js

@@ -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,12 @@ export function attribute_effect(
 	});
 
 	if (is_select) {
-		init_select(
-			/** @type {HTMLSelectElement} */ (element),
-			() => /** @type {Record<string | symbol, any>} */ (prev).value
-		);
+		var select = /** @type {HTMLSelectElement} */ (element);
+
+		effect(() => {
+			select_option(select, /** @type {Record<string | symbol, any>} */ (prev).value);
+			init_select(select);
+		});
 	}
 
 	inited = true;

packages/svelte/src/internal/client/dom/elements/bindings/select.js

@@ -1,6 +1,5 @@
-import { effect } from '../../../reactivity/effects.js';
+import { effect, teardown } 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';
@@ -51,40 +50,29 @@ 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
- * @param {() => V} [get_value]
  */
-export function init_select(select, get_value) {
-	let mounting = true;
-	effect(() => {
-		if (get_value) {
-			select_option(select, untrack(get_value), mounting);
-		}
-		mounting = false;
+export function init_select(select) {
+	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 <optgroup>
+		// 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']
+	});
 
-		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 <optgroup>
-			// 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();
-		};
+	teardown(() => {
+		observer.disconnect();
 	});
 }
 
@@ -136,7 +124,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);
 }