From a2539cfe1fd763e33c4508dd0ddd92714a910641 Mon Sep 17 00:00:00 2001
From: Simon H <5968653+dummdidumm@users.noreply.github.com>
Date: Tue, 10 Dec 2024 22:56:50 +0100
Subject: [PATCH] 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.
---
 .changeset/sharp-shoes-look.md                |  5 +++
 .../client/dom/elements/attributes.js         | 20 +++++++++---
 .../late-ce-mount/_config.js                  | 25 +++++++++++++++
 .../late-ce-mount/main.svelte                 | 31 +++++++++++++++++++
 4 files changed, 77 insertions(+), 4 deletions(-)
 create mode 100644 .changeset/sharp-shoes-look.md
 create mode 100644 packages/svelte/tests/runtime-browser/custom-elements-samples/late-ce-mount/_config.js
 create mode 100644 packages/svelte/tests/runtime-browser/custom-elements-samples/late-ce-mount/main.svelte

diff --git a/.changeset/sharp-shoes-look.md b/.changeset/sharp-shoes-look.md
new file mode 100644
index 0000000000..2caf6d421b
--- /dev/null
+++ b/.changeset/sharp-shoes-look.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: take into account registration state when setting custom element props
diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js
index 0276069eee..8f6588e94a 100644
--- a/packages/svelte/src/internal/client/dom/elements/attributes.js
+++ b/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
diff --git a/packages/svelte/tests/runtime-browser/custom-elements-samples/late-ce-mount/_config.js b/packages/svelte/tests/runtime-browser/custom-elements-samples/late-ce-mount/_config.js
new file mode 100644
index 0000000000..0679c820ba
--- /dev/null
+++ b/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"}');
+	}
+});
diff --git a/packages/svelte/tests/runtime-browser/custom-elements-samples/late-ce-mount/main.svelte b/packages/svelte/tests/runtime-browser/custom-elements-samples/late-ce-mount/main.svelte
new file mode 100644
index 0000000000..09a139e642
--- /dev/null
+++ b/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}