Skip to content

Little improvement to set_style() #13791

@adiguba

Description

@adiguba

Describe the problem

Hello,

I think that the current implementation of the style: directive can be slightly improved with small changes in client mode.

In the current implementation :

  • Values are cached into dom.__attributes using the key 'style-'+key. This is nothing, but a temporary string is created on each update.
  • If there a reactive style attribute or spreading, set_style() receive an force_check in order to verify the property via style.getPropertyValue().
    This is required to force update because the directive takes precedence over the style attribute, but this imply a dom access on each update.

I think this can be fixed by caching the data in a dedicated object.

Describe the proposed solution

This brings two main changes :

  • Values should be cached into his own object, for ex instead of dom.__attributes['style-'+key] we can use dom.__styles[key]
  • The force_check trick could be deleted and replaced by a small code in set_attribute()/set_attributes().

So the set_style() code will be changed like that :

/**
 * @param {HTMLElement} dom
 * @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, force_check) {
+export function set_style(dom, key, value, important) {
	// @ts-expect-error
+	var styles = (dom.__styles ??= {});
-	var attributes = (dom.__attributes ??= {});
-	var style = dom.style;
-	var style_key = 'style-' + key;

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

-	attributes[style_key] = value;
+	styles[key] = value;

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

And instead of force_check, it would be enough to slightly modify set_attribute() and set_attributes() to do something like this when style is modified :

// if dom has a __styles attributes, 
// we reset it to force the style directives to reapply
if ('__styles' in dom) {
    dom.__styles = {};
}

If this solution seems valid, I think I could do the PR.

Importance

nice to have

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions