chore: rewrite set_style() to handle directives

.changeset/strange-planes-shout.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+chore: rewrite set_style() to handle directives

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

@@ -769,17 +769,24 @@ export function analyze_component(root, source, options) {
 		}
 
 		let has_class = false;
+		let has_style = false;
 		let has_spread = false;
 		let has_class_directive = false;
+		let has_style_directive = false;
 
 		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;
+			} else if (attribute.type === 'Attribute') {
+				has_class ||= attribute.name.toLowerCase() === 'class';
+				has_style ||= attribute.name.toLowerCase() === 'style';
+			} else if (attribute.type === 'ClassDirective') {
+				has_class_directive = true;
+			} else if (attribute.type === 'StyleDirective') {
+				has_style_directive = true;
 			}
-			has_class_directive ||= attribute.type === 'ClassDirective';
-			has_class ||= attribute.type === 'Attribute' && attribute.name.toLowerCase() === 'class';
 		}
 
 		// We need an empty class to generate the set_class() or class="" correctly
@@ -796,6 +803,21 @@ export function analyze_component(root, source, options) {
 				])
 			);
 		}
+
+		// We need an empty style to generate the set_style() correctly
+		if (!has_spread && !has_style && has_style_directive) {
+			node.attributes.push(
+				create_attribute('style', -1, -1, [
+					{
+						type: 'Text',
+						data: '',
+						raw: '',
+						start: -1,
+						end: -1
+					}
+				])
+			);
+		}
 	}
 
 	// TODO

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

@@ -1,4 +1,4 @@
-/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, ObjectExpression, Statement } from 'estree' */
+/** @import { ArrayExpression, 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_style_directives,
 	build_set_attributes,
-	build_set_class
+	build_set_class,
+	build_set_style
 } from './shared/element.js';
 import { process_children } from './shared/fragment.js';
 import {
@@ -215,15 +215,13 @@ export function RegularElement(node, context) {
 
 	const node_id = context.state.node;
 
-	// Then do attributes
-	let is_attributes_reactive = has_spread;
-
 	if (has_spread) {
 		const attributes_id = b.id(context.state.scope.generate('attributes'));
 
 		build_set_attributes(
 			attributes,
 			class_directives,
+			style_directives,
 			context,
 			node,
 			node_id,
@@ -275,7 +273,8 @@ export function RegularElement(node, context) {
 				!is_custom_element &&
 				!cannot_be_set_statically(attribute.name) &&
 				(attribute.value === true || is_text_attribute(attribute)) &&
-				(name !== 'class' || class_directives.length === 0)
+				(name !== 'class' || class_directives.length === 0) &&
+				(name !== 'style' || style_directives.length === 0)
 			) {
 				let value = is_text_attribute(attribute) ? attribute.value[0].data : true;
 
@@ -299,24 +298,22 @@ export function RegularElement(node, context) {
 				continue;
 			}
 
-			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;
+			if (is_custom_element && name !== 'class' && name !== 'style') {
+				build_custom_element_attribute_update_assignment(node_id, attribute, context);
+			} else {
+				build_element_attribute_update_assignment(
+					node,
+					node_id,
+					attribute,
+					attributes,
+					class_directives,
+					style_directives,
+					context
+				);
+			}
 		}
 	}
 
-	// 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 (
 		is_load_error_element(node.name) &&
 		(has_spread || has_use || lookup.has('onload') || lookup.has('onerror'))
@@ -528,6 +525,35 @@ export function build_class_directives_object(class_directives, context) {
 	return b.object(properties);
 }
 
+/**
+ * @param {AST.StyleDirective[]} style_directives
+ * @param {ComponentContext} context
+ * @return {ObjectExpression | ArrayExpression}}
+ */
+export function build_style_directives_object(style_directives, context) {
+	let normal_properties = [];
+	let important_properties = [];
+
+	for (const directive of style_directives) {
+		let expression =
+			directive.value === true
+				? build_getter({ name: directive.name, type: 'Identifier' }, context.state)
+				: build_attribute_value(directive.value, context, (value, metadata) =>
+						metadata.has_call ? get_expression_id(context.state, value) : value
+					).value;
+		const property = b.init(directive.name, expression);
+		if (directive.modifiers.includes('important')) {
+			important_properties.push(property);
+		} else {
+			normal_properties.push(property);
+		}
+	}
+
+	return important_properties.length
+		? b.array([b.object(normal_properties), b.object(important_properties)])
+		: b.object(normal_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.
@@ -555,6 +581,7 @@ export function build_class_directives_object(class_directives, context) {
  * @param {AST.Attribute} attribute
  * @param {Array<AST.Attribute | AST.SpreadAttribute>} attributes
  * @param {AST.ClassDirective[]} class_directives
+ * @param {AST.StyleDirective[]} style_directives
  * @param {ComponentContext} context
  * @returns {boolean}
  */
@@ -564,6 +591,7 @@ function build_element_attribute_update_assignment(
 	attribute,
 	attributes,
 	class_directives,
+	style_directives,
 	context
 ) {
 	const state = context.state;
@@ -612,6 +640,8 @@ function build_element_attribute_update_assignment(
 			context,
 			!is_svg && !is_mathml
 		);
+	} else if (name === 'style') {
+		return build_set_style(node_id, value, has_state, style_directives, context);
 	} else if (name === 'value') {
 		update = b.stmt(b.call('$.set_value', node_id, value));
 	} else if (name === 'checked') {

packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js

@@ -5,12 +5,7 @@ import { dev, locator } from '../../../../state.js';
 import { is_text_attribute } from '../../../../utils/ast.js';
 import * as b from '../../../../utils/builders.js';
 import { determine_namespace_for_children } from '../../utils.js';
-import {
-	build_attribute_value,
-	build_set_attributes,
-	build_set_class,
-	build_style_directives
-} from './shared/element.js';
+import { build_attribute_value, build_set_attributes, build_set_class } from './shared/element.js';
 import { build_render_statement, get_expression_id } from './shared/utils.js';
 
 /**
@@ -77,9 +72,6 @@ export function SvelteElement(node, context) {
 	// Let bindings first, they can be used on attributes
 	context.state.init.push(...lets); // create computeds in the outer context; the dynamic element is the single child of this slot
 
-	// Then do attributes
-	let is_attributes_reactive = false;
-
 	if (
 		attributes.length === 1 &&
 		attributes[0].type === 'Attribute' &&
@@ -93,7 +85,7 @@ export function SvelteElement(node, context) {
 			(value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value)
 		);
 
-		is_attributes_reactive = build_set_class(
+		build_set_class(
 			node,
 			element_id,
 			attributes[0],
@@ -108,9 +100,10 @@ export function SvelteElement(node, context) {
 
 		// Always use spread because we don't know whether the element is a custom element or not,
 		// therefore we need to do the "how to set an attribute" logic at runtime.
-		is_attributes_reactive = build_set_attributes(
+		build_set_attributes(
 			attributes,
 			class_directives,
+			style_directives,
 			inner_context,
 			node,
 			element_id,
@@ -120,9 +113,6 @@ export function SvelteElement(node, context) {
 		);
 	}
 
-	// style directives must be applied last since they could override class/style attributes
-	build_style_directives(style_directives, element_id, inner_context, is_attributes_reactive);
-
 	const get_tag = b.thunk(/** @type {Expression} */ (context.visit(node.tag)));
 
 	if (dev) {