Skip to content

Commit 505b6cd

Browse files
committed
chore: perf tweaks for actions/styles/classes
- check if we really need to add/remove the class (calling `includes` first is cheaper than always setting/removing it) - check if we really need to update a style (calling `getPropertyValue/setProperty` is expensive) - check if we should call the action's update function (this is not only a perf tweak but also a correctness fix) closes #12652
1 parent d20e064 commit 505b6cd

File tree

4 files changed

+50
-12
lines changed

4 files changed

+50
-12
lines changed

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

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,15 @@ function get_attribute_name(element, attribute, context) {
8383
* @param {Identifier} element_id
8484
* @param {ComponentContext} context
8585
* @param {boolean} is_attributes_reactive
86+
* @param {boolean} force_check Should be `true` if we can't rely on our cached value, because for example there's also a `style` attribute
8687
*/
87-
function serialize_style_directives(style_directives, element_id, context, is_attributes_reactive) {
88+
function serialize_style_directives(
89+
style_directives,
90+
element_id,
91+
context,
92+
is_attributes_reactive,
93+
force_check
94+
) {
8895
const state = context.state;
8996

9097
for (const directive of style_directives) {
@@ -99,7 +106,8 @@ function serialize_style_directives(style_directives, element_id, context, is_at
99106
element_id,
100107
b.literal(directive.name),
101108
value,
102-
/** @type {Expression} */ (directive.modifiers.includes('important') ? b.true : undefined)
109+
/** @type {Expression} */ (directive.modifiers.includes('important') ? b.true : undefined),
110+
force_check ? b.true : undefined
103111
)
104112
);
105113

@@ -2029,6 +2037,7 @@ export const template_visitors = {
20292037
let img_might_be_lazy = false;
20302038
let might_need_event_replaying = false;
20312039
let has_direction_attribute = false;
2040+
let has_style_attribute = false;
20322041

20332042
if (is_custom_element) {
20342043
// cloneNode is faster, but it does not instantiate the underlying class of the
@@ -2047,6 +2056,9 @@ export const template_visitors = {
20472056
if (attribute.name === 'dir') {
20482057
has_direction_attribute = true;
20492058
}
2059+
if (attribute.name === 'style') {
2060+
has_style_attribute = true;
2061+
}
20502062
if (
20512063
(attribute.name === 'value' || attribute.name === 'checked') &&
20522064
!is_text_attribute(attribute)
@@ -2196,7 +2208,13 @@ export const template_visitors = {
21962208

21972209
// class/style directives must be applied last since they could override class/style attributes
21982210
serialize_class_directives(class_directives, node_id, context, is_attributes_reactive);
2199-
serialize_style_directives(style_directives, node_id, context, is_attributes_reactive);
2211+
serialize_style_directives(
2212+
style_directives,
2213+
node_id,
2214+
context,
2215+
is_attributes_reactive,
2216+
has_style_attribute || node.metadata.has_spread
2217+
);
22002218

22012219
if (might_need_event_replaying) {
22022220
context.state.after_update.push(b.stmt(b.call('$.replay_events', node_id)));
@@ -2357,7 +2375,13 @@ export const template_visitors = {
23572375

23582376
// class/style directives must be applied last since they could override class/style attributes
23592377
serialize_class_directives(class_directives, element_id, inner_context, is_attributes_reactive);
2360-
serialize_style_directives(style_directives, element_id, inner_context, is_attributes_reactive);
2378+
serialize_style_directives(
2379+
style_directives,
2380+
element_id,
2381+
inner_context,
2382+
is_attributes_reactive,
2383+
true
2384+
);
23612385

23622386
const get_tag = b.thunk(/** @type {Expression} */ (context.visit(node.tag)));
23632387

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/** @import { ActionPayload } from '#client' */
22
import { effect, render_effect } from '../../reactivity/effects.js';
3+
import { safe_not_equal } from '../../reactivity/equality.js';
34
import { deep_read_state, untrack } from '../../runtime.js';
45

56
/**
@@ -15,6 +16,8 @@ export function action(dom, action, get_value) {
1516

1617
if (get_value && payload?.update) {
1718
var inited = false;
19+
/** @type {P} */
20+
var prev = /** @type {any} */ ({}); // initialize with something so it's never equal on first run
1821

1922
render_effect(() => {
2023
var value = get_value();
@@ -24,7 +27,8 @@ export function action(dom, action, get_value) {
2427
// together with actions and mutation, it wouldn't notice the change without a deep read.
2528
deep_read_state(value);
2629

27-
if (inited) {
30+
if (inited && safe_not_equal(prev, value)) {
31+
prev = value;
2832
/** @type {Function} */ (payload.update)(value);
2933
}
3034
});

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,10 @@ function to_class(value) {
107107
*/
108108
export function toggle_class(dom, class_name, value) {
109109
if (value) {
110+
if (dom.classList.contains(class_name)) return;
110111
dom.classList.add(class_name);
111112
} else {
113+
if (!dom.classList.contains(class_name)) return;
112114
dom.classList.remove(class_name);
113115
}
114116
}

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,23 @@
33
* @param {string} key
44
* @param {string} value
55
* @param {boolean} [important]
6+
* @param {boolean} [force_check] Should be `true` if we can't rely on our cached value, because for example there's also a `style` attribute
67
*/
7-
export function set_style(dom, key, value, important) {
8-
const style = dom.style;
9-
const prev_value = style.getPropertyValue(key);
8+
export function set_style(dom, key, value, important, force_check) {
9+
// @ts-expect-error
10+
var attributes = (dom.__attributes ??= {});
11+
var style = dom.style;
12+
var style_key = 'style-' + key;
13+
14+
if (attributes[style_key] === value && (!force_check || style.getPropertyValue(key) === value)) {
15+
return;
16+
}
17+
18+
attributes[style_key] = value;
19+
1020
if (value == null) {
11-
if (prev_value !== '') {
12-
style.removeProperty(key);
13-
}
14-
} else if (prev_value !== value) {
21+
style.removeProperty(key);
22+
} else {
1523
style.setProperty(key, value, important ? 'important' : '');
1624
}
1725
}

0 commit comments

Comments
 (0)