fix: correctly set custom element props (#14508)

fixes #14391

In #13337 the "when to set as a property" logic for custom elements was adjusted. A bug was introduced during this, and it consists of several parts, of which the latter I'm not sure what's the best solution, hence opening this to discuss.

The problem is that during set_custom_element_data, get_setters is the only check done to differentiate between setting the value as a prop (has a setter) or as an attribute (doesn't have a setter).

The solution is to take into account whether or not the custom element is already registered, and defer getting (and caching) its setters until then. Instead, fall back to a "an object is always set as a prop" heuristic.

Author: dummdidumm

.changeset/sharp-shoes-look.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: take into account registration state when setting custom element props

packages/svelte/src/internal/client/dom/elements/attributes.js

@@ -201,7 +201,7 @@ export function set_xlink_attribute(dom, attribute, value) {
 }
 
 /**
- * @param {any} node
+ * @param {HTMLElement} node
  * @param {string} prop
  * @param {any} value
  */
@@ -216,10 +216,21 @@ export function set_custom_element_data(node, prop, value) {
 	set_active_reaction(null);
 	set_active_effect(null);
 	try {
-		if (get_setters(node).includes(prop)) {
+		if (
+			// Don't compute setters for custom elements while they aren't registered yet,
+			// because during their upgrade/instantiation they might add more setters.
+			// Instead, fall back to a simple "an object, then set as property" heuristic.
+			setters_cache.has(node.nodeName) || customElements.get(node.tagName.toLowerCase())
+				? get_setters(node).includes(prop)
+				: value && typeof value === 'object'
+		) {
+			// @ts-expect-error
 			node[prop] = value;
 		} else {
-			set_attribute(node, prop, value);
+			// We did getters etc checks already, stringify before passing to set_attribute
+			// to ensure it doesn't invoke the same logic again, and potentially populating
+			// the setters cache too early.
+			set_attribute(node, prop, value == null ? value : String(value));
 		}
 	} finally {
 		set_active_reaction(previous_reaction);
@@ -384,8 +395,9 @@ function get_setters(element) {
 	var setters = setters_cache.get(element.nodeName);
 	if (setters) return setters;
 	setters_cache.set(element.nodeName, (setters = []));
+
 	var descriptors;
-	var proto = get_prototype_of(element);
+	var proto = element; // In the case of custom elements there might be setters on the instance
 	var element_proto = Element.prototype;
 
 	// Stop at Element, from there on there's only unnecessary setters we're not interested in

packages/svelte/tests/runtime-browser/custom-elements-samples/late-ce-mount/_config.js

@@ -0,0 +1,25 @@
+import { flushSync } from 'svelte';
+import { test } from '../../assert';
+
+const tick = () => Promise.resolve();
+
+// Check that rendering a custom element and setting a property before it is registered
+// does not break the "when to set this as a property" logic
+export default test({
+	async test({ assert, target }) {
+		target.innerHTML = '<custom-element></custom-element>';
+		await tick();
+		await tick();
+
+		const ce_root = target.querySelector('custom-element').shadowRoot;
+
+		ce_root.querySelector('button')?.click();
+		flushSync();
+		await tick();
+		await tick();
+
+		const inner_ce_root = ce_root.querySelectorAll('set-property-before-mounted');
+		assert.htmlEqual(inner_ce_root[0].shadowRoot.innerHTML, 'object|{"foo":"bar"}');
+		assert.htmlEqual(inner_ce_root[1].shadowRoot.innerHTML, 'object|{"foo":"bar"}');
+	}
+});

packages/svelte/tests/runtime-browser/custom-elements-samples/late-ce-mount/main.svelte

@@ -0,0 +1,31 @@
+<svelte:options customElement="custom-element" />
+
+<script lang="ts">
+	import { onMount } from 'svelte';
+
+	class CustomElement extends HTMLElement {
+		constructor() {
+			super();
+			this.attachShadow({ mode: 'open' });
+			Object.defineProperty(this, 'property', {
+				set: (value) => {
+					this.shadowRoot.innerHTML = typeof value + '|' + JSON.stringify(value);
+				}
+			});
+		}
+	}
+
+	onMount(async () => {
+		customElements.define('set-property-before-mounted', CustomElement);
+	});
+
+	let property = $state();
+</script>
+
+<button onclick={() => (property = { foo: 'bar' })}>Update</button>
+<!-- one that's there before it's registered -->
+<set-property-before-mounted {property}></set-property-before-mounted>
+<!-- and one that's after registration but sets property to an object right away -->
+{#if property}
+	<set-property-before-mounted {property}></set-property-before-mounted>
+{/if}