From 39d65a47fce0d2080326b33e1f33f3c3ffbca6a0 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 2 Dec 2024 19:19:31 +0100 Subject: [PATCH 1/4] fix: correctly set custom element props WIP --- .../internal/client/dom/elements/attributes.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index d927af543ff2..26299889ddf9 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -323,11 +323,21 @@ var setters_cache = new Map(); /** @param {Element} element */ function get_setters(element) { - var setters = setters_cache.get(element.nodeName); + var name = element.nodeName; + var setters = setters_cache.get(name); + if (setters) return setters; - setters_cache.set(element.nodeName, (setters = [])); + + setters = []; + + // Don't cache the result for custom elements while they aren't connected yet, + // because during their upgrade they might add more setters + if (!name.includes('-') || element.isConnected) { + setters_cache.set(name, setters); + } + var descriptors; - var proto = get_prototype_of(element); + var proto = element; var element_proto = Element.prototype; // Stop at Element, from there on there's only unnecessary setters we're not interested in From b1dbb94d30fb79bb9c194fbeb978b796ec25e268 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 5 Dec 2024 15:18:02 +0100 Subject: [PATCH 2/4] refine approach, add test --- .../client/dom/elements/attributes.js | 33 ++++++++++--------- .../late-ce-mount/_config.js | 25 ++++++++++++++ .../late-ce-mount/main.svelte | 31 +++++++++++++++++ 3 files changed, 73 insertions(+), 16 deletions(-) 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/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 26299889ddf9..173ea844b264 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -146,7 +146,7 @@ export function set_xlink_attribute(dom, attribute, value) { } /** - * @param {any} node + * @param {HTMLElement} node * @param {string} prop * @param {any} value */ @@ -161,10 +161,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); @@ -323,21 +334,11 @@ var setters_cache = new Map(); /** @param {Element} element */ function get_setters(element) { - var name = element.nodeName; - var setters = setters_cache.get(name); - - if (setters) return setters; - - setters = []; - - // Don't cache the result for custom elements while they aren't connected yet, - // because during their upgrade they might add more setters - if (!name.includes('-') || element.isConnected) { - setters_cache.set(name, setters); - } + var setters = setters_cache.get(element.nodeName); + setters_cache.set(element.nodeName, (setters = [])); var descriptors; - var proto = 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 000000000000..0679c820ba8c --- /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 = ''; + 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 000000000000..09a139e642b1 --- /dev/null +++ b/packages/svelte/tests/runtime-browser/custom-elements-samples/late-ce-mount/main.svelte @@ -0,0 +1,31 @@ + + + + + + + + +{#if property} + +{/if} From 60c516db5df15166c77f31baa62118bfb3369426 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 5 Dec 2024 15:19:26 +0100 Subject: [PATCH 3/4] changeset --- .changeset/sharp-shoes-look.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/sharp-shoes-look.md diff --git a/.changeset/sharp-shoes-look.md b/.changeset/sharp-shoes-look.md new file mode 100644 index 000000000000..2caf6d421b99 --- /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 From eabcb3aa2400bb27b1e5d7d813af6280dfe246b8 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 10 Dec 2024 22:37:28 +0100 Subject: [PATCH 4/4] Update packages/svelte/src/internal/client/dom/elements/attributes.js --- packages/svelte/src/internal/client/dom/elements/attributes.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 173ea844b264..89aa76e78d41 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -335,6 +335,7 @@ var setters_cache = new Map(); /** @param {Element} element */ function get_setters(element) { var setters = setters_cache.get(element.nodeName); + if (setters) return setters; setters_cache.set(element.nodeName, (setters = [])); var descriptors;