fix: properly hydrate dynamic css props components and remove element removal (sveltejs#16118)

Author: paoloricciuti

.changeset/twelve-foxes-smell.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: properly hydrate dynamic css props components and remove element removal

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

@@ -1,32 +1,19 @@
-/** @import { Expression } from 'estree' */
 /** @import { AST } from '#compiler' */
 /** @import { ComponentContext } from '../types' */
-import * as b from '#compiler/builders';
+import { regex_is_valid_identifier } from '../../../patterns.js';
 import { build_component } from './shared/component.js';
 
 /**
  * @param {AST.Component} node
  * @param {ComponentContext} context
  */
 export function Component(node, context) {
-	if (node.metadata.dynamic) {
-		// Handle dynamic references to what seems like static inline components
-		const component = build_component(node, '$$component', context, b.id('$$anchor'));
-		context.state.init.push(
-			b.stmt(
-				b.call(
-					'$.component',
-					context.state.node,
-					// TODO use untrack here to not update when binding changes?
-					// Would align with Svelte 4 behavior, but it's arguably nicer/expected to update this
-					b.thunk(/** @type {Expression} */ (context.visit(b.member_id(node.name)))),
-					b.arrow([b.id('$$anchor'), b.id('$$component')], b.block([component]))
-				)
-			)
-		);
-		return;
-	}
-
-	const component = build_component(node, node.name, context);
+	const component = build_component(
+		node,
+		// if it's not dynamic we will just use the node name, if it is dynamic we will use the node name
+		// only if it's a valid identifier, otherwise we will use a default name
+		!node.metadata.dynamic || regex_is_valid_identifier.test(node.name) ? node.name : '$$component',
+		context
+	);
 	context.state.init.push(component);
 }

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

@@ -13,10 +13,13 @@ import { determine_slot } from '../../../../../utils/slot.js';
  * @param {AST.Component | AST.SvelteComponent | AST.SvelteSelf} node
  * @param {string} component_name
  * @param {ComponentContext} context
- * @param {Expression} anchor
  * @returns {Statement}
  */
-export function build_component(node, component_name, context, anchor = context.state.node) {
+export function build_component(node, component_name, context) {
+	/**
+	 * @type {Expression}
+	 */
+	const anchor = context.state.node;
 	/** @type {Array<Property[] | Expression>} */
 	const props_and_spreads = [];
 	/** @type {Array<() => void>} */
@@ -411,7 +414,7 @@ export function build_component(node, component_name, context, anchor = context.
 			// TODO We can remove this ternary once we remove legacy mode, since in runes mode dynamic components
 			// will be handled separately through the `$.component` function, and then the component name will
 			// always be referenced through just the identifier here.
-			node.type === 'SvelteComponent'
+			node.type === 'SvelteComponent' || (node.type === 'Component' && node.metadata.dynamic)
 				? component_name
 				: /** @type {Expression} */ (context.visit(b.member_id(component_name))),
 			node_id,
@@ -429,14 +432,18 @@ export function build_component(node, component_name, context, anchor = context.
 
 	const statements = [...snippet_declarations];
 
-	if (node.type === 'SvelteComponent') {
+	if (node.type === 'SvelteComponent' || (node.type === 'Component' && node.metadata.dynamic)) {
 		const prev = fn;
 
 		fn = (node_id) => {
 			return b.call(
 				'$.component',
 				node_id,
-				b.thunk(/** @type {Expression} */ (context.visit(node.expression))),
+				b.thunk(
+					/** @type {Expression} */ (
+						context.visit(node.type === 'Component' ? b.member_id(node.name) : node.expression)
+					)
+				),
 				b.arrow(
 					[b.id('$$anchor'), b.id(component_name)],
 					b.block([...binding_initializers, b.stmt(prev(b.id('$$anchor')))])

packages/svelte/src/internal/client/dom/blocks/css-props.js

@@ -26,8 +26,4 @@ export function css_props(element, get_styles) {
 			}
 		}
 	});
-
-	teardown(() => {
-		element.remove();
-	});
 }

packages/svelte/tests/runtime-browser/samples/css-props-dynamic-component/A.svelte

@@ -0,0 +1,7 @@
+<div>a</div>
+
+<style>
+	div{
+		color: var(--prop);
+	}
+</style>

packages/svelte/tests/runtime-browser/samples/css-props-dynamic-component/B.svelte

@@ -0,0 +1,7 @@
+<div>b</div>
+
+<style>
+	div{
+		color: var(--prop);
+	}
+</style>

packages/svelte/tests/runtime-browser/samples/css-props-dynamic-component/_config.js

@@ -0,0 +1,16 @@
+import { test } from '../../assert';
+import { flushSync } from 'svelte';
+
+export default test({
+	warnings: [],
+	async test({ assert, target }) {
+		const btn = target.querySelector('button');
+		let div = /** @type {HTMLElement} */ (target.querySelector('div'));
+		assert.equal(getComputedStyle(div).color, 'rgb(255, 0, 0)');
+		flushSync(() => {
+			btn?.click();
+		});
+		div = /** @type {HTMLElement} */ (target.querySelector('div'));
+		assert.equal(getComputedStyle(div).color, 'rgb(255, 0, 0)');
+	}
+});

packages/svelte/tests/runtime-browser/samples/css-props-dynamic-component/main.svelte

@@ -0,0 +1,11 @@
+<script>
+	import A from "./B.svelte";
+	import B from "./A.svelte";
+	let value = $state(0);
+
+	let Comp = $derived(value % 2 === 0 ? A : B);
+</script>
+
+<button onclick={()=>value++}>click</button>
+
+<Comp --prop="red"/>