Skip to content

Commit cdec39a

Browse files
trueadmdummdidumm
andauthored
fix: ensure custom element attribute/prop changes are in their own context (#14016)
Fixes #13848. When we set custom element attributes/props, we should be doing so without the current effect/reaction active. Otherwise, the custom element lifecycle might attach effects/dependencies to the wrong reaction and all manner of things can incorrectly occur --------- Co-authored-by: Simon Holthausen <[email protected]>
1 parent 4c6255f commit cdec39a

File tree

4 files changed

+84
-4
lines changed

4 files changed

+84
-4
lines changed

.changeset/wild-parents-begin.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: ensure custom element attribute/prop changes are in their own context

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

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ import * as w from '../../warnings.js';
77
import { LOADING_ATTR_SYMBOL } from '../../constants.js';
88
import { queue_idle_task, queue_micro_task } from '../task.js';
99
import { is_capture_event, is_delegated, normalize_attribute } from '../../../../utils.js';
10+
import {
11+
active_effect,
12+
active_reaction,
13+
set_active_effect,
14+
set_active_reaction
15+
} from '../../runtime.js';
1016

1117
/**
1218
* The value/checked attribute in the template actually corresponds to the defaultValue property, so we need
@@ -145,10 +151,24 @@ export function set_xlink_attribute(dom, attribute, value) {
145151
* @param {any} value
146152
*/
147153
export function set_custom_element_data(node, prop, value) {
148-
if (get_setters(node).includes(prop)) {
149-
node[prop] = value;
150-
} else {
151-
set_attribute(node, prop, value);
154+
// We need to ensure that setting custom element props, which can
155+
// invoke lifecycle methods on other custom elements, does not also
156+
// associate those lifecycle methods with the current active reaction
157+
// or effect
158+
var previous_reaction = active_reaction;
159+
var previous_effect = active_effect;
160+
161+
set_active_reaction(null);
162+
set_active_effect(null);
163+
try {
164+
if (get_setters(node).includes(prop)) {
165+
node[prop] = value;
166+
} else {
167+
set_attribute(node, prop, value);
168+
}
169+
} finally {
170+
set_active_reaction(previous_reaction);
171+
set_active_effect(previous_effect);
152172
}
153173
}
154174

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { test } from '../../assert';
2+
const tick = () => Promise.resolve();
3+
4+
export default test({
5+
async test({ assert, target }) {
6+
target.innerHTML = '<my-app/>';
7+
await tick();
8+
await tick();
9+
10+
/** @type {any} */
11+
const el = target.querySelector('my-app');
12+
const button = el.shadowRoot.querySelector('button');
13+
const p = el.shadowRoot.querySelector('my-tracking').shadowRoot.querySelector('p');
14+
15+
assert.equal(button.innerHTML, '0');
16+
assert.equal(p.innerHTML, 'false');
17+
18+
button.click();
19+
await tick();
20+
21+
assert.equal(button.innerHTML, '1');
22+
assert.equal(p.innerHTML, 'false');
23+
}
24+
});
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<svelte:options customElement="my-app" />
2+
3+
<script module>
4+
class Tracking extends HTMLElement {
5+
static observedAttributes = ["count"];
6+
tracking = false;
7+
8+
set count(_) {
9+
this.tracking = $effect.tracking();
10+
this.render();
11+
}
12+
13+
constructor() {
14+
super();
15+
this.attachShadow({ mode: 'open' });
16+
}
17+
18+
render() {
19+
this.shadowRoot.innerHTML = `<p>${this.tracking}</p>`;
20+
}
21+
}
22+
23+
customElements.define("my-tracking", Tracking);
24+
</script>
25+
26+
<script>
27+
let count = $state(0);
28+
</script>
29+
30+
<button onclick={() => (count += 1)}>{count}</button>
31+
<my-tracking {count}></my-tracking>

0 commit comments

Comments
 (0)