fix: improve attribute directive reactivity detection (#9907)

pull/9899/head
Dominic Gannaway 1 year ago committed by GitHub
parent 4e61db7201
commit b0511a5966
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: improve attribute directive reactivity detection

@ -63,13 +63,14 @@ function get_attribute_name(element, attribute, context) {
* @param {boolean} is_attributes_reactive * @param {boolean} is_attributes_reactive
*/ */
function serialize_style_directives(style_directives, element_id, context, is_attributes_reactive) { function serialize_style_directives(style_directives, element_id, context, is_attributes_reactive) {
if (style_directives.length > 0) { const state = context.state;
const values = style_directives.map((directive) => {
for (const directive of style_directives) {
let value = let value =
directive.value === true directive.value === true
? serialize_get_binding({ name: directive.name, type: 'Identifier' }, context.state) ? serialize_get_binding({ name: directive.name, type: 'Identifier' }, context.state)
: serialize_attribute_value(directive.value, context)[1]; : serialize_attribute_value(directive.value, context)[1];
return b.stmt( const grouped = b.stmt(
b.call( b.call(
'$.style', '$.style',
element_id, element_id,
@ -80,15 +81,30 @@ function serialize_style_directives(style_directives, element_id, context, is_at
) )
) )
); );
}); const singular = b.stmt(
b.call(
'$.style_effect',
element_id,
b.literal(directive.name),
b.arrow([], value),
/** @type {import('estree').Expression} */ (
directive.modifiers.includes('important') ? b.true : undefined
)
)
);
if ( const contains_call_expression =
is_attributes_reactive || Array.isArray(directive.value) &&
style_directives.some((directive) => directive.metadata.dynamic) directive.value.some(
) { (v) => v.type === 'ExpressionTag' && v.metadata.contains_call_expression
context.state.update.push(...values.map((v) => ({ grouped: v }))); );
if (!is_attributes_reactive && contains_call_expression) {
state.update_effects.push(singular);
} else if (is_attributes_reactive || directive.metadata.dynamic || contains_call_expression) {
state.update.push({ grouped, singular });
} else { } else {
context.state.init.push(...values); state.init.push(grouped);
} }
} }
} }
@ -123,21 +139,21 @@ function parse_directive_name(name) {
* @param {boolean} is_attributes_reactive * @param {boolean} is_attributes_reactive
*/ */
function serialize_class_directives(class_directives, element_id, context, is_attributes_reactive) { function serialize_class_directives(class_directives, element_id, context, is_attributes_reactive) {
if (class_directives.length > 0) { const state = context.state;
const values = class_directives.map((directive) => { for (const directive of class_directives) {
const value = /** @type {import('estree').Expression} */ ( const value = /** @type {import('estree').Expression} */ (context.visit(directive.expression));
context.visit(directive.expression) const grouped = b.stmt(b.call('$.class_toggle', element_id, b.literal(directive.name), value));
const singular = b.stmt(
b.call('$.class_toggle_effect', element_id, b.literal(directive.name), b.arrow([], value))
); );
return b.stmt(b.call('$.class_toggle', element_id, b.literal(directive.name), value)); const contains_call_expression = directive.expression.type === 'CallExpression';
});
if ( if (!is_attributes_reactive && contains_call_expression) {
is_attributes_reactive || state.update_effects.push(singular);
class_directives.some((directive) => directive.metadata.dynamic) } else if (is_attributes_reactive || directive.metadata.dynamic || contains_call_expression) {
) { state.update.push({ grouped, singular });
context.state.update.push(...values.map((v) => ({ grouped: v })));
} else { } else {
context.state.init.push(...values); state.init.push(grouped);
} }
} }
} }
@ -295,7 +311,9 @@ function serialize_element_spread_attributes(attributes, context, element, eleme
values.push(/** @type {import('estree').Expression} */ (context.visit(attribute))); values.push(/** @type {import('estree').Expression} */ (context.visit(attribute)));
} }
is_reactive ||= attribute.metadata.dynamic; is_reactive ||=
attribute.metadata.dynamic ||
(attribute.type === 'SpreadAttribute' && attribute.metadata.contains_call_expression);
} }
const lowercase_attributes = const lowercase_attributes =

@ -444,6 +444,20 @@ export function class_toggle(dom, class_name, value) {
dom.classList.remove(class_name); dom.classList.remove(class_name);
} }
} }
/**
* @param {Element} dom
* @param {string} class_name
* @param {() => boolean} value
* @returns {void}
*/
export function class_toggle_effect(dom, class_name, value) {
render_effect(() => {
const string = value();
class_toggle(dom, class_name, string);
});
}
/** /**
* Selects the correct option(s) (depending on whether this is a multiple select) * Selects the correct option(s) (depending on whether this is a multiple select)
* @template V * @template V
@ -2359,13 +2373,31 @@ export function set_custom_element_data(node, prop, value) {
* @param {boolean} [important] * @param {boolean} [important]
*/ */
export function style(dom, key, value, important) { export function style(dom, key, value, important) {
const style = dom.style;
const prev_value = style.getPropertyValue(key);
if (value == null) { if (value == null) {
dom.style.removeProperty(key); if (prev_value !== '') {
} else { style.removeProperty(key);
dom.style.setProperty(key, value, important ? 'important' : ''); }
} else if (prev_value !== value) {
style.setProperty(key, value, important ? 'important' : '');
} }
} }
/**
* @param {HTMLElement} dom
* @param {string} key
* @param {() => string} value
* @param {boolean} [important]
* @returns {void}
*/
export function style_effect(dom, key, value, important) {
render_effect(() => {
const string = value();
style(dom, key, string, important);
});
}
/** /**
* List of attributes that should always be set through the attr method, * List of attributes that should always be set through the attr method,
* because updating them through the property setter doesn't work reliably. * because updating them through the property setter doesn't work reliably.

@ -0,0 +1,16 @@
import { test } from '../../test';
export default test({
html: `<div style="color: red;"></div><div class="red"></div><button>toggle</button`,
async test({ assert, target }) {
const [b1] = target.querySelectorAll('button');
b1?.click();
await Promise.resolve();
assert.htmlEqual(
target.innerHTML,
'<div class="blue" style="color: blue;"></div><div class="blue"></div><button>toggle</button>'
);
}
});

@ -0,0 +1,18 @@
<script>
let value = $state('red');
const getValue = () => {
return value;
}
const getClass = () => {
return value === 'blue';
}
const getSpread = () => {
return { class: value };
}
</script>
<div class:blue={getClass()} style:color={getValue()}></div>
<div {...getSpread()}></div>
<button on:click={() => value = 'blue'}>toggle</button>
Loading…
Cancel
Save