From 386422941855fead58d00197d8ef73342572632c Mon Sep 17 00:00:00 2001
From: Paolo Ricciuti <ricciutipaolo@gmail.com>
Date: Mon, 16 Sep 2024 21:44:17 +0200
Subject: [PATCH] fix: separate `template_effect` for dynamic class/style
 directive with dynamic attributes (#13171)

* fix: separate `template_effect` for dynamic class/style directive with dynamic attributes

* fix: only move to `init` if it `has_call`

* fix: initialize spread `needs_isolation` based on if there are directives with `has_call`

* fix: revert splitting templates and generate deriveds instead

* small tweaks

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
---
 .changeset/nasty-eggs-walk.md                 |  5 ++++
 .../client/visitors/shared/element.js         | 26 ++++++++++++++-----
 .../_config.js                                | 19 ++++++++++++++
 .../main.svelte                               | 25 ++++++++++++++++++
 4 files changed, 69 insertions(+), 6 deletions(-)
 create mode 100644 .changeset/nasty-eggs-walk.md
 create mode 100644 packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive/_config.js
 create mode 100644 packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive/main.svelte

diff --git a/.changeset/nasty-eggs-walk.md b/.changeset/nasty-eggs-walk.md
new file mode 100644
index 0000000000..2e4b061dd5
--- /dev/null
+++ b/.changeset/nasty-eggs-walk.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: separate `template_effect` for dynamic class/style directive with dynamic attributes
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js
index 5ac6d0afa0..b46392bf11 100644
--- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js
+++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js
@@ -3,7 +3,7 @@
 /** @import { ComponentContext } from '../../types' */
 import { normalize_attribute } from '../../../../../../utils.js';
 import * as b from '../../../../../utils/builders.js';
-import { build_getter } from '../../utils.js';
+import { build_getter, create_derived } from '../../utils.js';
 import { build_template_literal, build_update } from './utils.js';
 
 /**
@@ -25,11 +25,20 @@ export function build_style_directives(
 	const state = context.state;
 
 	for (const directive of style_directives) {
+		const { has_state, has_call } = directive.metadata.expression;
+
 		let value =
 			directive.value === true
 				? build_getter({ name: directive.name, type: 'Identifier' }, context.state)
 				: build_attribute_value(directive.value, context).value;
 
+		if (has_call) {
+			const id = b.id(state.scope.generate('style_directive'));
+
+			state.init.push(b.const(id, create_derived(state, b.thunk(value))));
+			value = b.call('$.get', id);
+		}
+
 		const update = b.stmt(
 			b.call(
 				'$.set_style',
@@ -41,8 +50,6 @@ export function build_style_directives(
 			)
 		);
 
-		const { has_state, has_call } = directive.metadata.expression;
-
 		if (!is_attributes_reactive && has_call) {
 			state.init.push(build_update(update));
 		} else if (is_attributes_reactive || has_state || has_call) {
@@ -69,10 +76,17 @@ export function build_class_directives(
 ) {
 	const state = context.state;
 	for (const directive of class_directives) {
-		const value = /** @type {Expression} */ (context.visit(directive.expression));
-		const update = b.stmt(b.call('$.toggle_class', element_id, b.literal(directive.name), value));
-
 		const { has_state, has_call } = directive.metadata.expression;
+		let value = /** @type {Expression} */ (context.visit(directive.expression));
+
+		if (has_call) {
+			const id = b.id(state.scope.generate('class_directive'));
+
+			state.init.push(b.const(id, create_derived(state, b.thunk(value))));
+			value = b.call('$.get', id);
+		}
+
+		const update = b.stmt(b.call('$.toggle_class', element_id, b.literal(directive.name), value));
 
 		if (!is_attributes_reactive && has_call) {
 			state.init.push(build_update(update));
diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive/_config.js b/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive/_config.js
new file mode 100644
index 0000000000..81fb43595d
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive/_config.js
@@ -0,0 +1,19 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	test({ target, logs, assert }) {
+		const [div, div2] = target.querySelectorAll('div');
+		const button = target.querySelector('button');
+
+		assert.deepEqual(logs, ['called', 'called']);
+
+		// this is to assert that the order of the attributes is still not relevant
+		// and directives take precedence over generic attribute
+		assert.equal(div.classList.contains('dark'), false);
+		assert.equal(div2.style.color, 'red');
+
+		flushSync(() => button?.click());
+		assert.deepEqual(logs, ['called', 'called']);
+	}
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive/main.svelte b/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive/main.svelte
new file mode 100644
index 0000000000..ad0459eae0
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive/main.svelte
@@ -0,0 +1,25 @@
+<script>
+	let value = $state(0);
+
+	function dark(){
+		console.log('called')
+		return false;
+	}
+
+	function get_class(){
+		return 'dark';
+	}
+
+	function color(){
+		console.log('called')
+		return 'red';
+	}
+
+	function get_style(){
+		return 'color: green';
+	}
+</script>
+
+<div class:dark={dark()} class={get_class()}></div>
+<div style:color={color()} style={get_style()}></div>
+<button onclick={()=> value++}>{value}</button>