fix: unify <select> attribute handling and prevent double-await

Author: LeeWxx

packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js

@@ -7,12 +7,11 @@ import { is_void } from '../../../../../utils.js';
 import { dev, locator } from '../../../../state.js';
 import * as b from '#compiler/builders';
 import { clean_nodes, determine_namespace_for_children } from '../../utils.js';
-import { is_custom_element_node } from '../../../nodes.js';
 import {
-	ELEMENT_IS_NAMESPACED,
-	ELEMENT_PRESERVE_ATTRIBUTE_CASE
-} from '../../../../../constants.js';
-import { build_element_attributes, build_spread_object } from './shared/element.js';
+	build_element_attributes,
+	build_spread_object,
+	prepare_element_spread
+} from './shared/element.js';
 import {
 	process_children,
 	build_template,
@@ -43,8 +42,24 @@ export function RegularElement(node, context) {
 	const optimiser = new PromiseOptimiser();
 
 	state.template.push(b.literal(`<${node.name}`));
-	const body = build_element_attributes(node, { ...context, state }, optimiser.transform);
-	state.template.push(b.literal(node_is_void ? '/>' : '>')); // add `/>` for XHTML compliance
+
+	// If this element needs special handling (like <select value>),
+	// avoid calling build_element_attributes here to prevent evaluating/awaiting
+	// attribute expressions twice. We'll handle attributes in the special branch.
+	const is_select_special =
+		node.name === 'select' &&
+		node.attributes.some(
+			(attribute) =>
+				((attribute.type === 'Attribute' || attribute.type === 'BindDirective') &&
+					attribute.name === 'value') ||
+				attribute.type === 'SpreadAttribute'
+		);
+
+	let body = /** @type {Expression | null} */ (null);
+	if (!is_select_special) {
+		body = build_element_attributes(node, { ...context, state }, optimiser.transform);
+		state.template.push(b.literal(node_is_void ? '/>' : '>')); // add `/>` for XHTML compliance
+	}
 
 	if ((node.name === 'script' || node.name === 'style') && node.fragment.nodes.length === 1) {
 		state.template.push(
@@ -100,15 +115,7 @@ export function RegularElement(node, context) {
 		);
 	}
 
-	if (
-		node.name === 'select' &&
-		node.attributes.some(
-			(attribute) =>
-				((attribute.type === 'Attribute' || attribute.type === 'BindDirective') &&
-					attribute.name === 'value') ||
-				attribute.type === 'SpreadAttribute'
-		)
-	) {
+	if (is_select_special) {
 		/** @type {Array<AST.Attribute | AST.SpreadAttribute | AST.BindDirective>} */
 		const select_attributes = [];
 		/** @type {AST.ClassDirective[]} */
@@ -130,58 +137,15 @@ export function RegularElement(node, context) {
 			}
 		}
 
-		const attributes_expression = build_spread_object(
+		const { object, css_hash, classes, styles, flags } = prepare_element_spread(
 			node,
 			select_attributes,
+			style_directives,
+			class_directives,
 			context,
 			optimiser.transform
 		);
 
-		/** @type {ObjectExpression | undefined} */
-		let classes;
-		if (class_directives.length) {
-			const properties = class_directives.map((directive) =>
-				b.init(
-					directive.name,
-					directive.expression.type === 'Identifier' && directive.expression.name === directive.name
-						? b.id(directive.name)
-						: /** @type {Expression} */ (context.visit(directive.expression))
-				)
-			);
-
-			classes = b.object(properties);
-		}
-
-		/** @type {ObjectExpression | undefined} */
-		let styles;
-		if (style_directives.length > 0) {
-			const properties = style_directives.map((directive) =>
-				b.init(
-					directive.name,
-					directive.value === true
-						? b.id(directive.name)
-						: build_attribute_value(directive.value, context, optimiser.transform, true)
-				)
-			);
-
-			styles = b.object(properties);
-		}
-
-		let flags = 0;
-
-		if (node.metadata.svg || node.metadata.mathml) {
-			flags |= ELEMENT_IS_NAMESPACED | ELEMENT_PRESERVE_ATTRIBUTE_CASE;
-		} else if (is_custom_element_node(node)) {
-			flags |= ELEMENT_PRESERVE_ATTRIBUTE_CASE;
-		}
-
-		const css_hash =
-			node.metadata.scoped && context.state.analysis.css.hash
-				? b.literal(context.state.analysis.css.hash)
-				: undefined;
-
-		const flag_literal = flags ? b.literal(flags) : undefined;
-
 		const inner_state = { ...state, template: [], init: [] };
 		process_children(trimmed, { ...context, state: inner_state });
 
@@ -191,15 +155,7 @@ export function RegularElement(node, context) {
 		);
 
 		const statement = b.stmt(
-			b.call(
-				'$$renderer.select',
-				attributes_expression,
-				css_hash,
-				classes,
-				styles,
-				flag_literal,
-				fn
-			)
+			b.call('$$renderer.select', object, fn, css_hash, classes, styles, flags)
 		);
 
 		if (optimiser.expressions.length > 0) {

packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js

@@ -358,62 +358,84 @@ function build_element_spread_attributes(
 	context,
 	transform
 ) {
+	const { object, css_hash, classes, styles, flags } = prepare_element_spread(
+		element,
+		/** @type {Array<AST.Attribute | AST.SpreadAttribute | AST.BindDirective>} */ (attributes),
+		style_directives,
+		class_directives,
+		context,
+		transform
+	);
+
+	let call = b.call('$.attributes', object, css_hash, classes, styles, flags);
+
+	context.state.template.push(call);
+}
+
+/**
+ * Prepare args for $.attributes(...): compute object, css_hash, classes, styles and flags.
+ * @param {AST.RegularElement | AST.SvelteElement} element
+ * @param {Array<AST.Attribute | AST.SpreadAttribute | AST.BindDirective>} attributes
+ * @param {AST.StyleDirective[]} style_directives
+ * @param {AST.ClassDirective[]} class_directives
+ * @param {ComponentContext} context
+ * @param {(expression: Expression, metadata: ExpressionMetadata) => Expression} transform
+ * @returns {{ object: ObjectExpression, css_hash: Literal | undefined, classes: ObjectExpression | undefined, styles: ObjectExpression | undefined, flags: Literal | undefined }}
+ */
+export function prepare_element_spread(
+	element,
+	attributes,
+	style_directives,
+	class_directives,
+	context,
+	transform
+) {
+	/** @type {ObjectExpression | undefined} */
 	let classes;
+	/** @type {ObjectExpression | undefined} */
 	let styles;
-	let flags = 0;
-
-	let has_await = false;
+	let flags_num = 0;
 
 	if (class_directives.length) {
-		const properties = class_directives.map((directive) => {
-			has_await ||= directive.metadata.expression.has_await;
-
-			return b.init(
+		const properties = class_directives.map((directive) =>
+			b.init(
 				directive.name,
 				directive.expression.type === 'Identifier' && directive.expression.name === directive.name
 					? b.id(directive.name)
 					: /** @type {Expression} */ (context.visit(directive.expression))
-			);
-		});
-
+			)
+		);
 		classes = b.object(properties);
 	}
 
 	if (style_directives.length > 0) {
-		const properties = style_directives.map((directive) => {
-			has_await ||= directive.metadata.expression.has_await;
-
-			return b.init(
+		const properties = style_directives.map((directive) =>
+			b.init(
 				directive.name,
 				directive.value === true
 					? b.id(directive.name)
 					: build_attribute_value(directive.value, context, transform, true)
-			);
-		});
-
+			)
+		);
 		styles = b.object(properties);
 	}
 
 	if (element.metadata.svg || element.metadata.mathml) {
-		flags |= ELEMENT_IS_NAMESPACED | ELEMENT_PRESERVE_ATTRIBUTE_CASE;
+		flags_num |= ELEMENT_IS_NAMESPACED | ELEMENT_PRESERVE_ATTRIBUTE_CASE;
 	} else if (is_custom_element_node(element)) {
-		flags |= ELEMENT_PRESERVE_ATTRIBUTE_CASE;
+		flags_num |= ELEMENT_PRESERVE_ATTRIBUTE_CASE;
 	} else if (element.type === 'RegularElement' && element.name === 'input') {
-		flags |= ELEMENT_IS_INPUT;
+		flags_num |= ELEMENT_IS_INPUT;
 	}
 
 	const object = build_spread_object(element, attributes, context, transform);
-
 	const css_hash =
 		element.metadata.scoped && context.state.analysis.css.hash
 			? b.literal(context.state.analysis.css.hash)
 			: undefined;
+	const flags = flags_num ? b.literal(flags_num) : undefined;
 
-	const args = [object, css_hash, classes, styles, flags ? b.literal(flags) : undefined];
-
-	let call = b.call('$.attributes', ...args);
-
-	context.state.template.push(call);
+	return { object, css_hash, classes, styles, flags };
 }
 
 /**

packages/svelte/src/internal/server/renderer.js

@@ -158,47 +158,21 @@ export class Renderer {
 	}
 
 	/**
-	 * @overload
 	 * @param {Record<string, any>} attrs
 	 * @param {(renderer: Renderer) => void} fn
+	 * @param {string | undefined} [css_hash]
+	 * @param {Record<string, boolean> | undefined} [classes]
+	 * @param {Record<string, string> | undefined} [styles]
+	 * @param {number | undefined} [flags]
 	 * @returns {void}
 	 */
-	/**
-	 * @overload
-	 * @param {Record<string, any>} attrs
-	 * @param {string | undefined} css_hash
-	 * @param {Record<string, boolean> | undefined} classes
-	 * @param {Record<string, string> | undefined} styles
-	 * @param {number | undefined} flags
-	 * @param {(renderer: Renderer) => void} fn
-	 * @returns {void}
-	 */
-	/**
-	 * @param {Record<string, any>} attrs
-	 * @param {...any} rest
-	 * @returns {void}
-	 */
-	select(attrs, ...rest) {
-		const callback = /** @type {(renderer: Renderer) => void} */ (rest.pop() ?? (() => {}));
-		/** @type {[
-			string | undefined,
-			Record<string, boolean> | undefined,
-			Record<string, string> | undefined,
-			number | undefined
-		]} */
-		const [css_hash, classes, styles, flags] = /** @type {[
-			string | undefined,
-			Record<string, boolean> | undefined,
-			Record<string, string> | undefined,
-			number | undefined
-		]} */ (rest);
-
+	select(attrs, fn, css_hash, classes, styles, flags) {
 		const { value, ...select_attrs } = attrs;
 
 		this.push(`<select${attributes(select_attrs, css_hash, classes, styles, flags)}>`);
 		this.child((renderer) => {
 			renderer.local.select_value = value;
-			callback(renderer);
+			fn(renderer);
 		});
 		this.push('</select>');
 	}