Skip to content
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

Automatically separate static vs. dynamic values into className/style #96

Open
stephenh opened this issue Jan 18, 2023 · 0 comments
Open

Comments

@stephenh
Copy link
Contributor

stephenh commented Jan 18, 2023

Per https://emotion.sh/docs/best-practices it is a good idea to use classNames for things like mt5 which will have super-stable class names, but a bad idea to use classNames for things like backgroundUrl: product-id-123.png or mt(cursorX) because the latter two will spam new class names for every single little new value.

In straight Emotion, developers have to know which one of these to use, and consciously pick "use <div className=... / <div css=...for this static style (likely 95%+ of styles?) / use<div style=...` for that dynamic style (likely very few styles)".

Note that at Homebound, we don't do this, and always (afaiu/surely?) use css={...} / classNamees, and to my knowledge basically never use the style={...} attribute for frequently-updated dynamic values.

In theory, Truss is actually well-suited to do this optimization automatically by a heuristic like:

  • Any "static" / param-less method like Css.mt1.$ or Css.fdr$ goes to a regular emotion-generated className attribute
  • Any "dynamic" / param-based method like Css.mt(10).$ or Css.backgroundUrl(fooBar).$ goes to the style attribute
  • Given something like Css.mt1.mb(cursorX).$ we could put mt1 in className= and mb: cursorX into style= basically automatically / for free.

Granted, to do this we would have to:

  1. Update Css.ts to internally track "static styles" vs. "dynamic styles" separately
  2. Write our own JSX handler that is Css.ts-aware and outputs "static styles" to className (via emotion-generated class names / <style> injections) and "dynamic styles" to the element's style= attribute.

We have a prototype of 2 here and neither of these would be that hard to do; like 1.5 hack days maybe.

Disclaimers/wrinkles:

  • Anything with psuedo-selectors, like Css.ifSm.mt(10).$ would have to go through className so that it can be conditionally applied by the media query

  • In theory "some things are set via className / some things are set via style" can lead to precedence / cascading issues, which I think in theory has historically been a win for CSS-in-JS libraries just "always using class names" (to remove the precedence complexity/foot gun for developers), but in practice we purposefully don't use cascading anyway, so that probably? doesn't matter to us.

  • It's pure speculation whether this optimization really matters to us; being smart about className vs. style is pointed out in the Emotion best practice guide, but to my knowledge we don't have any perf issues with the "all className" based approach today.


Or we could just continue using css= for basically everything, and when we realize we have a dynamic value case that is actually causing a performance issue (spamming style tags), just set style by hand, i.e. <div style={Css.mt(23).$} /> would probably work, given Truss's current "I just make a hash of key/value pairs" approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant