Skip to content

Commit b1dbb94

Browse files
committed
refine approach, add test
1 parent 39d65a4 commit b1dbb94

File tree

3 files changed

+73
-16
lines changed

3 files changed

+73
-16
lines changed

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

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ export function set_xlink_attribute(dom, attribute, value) {
146146
}
147147

148148
/**
149-
* @param {any} node
149+
* @param {HTMLElement} node
150150
* @param {string} prop
151151
* @param {any} value
152152
*/
@@ -161,10 +161,21 @@ export function set_custom_element_data(node, prop, value) {
161161
set_active_reaction(null);
162162
set_active_effect(null);
163163
try {
164-
if (get_setters(node).includes(prop)) {
164+
if (
165+
// Don't compute setters for custom elements while they aren't registered yet,
166+
// because during their upgrade/instantiation they might add more setters.
167+
// Instead, fall back to a simple "an object, then set as property" heuristic.
168+
setters_cache.has(node.nodeName) || customElements.get(node.tagName.toLowerCase())
169+
? get_setters(node).includes(prop)
170+
: value && typeof value === 'object'
171+
) {
172+
// @ts-expect-error
165173
node[prop] = value;
166174
} else {
167-
set_attribute(node, prop, value);
175+
// We did getters etc checks already, stringify before passing to set_attribute
176+
// to ensure it doesn't invoke the same logic again, and potentially populating
177+
// the setters cache too early.
178+
set_attribute(node, prop, value == null ? value : String(value));
168179
}
169180
} finally {
170181
set_active_reaction(previous_reaction);
@@ -323,21 +334,11 @@ var setters_cache = new Map();
323334

324335
/** @param {Element} element */
325336
function get_setters(element) {
326-
var name = element.nodeName;
327-
var setters = setters_cache.get(name);
328-
329-
if (setters) return setters;
330-
331-
setters = [];
332-
333-
// Don't cache the result for custom elements while they aren't connected yet,
334-
// because during their upgrade they might add more setters
335-
if (!name.includes('-') || element.isConnected) {
336-
setters_cache.set(name, setters);
337-
}
337+
var setters = setters_cache.get(element.nodeName);
338+
setters_cache.set(element.nodeName, (setters = []));
338339

339340
var descriptors;
340-
var proto = element;
341+
var proto = element; // In the case of custom elements there might be setters on the instance
341342
var element_proto = Element.prototype;
342343

343344
// Stop at Element, from there on there's only unnecessary setters we're not interested in
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { flushSync } from 'svelte';
2+
import { test } from '../../assert';
3+
4+
const tick = () => Promise.resolve();
5+
6+
// Check that rendering a custom element and setting a property before it is registered
7+
// does not break the "when to set this as a property" logic
8+
export default test({
9+
async test({ assert, target }) {
10+
target.innerHTML = '<custom-element></custom-element>';
11+
await tick();
12+
await tick();
13+
14+
const ce_root = target.querySelector('custom-element').shadowRoot;
15+
16+
ce_root.querySelector('button')?.click();
17+
flushSync();
18+
await tick();
19+
await tick();
20+
21+
const inner_ce_root = ce_root.querySelectorAll('set-property-before-mounted');
22+
assert.htmlEqual(inner_ce_root[0].shadowRoot.innerHTML, 'object|{"foo":"bar"}');
23+
assert.htmlEqual(inner_ce_root[1].shadowRoot.innerHTML, 'object|{"foo":"bar"}');
24+
}
25+
});
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<svelte:options customElement="custom-element" />
2+
3+
<script lang="ts">
4+
import { onMount } from 'svelte';
5+
6+
class CustomElement extends HTMLElement {
7+
constructor() {
8+
super();
9+
this.attachShadow({ mode: 'open' });
10+
Object.defineProperty(this, 'property', {
11+
set: (value) => {
12+
this.shadowRoot.innerHTML = typeof value + '|' + JSON.stringify(value);
13+
}
14+
});
15+
}
16+
}
17+
18+
onMount(async () => {
19+
customElements.define('set-property-before-mounted', CustomElement);
20+
});
21+
22+
let property = $state();
23+
</script>
24+
25+
<button onclick={() => (property = { foo: 'bar' })}>Update</button>
26+
<!-- one that's there before it's registered -->
27+
<set-property-before-mounted {property}></set-property-before-mounted>
28+
<!-- and one that's after registration but sets property to an object right away -->
29+
{#if property}
30+
<set-property-before-mounted {property}></set-property-before-mounted>
31+
{/if}

0 commit comments

Comments
 (0)