RFC: Use setProperty API for Styles

[ryansolid]

Summary

I decided to write an RFC because it has come up a few times and I want some community feedback even if it is a rather small change. Essentially I want to change Solid's style API from setting style properties directly to using setProperty.

Ie

// setting the property
el.style.backgroundColor = "red";

// setProperty
el.style.setProperty("background-color", "red");

This approach is already used by libraries like Inferno, Svelte, (and supported by Preact/Vue).

Please leave any feedback below. Or simply just leave a thumbs up etc...

Motivation

There are 3 main reasons this is attractive.

1. It is more performance. style.setProperty is the most performant approach. The performance gain might only be like 10-15% percent but it's something when considering JS animations.
2. Support for modern features like inline css variables. If more new features come this path supports them better.
3. Better alignment with SSR and string binding. Solid supports setting style strings directly. Moving between those and setProperty objects uses the same naming.

Proposal

Solid style binding uses css property style bindings. This means instead of camel case you use string hyphenated values:

<div style={{ color: "green", "background-color": state.color }} />

CSS variables:

<div style={{ "--my-custom-color": state.themeColor }} />

Drawbacks

You can't use camelCase styles. It might not be obvious coming from a React background. Also it is important to put units on. Like "px" etc.

Adoption

It is a breaking change across all libraries. And will be released as a new minor version. For the most part this will be a simple code change to make in existing Solid code.

[amoutonbrady]

That sounds like a reasonable and fair change. For the three reasons you've mentioned. Essentially what I'm getting is that you are trading a little bit of DX (since you have to hyphenate and specify the unit) for more performances & more features.

Now, being a big Tailwind fan, I quite rarely use inline styles, and therefore I might not be the best person to talk on it, but since I use Solid quite often, I feel like I should still give my perspective in it.

In my opinion, inline styles should be avoided as much as possible. They bite you in the back more often than not and as Andrea Giammarchi (creator of LighterHTML, HyperHTML, etc.) mentioned on twitter, they should mostly be used for custom properties which is what you are offering with this RFC.

[high1]

I think that this is the way to go. Improved performance, being used in other libraries signifies a good design choice - and deviating from the React-like API here is also a plus for me, since it keeps the original property name intact.

[ricochet1k]

I'm fine with this change.

If someone really wants to use camel-case properties, would it be possible to give them a hook to change the way it works? Would that break performance?

[ryansolid]

It wouldn't break performance. I know I harp on about it, but we are talking about milliseconds. Like single digit. Even under reasonable animation scenarios.

The interesting question is how to facilitate it. Refs are really the only syntax for custom directives. That approach might be a bit verbose but take almost no performance hit over something native. A function could convert at runtime but that is a bit of a cost. I thought about auto converting at compile time but I can only do that easily with styles that are inline objects I can statically analyze. The only other option really is to increase the runtime. It might be doable although opting in would need to be consistent on compile and runtime sides so it's a bit trickier for people to setup.

I was curious if there were any cases of incompatibility. It seems every time I seem to have a good idea TypeScript rears its head to remind me why I'm mistaken.

[ricochet1k]

What about a compile-time setting on babel-plugin-solid? Give it the name of a package and a function that will be auto-imported when used. Then any style={} that isn't statically analyzable would be wrapped in that function. And I guess there should be a similar way to patch the HyperScript version too for runtime.

It might actually be nice to use something like that for custom directives too.

[ryansolid]

Yeah that's what I mean you'd need to do both. I think I would want to thing of how to generalize this. I'm actually actively working through this right. I am thinking of how to add plugins to the compiler. But I don't think most things will warrant it. I'm thinking this more in ways to support things like Svelte Motion.

I actually had a really good syntax for custom directives before but TypeScript was not very happy having arbitrary naming (and doesn't support namespaces). I don't actually think we need the compiler to do a lot of things people would want to do. This is an exception because I went out of my way to optimize here. But I wasn't before now and it was perfectly fine. forwardRef is the way you can do this now but the syntax isn't as clean.

// somewhere
const style = (styleGetter) => {
  return el => {
    createEffect(() => Object.assign(el.style, styleGetter()))
  } 
}

<div forwardRef={style(() => { backgroundColor: state.color })} />

I used to have a syntax that automatically detected dynamic expressions and expect a function that takes (el, expr) => void:

// somewhere
const style = (el, styleGetter) => {
  createEffect(() => Object.assign(el.style, styleGetter()))
}

<div $style={{ backgroundColor: state.color }} />

If it can work with TypeScript maybe worth considering. There are just other ways to do this. Could just wrap with a Component like Styled Components etc. I wasn't particularly into creating different ways to do the same thing.

[high1]

Maybe release a version with the new API, and later, if it's doable in a reasonable way, provide a camelCase option. There would be two ways to do the same thing, right? And both would work?

style = {{ backgroundColor: 'red', 'font-size': '4em }}

It would be confusing. I'm not sure if using non camelCase option here would deviate a lot from the existing API...

[ricochet1k]

Typescript declaration merging should be able to make that case typecheck. Just throw something like this next to the style function:

namespace JSX {
    interface DOMAttributes<T> {
        $style: StyleObject;
    }
}

[ryansolid]

If that packages up right I suppose that could be a pattern. I'm basically picturing this in user land. Like you could write the directive and import it. Maybe I should consider this pattern alongside ref and forwardRef. I feel like maybe something better can be done there. I pretty sure with the work I'm doing there will be a natural time to reconsider this soon.

[ryansolid]

I think in the case of custom directives I'm going to wait til NameSpaces land. This blows things wide open and I don't want to have to change syntax in the future. https://github.com/microsoft/TypeScript/pull/37421

[ryansolid]

This Style RFC has been merged in v0.18.0.