breaking: warn on quotes single-expression attributes in runes mode (#12479)

* parse `foo={bar}` attribute as `value: { type: 'ExpressionTag', .. }` (i.e. don't wrap in an array)

* warn on quoted single-expression-attributes

* breaking: warn on quotes single-expression attributes in runes mode

In a future version, that will mean things are getting stringified, which is a departure from how things work today, therefore a warning first.
Related to #7925

* Update .changeset/plenty-items-build.md

* Apply suggestions from code review

* missed a spot

---------

Co-authored-by: Rich Harris <[email protected]>

Author: dummdidumm

.changeset/plenty-items-build.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+breaking: warn on quoted single-expression attributes in runes mode

packages/svelte/messages/compile-warnings/template.md

@@ -14,6 +14,10 @@
 
 > '%wrong%' is not a valid HTML attribute. Did you mean '%right%'?
 
+## attribute_quoted
+
+> Quoted attributes on components and custom elements will be stringified in a future version of Svelte. If this isn't what you want, remove the quotes
+
 ## bind_invalid_each_rest
 
 > The rest operator (...) will create a new object and binding '%name%' with the original object will not work

packages/svelte/src/compiler/legacy.js

@@ -411,6 +411,34 @@ export function convert(source, ast) {
 					)
 				};
 			},
+			Attribute(node, { visit, next, path }) {
+				if (node.value !== true && !Array.isArray(node.value)) {
+					path.push(node);
+					const value = /** @type {Legacy.LegacyAttribute['value']} */ ([visit(node.value)]);
+					path.pop();
+
+					return {
+						...node,
+						value
+					};
+				} else {
+					return next();
+				}
+			},
+			StyleDirective(node, { visit, next, path }) {
+				if (node.value !== true && !Array.isArray(node.value)) {
+					path.push(node);
+					const value = /** @type {Legacy.LegacyStyleDirective['value']} */ ([visit(node.value)]);
+					path.pop();
+
+					return {
+						...node,
+						value
+					};
+				} else {
+					return next();
+				}
+			},
 			SpreadAttribute(node) {
 				return { ...node, type: 'Spread' };
 			},

packages/svelte/src/compiler/migrate/index.js

@@ -532,11 +532,13 @@ const template = {
 				if (attr.name === 'name') {
 					slot_name = /** @type {any} */ (attr.value)[0].data;
 				} else {
+					const attr_value =
+						attr.value === true || Array.isArray(attr.value) ? attr.value : [attr.value];
 					const value =
-						attr.value !== true
+						attr_value !== true
 							? state.str.original.substring(
-									attr.value[0].start,
-									attr.value[attr.value.length - 1].end
+									attr_value[0].start,
+									attr_value[attr_value.length - 1].end
 								)
 							: 'true';
 					slot_props += value === attr.name ? `${value}, ` : `${attr.name}: ${value}, `;

packages/svelte/src/compiler/phases/1-parse/read/options.js

@@ -41,8 +41,9 @@ export default function read_options(node) {
 			case 'customElement': {
 				/** @type {SvelteOptions['customElement']} */
 				const ce = { tag: '' };
+				const { value: v } = attribute;
+				const value = v === true || Array.isArray(v) ? v : [v];
 
-				const { value } = attribute;
 				if (value === true) {
 					e.svelte_options_invalid_customelement(attribute);
 				} else if (value[0].type === 'Text') {
@@ -199,7 +200,11 @@ export default function read_options(node) {
  */
 function get_static_value(attribute) {
 	const { value } = attribute;
-	const chunk = value[0];
+
+	if (value === true) return true;
+
+	const chunk = Array.isArray(value) ? value[0] : value;
+
 	if (!chunk) return true;
 	if (value.length > 1) {
 		return null;
@@ -208,6 +213,7 @@ function get_static_value(attribute) {
 	if (chunk.expression.type !== 'Literal') {
 		return null;
 	}
+
 	return chunk.expression.value;
 }
 

packages/svelte/src/compiler/phases/1-parse/state/element.js

@@ -9,6 +9,7 @@ import * as e from '../../../errors.js';
 import * as w from '../../../warnings.js';
 import { create_fragment } from '../utils/create.js';
 import { create_attribute } from '../../nodes.js';
+import { get_attribute_expression, is_expression_attribute } from '../../../utils/ast.js';
 
 // eslint-disable-next-line no-useless-escape
 const valid_tag_name = /^\!?[a-zA-Z]{1,}:?[a-zA-Z0-9\-]*/;
@@ -241,15 +242,11 @@ export default function element(parser) {
 		}
 
 		const definition = /** @type {Compiler.Attribute} */ (element.attributes.splice(index, 1)[0]);
-		if (
-			definition.value === true ||
-			definition.value.length !== 1 ||
-			definition.value[0].type === 'Text'
-		) {
+		if (!is_expression_attribute(definition)) {
 			e.svelte_component_invalid_this(definition.start);
 		}
 
-		element.expression = definition.value[0].expression;
+		element.expression = get_attribute_expression(definition);
 	}
 
 	if (element.type === 'SvelteElement') {
@@ -267,15 +264,16 @@ export default function element(parser) {
 			e.svelte_element_missing_this(definition);
 		}
 
-		const chunk = definition.value[0];
-
-		if (definition.value.length !== 1 || chunk.type !== 'ExpressionTag') {
+		if (!is_expression_attribute(definition)) {
 			w.svelte_element_invalid_this(definition);
 
 			// note that this is wrong, in the case of e.g. `this="h{n}"` — it will result in `<h>`.
 			// it would be much better to just error here, but we are preserving the existing buggy
 			// Svelte 4 behaviour out of an overabundance of caution regarding breaking changes.
 			// TODO in 6.0, error
+			const chunk = /** @type {Array<Compiler.ExpressionTag | Compiler.Text>} */ (
+				definition.value
+			)[0];
 			element.tag =
 				chunk.type === 'Text'
 					? {
@@ -287,7 +285,7 @@ export default function element(parser) {
 						}
 					: chunk.expression;
 		} else {
-			element.tag = chunk.expression;
+			element.tag = get_attribute_expression(definition);
 		}
 	}
 
@@ -543,7 +541,7 @@ function read_attribute(parser) {
 				}
 			};
 
-			return create_attribute(name, start, parser.index, [expression]);
+			return create_attribute(name, start, parser.index, expression);
 		}
 	}
 
@@ -557,7 +555,7 @@ function read_attribute(parser) {
 	const colon_index = name.indexOf(':');
 	const type = colon_index !== -1 && get_directive_type(name.slice(0, colon_index));
 
-	/** @type {true | Array<Compiler.Text | Compiler.ExpressionTag>} */
+	/** @type {true | Compiler.ExpressionTag | Array<Compiler.Text | Compiler.ExpressionTag>} */
 	let value = true;
 	if (parser.eat('=')) {
 		parser.allow_whitespace();
@@ -589,7 +587,9 @@ function read_attribute(parser) {
 			};
 		}
 
-		const first_value = value === true ? undefined : value[0];
+		const first_value = value === true ? undefined : Array.isArray(value) ? value[0] : value;
+
+		/** @type {import('estree').Expression | null} */
 		let expression = null;
 
 		if (first_value) {
@@ -598,6 +598,8 @@ function read_attribute(parser) {
 			if (attribute_contains_text) {
 				e.directive_invalid_value(/** @type {number} */ (first_value.start));
 			} else {
+				// TODO throw a parser error in a future version here if this `[ExpressionTag]` instead of `ExpressionTag`,
+				// which means stringified value, which isn't allowed for some directives?
 				expression = first_value.expression;
 			}
 		}
@@ -662,6 +664,7 @@ function get_directive_type(name) {
 
 /**
  * @param {Parser} parser
+ * @return {Compiler.ExpressionTag | Array<Compiler.ExpressionTag | Compiler.Text>}
  */
 function read_attribute_value(parser) {
 	const quote_mark = parser.eat("'") ? "'" : parser.eat('"') ? '"' : null;
@@ -678,6 +681,7 @@ function read_attribute_value(parser) {
 		];
 	}
 
+	/** @type {Array<Compiler.ExpressionTag | Compiler.Text>} */
 	let value;
 	try {
 		value = read_sequence(
@@ -708,7 +712,12 @@ function read_attribute_value(parser) {
 	}
 
 	if (quote_mark) parser.index += 1;
-	return value;
+
+	if (quote_mark || value.length > 1 || value[0].type === 'Text') {
+		return value;
+	} else {
+		return value[0];
+	}
 }
 
 /**

packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js

@@ -3,6 +3,7 @@
 import { walk } from 'zimmerframe';
 import { get_possible_values } from './utils.js';
 import { regex_ends_with_whitespace, regex_starts_with_whitespace } from '../../patterns.js';
+import { get_attribute_chunks, is_text_attribute } from '../../../utils/ast.js';
 
 /**
  * @typedef {{
@@ -444,14 +445,11 @@ function attribute_matches(node, name, expected_value, operator, case_insensitiv
 		if (attribute.value === true) return operator === null;
 		if (expected_value === null) return true;
 
-		const chunks = attribute.value;
-		if (chunks.length === 1) {
-			const value = chunks[0];
-			if (value.type === 'Text') {
-				return test_attribute(operator, expected_value, case_insensitive, value.data);
-			}
+		if (is_text_attribute(attribute)) {
+			return test_attribute(operator, expected_value, case_insensitive, attribute.value[0].data);
 		}
 
+		const chunks = get_attribute_chunks(attribute.value);
 		const possible_values = new Set();
 
 		/** @type {string[]} */

packages/svelte/src/compiler/phases/2-analyze/index.js

@@ -9,7 +9,9 @@ import {
 	is_event_attribute,
 	is_text_attribute,
 	object,
-	unwrap_optional
+	unwrap_optional,
+	get_attribute_expression,
+	get_attribute_chunks
 } from '../../utils/ast.js';
 import * as b from '../../utils/builders.js';
 import { MathMLElements, ReservedKeywords, Runes, SVGElements } from '../constants.js';
@@ -597,19 +599,24 @@ export function analyze_component(root, source, options) {
 				}
 
 				if (class_attribute && class_attribute.value !== true) {
-					const chunks = class_attribute.value;
-
-					if (chunks.length === 1 && chunks[0].type === 'Text') {
-						chunks[0].data += ` ${analysis.css.hash}`;
+					if (is_text_attribute(class_attribute)) {
+						class_attribute.value[0].data += ` ${analysis.css.hash}`;
 					} else {
-						chunks.push({
+						/** @type {import('#compiler').Text} */
+						const css_text = {
 							type: 'Text',
 							data: ` ${analysis.css.hash}`,
 							raw: ` ${analysis.css.hash}`,
 							start: -1,
 							end: -1,
 							parent: null
-						});
+						};
+
+						if (Array.isArray(class_attribute.value)) {
+							class_attribute.value.push(css_text);
+						} else {
+							class_attribute.value = [class_attribute.value, css_text];
+						}
 					}
 				} else {
 					element.attributes.push(
@@ -1171,7 +1178,7 @@ const common_visitors = {
 
 		context.next();
 
-		node.metadata.dynamic = node.value.some((chunk) => {
+		node.metadata.dynamic = get_attribute_chunks(node.value).some((chunk) => {
 			if (chunk.type !== 'ExpressionTag') {
 				return false;
 			}
@@ -1192,8 +1199,7 @@ const common_visitors = {
 				context.state.analysis.uses_event_attributes = true;
 			}
 
-			const expression = node.value[0].expression;
-
+			const expression = get_attribute_expression(node);
 			const delegated_event = get_delegated_event(node.name.slice(2), expression, context);
 
 			if (delegated_event !== null) {
@@ -1228,7 +1234,7 @@ const common_visitors = {
 			}
 		} else {
 			context.next();
-			node.metadata.dynamic = node.value.some(
+			node.metadata.dynamic = get_attribute_chunks(node.value).some(
 				(node) => node.type === 'ExpressionTag' && node.metadata.dynamic
 			);
 		}

packages/svelte/src/compiler/phases/2-analyze/validation.js

@@ -7,6 +7,7 @@ import {
 import * as e from '../../errors.js';
 import {
 	extract_identifiers,
+	get_attribute_expression,
 	get_parent,
 	is_expression_attribute,
 	is_text_attribute,
@@ -33,9 +34,26 @@ import { Scope, get_rune } from '../scope.js';
 import { merge } from '../visitors.js';
 import { a11y_validators } from './a11y.js';
 
-/** @param {import('#compiler').Attribute} attribute */
-function validate_attribute(attribute) {
-	if (attribute.value === true || attribute.value.length === 1) return;
+/**
+ * @param {import('#compiler').Attribute} attribute
+ * @param {import('#compiler').ElementLike} parent
+ */
+function validate_attribute(attribute, parent) {
+	if (
+		Array.isArray(attribute.value) &&
+		attribute.value.length === 1 &&
+		attribute.value[0].type === 'ExpressionTag' &&
+		(parent.type === 'Component' ||
+			parent.type === 'SvelteComponent' ||
+			parent.type === 'SvelteSelf' ||
+			(parent.type === 'RegularElement' && is_custom_element_node(parent)))
+	) {
+		w.attribute_quoted(attribute);
+	}
+
+	if (attribute.value === true || !Array.isArray(attribute.value) || attribute.value.length === 1) {
+		return;
+	}
 
 	const is_quoted = attribute.value.at(-1)?.end !== attribute.end;
 
@@ -69,10 +87,10 @@ function validate_component(node, context) {
 
 		if (attribute.type === 'Attribute') {
 			if (context.state.analysis.runes) {
-				validate_attribute(attribute);
+				validate_attribute(attribute, node);
 
 				if (is_expression_attribute(attribute)) {
-					const expression = attribute.value[0].expression;
+					const expression = get_attribute_expression(attribute);
 					if (expression.type === 'SequenceExpression') {
 						let i = /** @type {number} */ (expression.start);
 						while (--i > 0) {
@@ -122,10 +140,10 @@ function validate_element(node, context) {
 			const is_expression = is_expression_attribute(attribute);
 
 			if (context.state.analysis.runes) {
-				validate_attribute(attribute);
+				validate_attribute(attribute, node);
 
 				if (is_expression) {
-					const expression = attribute.value[0].expression;
+					const expression = get_attribute_expression(attribute);
 					if (expression.type === 'SequenceExpression') {
 						let i = /** @type {number} */ (expression.start);
 						while (--i > 0) {
@@ -146,7 +164,7 @@ function validate_element(node, context) {
 					e.attribute_invalid_event_handler(attribute);
 				}
 
-				const value = attribute.value[0].expression;
+				const value = get_attribute_expression(attribute);
 				if (
 					value.type === 'Identifier' &&
 					value.name === attribute.name &&