Skip to content

Commit f0d9919

Browse files
dummdidummdummdidumm
committed
fixes #10013 
1 parent fa3e4d0 commit f0d9919

File tree

6 files changed

+161
-33
lines changed

6 files changed

+161
-33
lines changed

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -289,15 +289,15 @@ function setup_select_synchronization(value_binding, context) {
289289
* value = $.spread_attributes(element, value, [...])
290290
* });
291291
* ```
292-
* Returns the id of the spread_attribute variable if spread is deemed reactive, `null` otherwise.
292+
* Returns the id of the spread_attribute variable if spread isn't isolated, `null` otherwise.
293293
* @param {Array<import('#compiler').Attribute | import('#compiler').SpreadAttribute>} attributes
294294
* @param {import('../types.js').ComponentContext} context
295295
* @param {import('#compiler').RegularElement} element
296296
* @param {import('estree').Identifier} element_id
297297
* @returns {string | null}
298298
*/
299299
function serialize_element_spread_attributes(attributes, context, element, element_id) {
300-
let is_reactive = false;
300+
let needs_isolation = false;
301301

302302
/** @type {import('estree').Expression[]} */
303303
const values = [];
@@ -312,19 +312,32 @@ function serialize_element_spread_attributes(attributes, context, element, eleme
312312
values.push(/** @type {import('estree').Expression} */ (context.visit(attribute)));
313313
}
314314

315-
is_reactive ||=
316-
attribute.metadata.dynamic ||
317-
// objects could contain reactive getters -> play it safe and always assume spread attributes are reactive
318-
attribute.type === 'SpreadAttribute';
315+
needs_isolation ||=
316+
attribute.type === 'SpreadAttribute' && attribute.metadata.contains_call_expression;
319317
}
320318

321319
const lowercase_attributes =
322320
element.metadata.svg || is_custom_element_node(element) ? b.false : b.true;
323321

324-
if (is_reactive) {
322+
const isolated = b.stmt(
323+
b.call(
324+
'$.spread_attributes_effect',
325+
element_id,
326+
b.thunk(b.array(values)),
327+
lowercase_attributes,
328+
b.literal(context.state.analysis.stylesheet.id)
329+
)
330+
);
331+
332+
// objects could contain reactive getters -> play it safe and always assume spread attributes are reactive
333+
if (needs_isolation) {
334+
context.state.update_effects.push(isolated);
335+
return null;
336+
} else {
325337
const id = context.state.scope.generate('spread_attributes');
326-
context.state.init.push(b.let(id, undefined));
338+
context.state.init.push(b.let(id));
327339
context.state.update.push({
340+
singular: isolated,
328341
grouped: b.stmt(
329342
b.assignment(
330343
'=',
@@ -341,20 +354,6 @@ function serialize_element_spread_attributes(attributes, context, element, eleme
341354
)
342355
});
343356
return id;
344-
} else {
345-
context.state.init.push(
346-
b.stmt(
347-
b.call(
348-
'$.spread_attributes',
349-
element_id,
350-
b.literal(null),
351-
b.array(values),
352-
lowercase_attributes,
353-
b.literal(context.state.analysis.stylesheet.id)
354-
)
355-
)
356-
);
357-
return null;
358357
}
359358
}
360359

@@ -366,7 +365,7 @@ function serialize_element_spread_attributes(attributes, context, element, eleme
366365
* @param {import('estree').Identifier} element_id
367366
* @returns {boolean}
368367
*/
369-
function serialize_dynamic_element_spread_attributes(attributes, context, element_id) {
368+
function serialize_dynamic_element_attributes(attributes, context, element_id) {
370369
if (attributes.length === 0) {
371370
if (context.state.analysis.stylesheet.id) {
372371
context.state.init.push(
@@ -376,6 +375,7 @@ function serialize_dynamic_element_spread_attributes(attributes, context, elemen
376375
return false;
377376
}
378377

378+
let needs_isolation = false;
379379
let is_reactive = false;
380380

381381
/** @type {import('estree').Expression[]} */
@@ -393,12 +393,27 @@ function serialize_dynamic_element_spread_attributes(attributes, context, elemen
393393
attribute.metadata.dynamic ||
394394
// objects could contain reactive getters -> play it safe and always assume spread attributes are reactive
395395
attribute.type === 'SpreadAttribute';
396+
needs_isolation ||=
397+
attribute.type === 'SpreadAttribute' && attribute.metadata.contains_call_expression;
396398
}
397399

398-
if (is_reactive) {
400+
const isolated = b.stmt(
401+
b.call(
402+
'$.spread_dynamic_element_attributes_effect',
403+
element_id,
404+
b.thunk(b.array(values)),
405+
b.literal(context.state.analysis.stylesheet.id)
406+
)
407+
);
408+
409+
if (needs_isolation) {
410+
context.state.update_effects.push(isolated);
411+
return false;
412+
} else if (is_reactive) {
399413
const id = context.state.scope.generate('spread_attributes');
400414
context.state.init.push(b.let(id));
401415
context.state.update.push({
416+
singular: isolated,
402417
grouped: b.stmt(
403418
b.assignment(
404419
'=',
@@ -2105,7 +2120,7 @@ export const template_visitors = {
21052120
// Always use spread because we don't know whether the element is a custom element or not,
21062121
// therefore we need to do the "how to set an attribute" logic at runtime.
21072122
const is_attributes_reactive =
2108-
serialize_dynamic_element_spread_attributes(attributes, inner_context, element_id) !== null;
2123+
serialize_dynamic_element_attributes(attributes, inner_context, element_id) !== null;
21092124

21102125
// class/style directives must be applied last since they could override class/style attributes
21112126
serialize_class_directives(class_directives, element_id, inner_context, is_attributes_reactive);

‎packages/svelte/src/internal/client/render.js

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2428,10 +2428,26 @@ function get_setters(element) {
24282428
return setters;
24292429
}
24302430

2431+
/**
2432+
* Like `spread_attributes` but self-contained
2433+
* @param {Element & ElementCSSInlineStyle} dom
2434+
* @param {() => Record<string, unknown>[]} attrs
2435+
* @param {boolean} lowercase_attributes
2436+
* @param {string} css_hash
2437+
*/
2438+
export function spread_attributes_effect(dom, attrs, lowercase_attributes, css_hash) {
2439+
/** @type {Record<string, any> | undefined} */
2440+
let current = undefined;
2441+
2442+
render_effect(() => {
2443+
current = spread_attributes(dom, current, attrs(), lowercase_attributes, css_hash);
2444+
});
2445+
}
2446+
24312447
/**
24322448
* Spreads attributes onto a DOM element, taking into account the currently set attributes
24332449
* @param {Element & ElementCSSInlineStyle} dom
2434-
* @param {Record<string, unknown> | null} prev
2450+
* @param {Record<string, unknown> | undefined} prev
24352451
* @param {Record<string, unknown>[]} attrs
24362452
* @param {boolean} lowercase_attributes
24372453
* @param {string} css_hash
@@ -2528,18 +2544,30 @@ export function spread_attributes(dom, prev, attrs, lowercase_attributes, css_ha
25282544

25292545
/**
25302546
* @param {Element} node
2531-
* @param {Record<string, unknown> | null} prev
2547+
* @param {() => Record<string, unknown>[]} attrs
2548+
* @param {string} css_hash
2549+
*/
2550+
export function spread_dynamic_element_attributes_effect(node, attrs, css_hash) {
2551+
/** @type {Record<string, any> | undefined} */
2552+
let current = undefined;
2553+
2554+
render_effect(() => {
2555+
current = spread_dynamic_element_attributes(node, current, attrs(), css_hash);
2556+
});
2557+
}
2558+
2559+
/**
2560+
* @param {Element} node
2561+
* @param {Record<string, unknown> | undefined} prev
25322562
* @param {Record<string, unknown>[]} attrs
25332563
* @param {string} css_hash
25342564
*/
25352565
export function spread_dynamic_element_attributes(node, prev, attrs, css_hash) {
25362566
if (node.tagName.includes('-')) {
25372567
const next = object_assign({}, ...attrs);
2538-
if (prev !== null) {
2539-
for (const key in prev) {
2540-
if (!(key in next)) {
2541-
next[key] = null;
2542-
}
2568+
for (const key in prev) {
2569+
if (!(key in next)) {
2570+
next[key] = null;
25432571
}
25442572
}
25452573
for (const key in next) {

‎packages/svelte/tests/runtime-runes/samples/attribute-spread-call-expression/_config.js

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
html: `
5+
<button class="red">red</button>
6+
<button class="red">red</button>
7+
<button class="red">red</button>
8+
<button class="red">red</button>
9+
`,
10+
11+
async test({ assert, target }) {
12+
const [b1, b2, b3, b4] = target.querySelectorAll('button');
13+
14+
b1?.click();
15+
await Promise.resolve();
16+
assert.htmlEqual(
17+
target.innerHTML,
18+
`
19+
<button class="blue">blue</button>
20+
<button class="red">red</button>
21+
<button class="red">red</button>
22+
<button class="red">red</button>
23+
`
24+
);
25+
26+
b2?.click();
27+
await Promise.resolve();
28+
assert.htmlEqual(
29+
target.innerHTML,
30+
`
31+
<button class="blue">blue</button>
32+
<button class="blue">blue</button>
33+
<button class="red">red</button>
34+
<button class="red">red</button>
35+
`
36+
);
37+
38+
b3?.click();
39+
await Promise.resolve();
40+
assert.htmlEqual(
41+
target.innerHTML,
42+
`
43+
<button class="blue">blue</button>
44+
<button class="blue">blue</button>
45+
<button class="blue">blue</button>
46+
<button class="red">red</button>
47+
`
48+
);
49+
50+
b4?.click();
51+
await Promise.resolve();
52+
assert.htmlEqual(
53+
target.innerHTML,
54+
`
55+
<button class="blue">blue</button>
56+
<button class="blue">blue</button>
57+
<button class="blue">blue</button>
58+
<button class="blue">blue</button>
59+
`
60+
);
61+
}
62+
});

‎packages/svelte/tests/runtime-runes/samples/attribute-spread-call-expression/main.svelte

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<script>
2+
let tag = $state('button');
3+
let values = $state({ a: 'red', b: 'red', c: 'red', d: 'red' });
4+
5+
let count = 0;
6+
const factory = (name) => {
7+
count++;
8+
// check that spread effects are isolated from each other
9+
if (count > 8) throw new Error('too many calls');
10+
11+
return {
12+
class: values[name],
13+
onclick: () => {
14+
values[name] = 'blue';
15+
}
16+
}
17+
}
18+
</script>
19+
20+
<button {...factory('a')}>{values.a}</button>
21+
<button {...factory('b')}>{values.b}</button>
22+
23+
<svelte:element this={tag} {...factory('c')}>{values.c}</svelte:element>
24+
<svelte:element this={tag} {...factory('d')}>{values.d}</svelte:element>

‎packages/svelte/tests/runtime-runes/samples/event-attribute-call-expressions/_config.js renamed to ‎packages/svelte/tests/runtime-runes/samples/attribute-spread-reactivitiy/_config.js

File renamed without changes.

‎packages/svelte/tests/runtime-runes/samples/event-attribute-call-expressions/main.svelte renamed to ‎packages/svelte/tests/runtime-runes/samples/attribute-spread-reactivitiy/main.svelte

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,3 @@
2727
<svelte:element this={tag} {...props}></svelte:element>
2828

2929
<button on:click={() => value = 'blue'}>toggle</button>
30-

0 commit comments

Comments
 (0)