Skip to content

chore: perf tweaks for actions/styles/classes #12654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/sharp-fishes-serve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

feat: perf tweaks for actions/styles/classes
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,15 @@ function get_attribute_name(element, attribute, context) {
* @param {Identifier} element_id
* @param {ComponentContext} context
* @param {boolean} is_attributes_reactive
* @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
*/
function serialize_style_directives(style_directives, element_id, context, is_attributes_reactive) {
function serialize_style_directives(
style_directives,
element_id,
context,
is_attributes_reactive,
force_check
) {
const state = context.state;

for (const directive of style_directives) {
Expand All @@ -99,7 +106,8 @@ function serialize_style_directives(style_directives, element_id, context, is_at
element_id,
b.literal(directive.name),
value,
/** @type {Expression} */ (directive.modifiers.includes('important') ? b.true : undefined)
/** @type {Expression} */ (directive.modifiers.includes('important') ? b.true : undefined),
force_check ? b.true : undefined
)
);

Expand Down Expand Up @@ -2029,6 +2037,7 @@ export const template_visitors = {
let img_might_be_lazy = false;
let might_need_event_replaying = false;
let has_direction_attribute = false;
let has_style_attribute = false;

if (is_custom_element) {
// cloneNode is faster, but it does not instantiate the underlying class of the
Expand All @@ -2047,6 +2056,9 @@ export const template_visitors = {
if (attribute.name === 'dir') {
has_direction_attribute = true;
}
if (attribute.name === 'style') {
has_style_attribute = true;
}
if (
(attribute.name === 'value' || attribute.name === 'checked') &&
!is_text_attribute(attribute)
Expand Down Expand Up @@ -2196,7 +2208,13 @@ export const template_visitors = {

// class/style directives must be applied last since they could override class/style attributes
serialize_class_directives(class_directives, node_id, context, is_attributes_reactive);
serialize_style_directives(style_directives, node_id, context, is_attributes_reactive);
serialize_style_directives(
style_directives,
node_id,
context,
is_attributes_reactive,
has_style_attribute || node.metadata.has_spread
);

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

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

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

Expand Down
6 changes: 5 additions & 1 deletion packages/svelte/src/internal/client/dom/elements/actions.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/** @import { ActionPayload } from '#client' */
import { effect, render_effect } from '../../reactivity/effects.js';
import { safe_not_equal } from '../../reactivity/equality.js';
import { deep_read_state, untrack } from '../../runtime.js';

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

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

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

if (inited) {
if (inited && safe_not_equal(prev, value)) {
prev = value;
/** @type {Function} */ (payload.update)(value);
}
});
Expand Down
2 changes: 2 additions & 0 deletions packages/svelte/src/internal/client/dom/elements/class.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,10 @@ function to_class(value) {
*/
export function toggle_class(dom, class_name, value) {
if (value) {
if (dom.classList.contains(class_name)) return;
dom.classList.add(class_name);
} else {
if (!dom.classList.contains(class_name)) return;
dom.classList.remove(class_name);
}
}
22 changes: 15 additions & 7 deletions packages/svelte/src/internal/client/dom/elements/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,23 @@
* @param {string} key
* @param {string} value
* @param {boolean} [important]
* @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
*/
export function set_style(dom, key, value, important) {
const style = dom.style;
const prev_value = style.getPropertyValue(key);
export function set_style(dom, key, value, important, force_check) {
// @ts-expect-error
var attributes = (dom.__attributes ??= {});
var style = dom.style;
var style_key = 'style-' + key;

if (attributes[style_key] === value && (!force_check || style.getPropertyValue(key) === value)) {
return;
}

attributes[style_key] = value;

if (value == null) {
if (prev_value !== '') {
style.removeProperty(key);
}
} else if (prev_value !== value) {
style.removeProperty(key);
} else {
style.setProperty(key, value, important ? 'important' : '');
}
}
Loading