Merge branch 'main' into tweak-reactive-statements

Author: Rich-Harris

.changeset/serious-glasses-kiss.md

@@ -1,5 +1,15 @@
 # svelte
 
+## 5.20.3
+
+### Patch Changes
+
+- fix: allow `@const` inside `#key` ([#15377](https://github.com/sveltejs/svelte/pull/15377))
+
+- fix: remove unnecessary `?? ''` on some expressions ([#15287](https://github.com/sveltejs/svelte/pull/15287))
+
+- fix: correctly override class attributes with class directives ([#15352](https://github.com/sveltejs/svelte/pull/15352))
+
 ## 5.20.2
 
 ### Patch Changes

packages/svelte/CHANGELOG.md

@@ -2,7 +2,7 @@
   "name": "svelte",
   "description": "Cybernetically enhanced web apps",
   "license": "MIT",
-  "version": "5.20.2",
+  "version": "5.20.3",
   "type": "module",
   "types": "./types/index.d.ts",
   "engines": {

packages/svelte/package.json

@@ -267,7 +267,6 @@ export function analyze_module(ast, options) {
 			expression: null,
 			function_depth: 0,
 			has_props_rune: false,
-			instance_scope: /** @type {any} */ (null),
 			options: /** @type {ValidatedCompileOptions} */ (options),
 			parent_element: null,
 			reactive_statement: null
@@ -619,7 +618,6 @@ export function analyze_component(root, source, options) {
 				expression: null,
 				derived_state: [],
 				function_depth: scope.function_depth,
-				instance_scope: instance.scope,
 				reactive_statement: null
 			};
 
@@ -682,7 +680,6 @@ export function analyze_component(root, source, options) {
 				parent_element: null,
 				has_props_rune: false,
 				ast_type: ast === instance.ast ? 'instance' : ast === template.ast ? 'template' : 'module',
-				instance_scope: instance.scope,
 				reactive_statement: null,
 				component_slots: new Set(),
 				expression: null,
@@ -764,66 +761,40 @@ export function analyze_component(root, source, options) {
 		if (!should_ignore_unused) {
 			warn_unused(analysis.css.ast);
 		}
+	}
 
-		outer: for (const node of analysis.elements) {
-			if (node.metadata.scoped) {
-				// Dynamic elements in dom mode always use spread for attributes and therefore shouldn't have a class attribute added to them
-				// TODO this happens during the analysis phase, which shouldn't know anything about client vs server
-				if (node.type === 'SvelteElement' && options.generate === 'client') continue;
-
-				/** @type {AST.Attribute | undefined} */
-				let class_attribute = undefined;
-
-				for (const attribute of node.attributes) {
-					if (attribute.type === 'SpreadAttribute') {
-						// The spread method appends the hash to the end of the class attribute on its own
-						continue outer;
-					}
+	for (const node of analysis.elements) {
+		if (node.metadata.scoped && is_custom_element_node(node)) {
+			mark_subtree_dynamic(node.metadata.path);
+		}
 
-					if (attribute.type !== 'Attribute') continue;
-					if (attribute.name.toLowerCase() !== 'class') continue;
-					// The dynamic class method appends the hash to the end of the class attribute on its own
-					if (attribute.metadata.needs_clsx) continue outer;
+		let has_class = false;
+		let has_spread = false;
+		let has_class_directive = false;
 
-					class_attribute = attribute;
-				}
+		for (const attribute of node.attributes) {
+			// The spread method appends the hash to the end of the class attribute on its own
+			if (attribute.type === 'SpreadAttribute') {
+				has_spread = true;
+				break;
+			}
+			has_class_directive ||= attribute.type === 'ClassDirective';
+			has_class ||= attribute.type === 'Attribute' && attribute.name.toLowerCase() === 'class';
+		}
 
-				if (class_attribute && class_attribute.value !== true) {
-					if (is_text_attribute(class_attribute)) {
-						class_attribute.value[0].data += ` ${analysis.css.hash}`;
-					} else {
-						/** @type {AST.Text} */
-						const css_text = {
-							type: 'Text',
-							data: ` ${analysis.css.hash}`,
-							raw: ` ${analysis.css.hash}`,
-							start: -1,
-							end: -1
-						};
-
-						if (Array.isArray(class_attribute.value)) {
-							class_attribute.value.push(css_text);
-						} else {
-							class_attribute.value = [class_attribute.value, css_text];
-						}
-					}
-				} else {
-					node.attributes.push(
-						create_attribute('class', -1, -1, [
-							{
-								type: 'Text',
-								data: analysis.css.hash,
-								raw: analysis.css.hash,
-								start: -1,
-								end: -1
-							}
-						])
-					);
-					if (is_custom_element_node(node) && node.attributes.length === 1) {
-						mark_subtree_dynamic(node.metadata.path);
+		// We need an empty class to generate the set_class() or class="" correctly
+		if (!has_spread && !has_class && (node.metadata.scoped || has_class_directive)) {
+			node.attributes.push(
+				create_attribute('class', -1, -1, [
+					{
+						type: 'Text',
+						data: '',
+						raw: '',
+						start: -1,
+						end: -1
 					}
-				}
-			}
+				])
+			);
 		}
 	}
 

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

@@ -22,7 +22,6 @@ export interface AnalysisState {
 	function_depth: number;
 
 	// legacy stuff
-	instance_scope: Scope;
 	reactive_statement: null | ReactiveStatement;
 }
 

packages/svelte/src/compiler/phases/2-analyze/types.d.ts

@@ -25,6 +25,7 @@ export function ConstTag(node, context) {
 			grand_parent?.type !== 'AwaitBlock' &&
 			grand_parent?.type !== 'SnippetBlock' &&
 			grand_parent?.type !== 'SvelteBoundary' &&
+			grand_parent?.type !== 'KeyBlock' &&
 			((grand_parent?.type !== 'RegularElement' && grand_parent?.type !== 'SvelteElement') ||
 				!grand_parent.attributes.some((a) => a.type === 'Attribute' && a.name === 'slot')))
 	) {

packages/svelte/src/compiler/phases/2-analyze/visitors/ConstTag.js

@@ -1,4 +1,4 @@
-/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, Statement } from 'estree' */
+/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, ObjectExpression, Statement } from 'estree' */
 /** @import { AST } from '#compiler' */
 /** @import { SourceLocation } from '#shared' */
 /** @import { ComponentClientTransformState, ComponentContext } from '../types' */
@@ -20,9 +20,9 @@ import { build_getter } from '../utils.js';
 import {
 	get_attribute_name,
 	build_attribute_value,
-	build_class_directives,
 	build_style_directives,
-	build_set_attributes
+	build_set_attributes,
+	build_set_class
 } from './shared/element.js';
 import { process_children } from './shared/fragment.js';
 import {
@@ -223,13 +223,13 @@ export function RegularElement(node, context) {
 
 		build_set_attributes(
 			attributes,
+			class_directives,
 			context,
 			node,
 			node_id,
 			attributes_id,
 			(node.metadata.svg || node.metadata.mathml || is_custom_element_node(node)) && b.true,
-			is_custom_element_node(node) && b.true,
-			context.state
+			is_custom_element_node(node) && b.true
 		);
 
 		// If value binding exists, that one takes care of calling $.init_select
@@ -270,13 +270,22 @@ export function RegularElement(node, context) {
 				continue;
 			}
 
+			const name = get_attribute_name(node, attribute);
 			if (
 				!is_custom_element &&
 				!cannot_be_set_statically(attribute.name) &&
-				(attribute.value === true || is_text_attribute(attribute))
+				(attribute.value === true || is_text_attribute(attribute)) &&
+				(name !== 'class' || class_directives.length === 0)
 			) {
-				const name = get_attribute_name(node, attribute);
-				const value = is_text_attribute(attribute) ? attribute.value[0].data : true;
+				let value = is_text_attribute(attribute) ? attribute.value[0].data : true;
+
+				if (name === 'class' && node.metadata.scoped && context.state.analysis.css.hash) {
+					if (value === true || value === '') {
+						value = context.state.analysis.css.hash;
+					} else {
+						value += ' ' + context.state.analysis.css.hash;
+					}
+				}
 
 				if (name !== 'class' || value) {
 					context.state.template.push(
@@ -290,15 +299,22 @@ export function RegularElement(node, context) {
 				continue;
 			}
 
-			const is = is_custom_element
-				? build_custom_element_attribute_update_assignment(node_id, attribute, context)
-				: build_element_attribute_update_assignment(node, node_id, attribute, attributes, context);
+			const is =
+				is_custom_element && name !== 'class'
+					? build_custom_element_attribute_update_assignment(node_id, attribute, context)
+					: build_element_attribute_update_assignment(
+							node,
+							node_id,
+							attribute,
+							attributes,
+							class_directives,
+							context
+						);
 			if (is) is_attributes_reactive = true;
 		}
 	}
 
-	// class/style directives must be applied last since they could override class/style attributes
-	build_class_directives(class_directives, node_id, context, is_attributes_reactive);
+	// style directives must be applied last since they could override class/style attributes
 	build_style_directives(style_directives, node_id, context, is_attributes_reactive);
 
 	if (
@@ -491,6 +507,27 @@ function setup_select_synchronization(value_binding, context) {
 	);
 }
 
+/**
+ * @param {AST.ClassDirective[]} class_directives
+ * @param {ComponentContext} context
+ * @return {ObjectExpression}
+ */
+export function build_class_directives_object(class_directives, context) {
+	let properties = [];
+
+	for (const d of class_directives) {
+		let expression = /** @type Expression */ (context.visit(d.expression));
+
+		if (d.metadata.expression.has_call) {
+			expression = get_expression_id(context.state, expression);
+		}
+
+		properties.push(b.init(d.name, expression));
+	}
+
+	return b.object(properties);
+}
+
 /**
  * Serializes an assignment to an element property by adding relevant statements to either only
  * the init or the the init and update arrays, depending on whether or not the value is dynamic.
@@ -517,6 +554,7 @@ function setup_select_synchronization(value_binding, context) {
  * @param {Identifier} node_id
  * @param {AST.Attribute} attribute
  * @param {Array<AST.Attribute | AST.SpreadAttribute>} attributes
+ * @param {AST.ClassDirective[]} class_directives
  * @param {ComponentContext} context
  * @returns {boolean}
  */
@@ -525,6 +563,7 @@ function build_element_attribute_update_assignment(
 	node_id,
 	attribute,
 	attributes,
+	class_directives,
 	context
 ) {
 	const state = context.state;
@@ -563,19 +602,15 @@ function build_element_attribute_update_assignment(
 	let update;
 
 	if (name === 'class') {
-		if (attribute.metadata.needs_clsx) {
-			value = b.call('$.clsx', value);
-		}
-
-		update = b.stmt(
-			b.call(
-				is_svg ? '$.set_svg_class' : is_mathml ? '$.set_mathml_class' : '$.set_class',
-				node_id,
-				value,
-				attribute.metadata.needs_clsx && context.state.analysis.css.hash
-					? b.literal(context.state.analysis.css.hash)
-					: undefined
-			)
+		return build_set_class(
+			element,
+			node_id,
+			attribute,
+			value,
+			has_state,
+			class_directives,
+			context,
+			!is_svg && !is_mathml
 		);
 	} else if (name === 'value') {
 		update = b.stmt(b.call('$.set_value', node_id, value));
@@ -639,14 +674,6 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co
 	const name = attribute.name; // don't lowercase, as we set the element's property, which might be case sensitive
 	let { value, has_state } = build_attribute_value(attribute.value, context);
 
-	// We assume that noone's going to redefine the semantics of the class attribute on custom elements, i.e. it's still used for CSS classes
-	if (name === 'class' && attribute.metadata.needs_clsx) {
-		if (context.state.analysis.css.hash) {
-			value = b.array([value, b.literal(context.state.analysis.css.hash)]);
-		}
-		value = b.call('$.clsx', value);
-	}
-
 	const update = b.stmt(b.call('$.set_custom_element_data', node_id, b.literal(name), value));
 
 	if (has_state) {