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

Don't use Custom Properties to implement dynamic styles #800

Open
necolas opened this issue Dec 5, 2024 · 3 comments
Open

Don't use Custom Properties to implement dynamic styles #800

necolas opened this issue Dec 5, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@necolas
Copy link
Contributor

necolas commented Dec 5, 2024

Describe the feature request

When using inline styles directly, you don't incur the runtime cost of using custom properties to back the dynamic values of properties. For most dynamic use cases, we can simply render inline styles without using custom properties or creating additional generated CSS. There are runtime and bundle size wins to be had there.

@necolas necolas added the enhancement New feature or request label Dec 5, 2024
@nmn
Copy link
Contributor

nmn commented Dec 5, 2024

Just adding some details for this task.

When inline styles are possible

  • We can use inline styles directly for longhand CSS properties
  • We cannot safely use inline styles for CSS shorthands because we need shorthand styles to have a lower specificity than longhand styles.
  • This is only possible if we assume there is no usage of the legacy patterns of :hover and @media queries. (i.e, using them outside the CSS property)

The trade-offs

  • We are currently discussing a feature to automatically detect CSS variables that are never re-assigned and inline their usage (treat them like a constant).
    • This would no longer be possible if we use inline styles for dynamic styles as we would not be able to have global knowledge by reading the generated CSS file.
    • However, we should be able to make this work an explicit stylex.defineConsts API
  • Using global CSS selectors to override StyleX styles may break due to specificity change.
    • we can warn them, discourage this pattern and ask them to use !important as an escape hatch

Performance

  • A recent investigation suggest that inline styles are just as fast or faster than using CSS.
    • There is a cost of serializing objects to strings within React, but we need inline styles either way, so there is no regression on this
    • The styles are not cache-able, but this is fine because the values are dynamic anyway.
  • There is currently no evidence suggesting any style engine performance regression when mixing atomic CSS and inline styles.

@necolas
Copy link
Contributor Author

necolas commented Dec 6, 2024

We are currently working on implementing a feature to automatically detect CSS variables that are never re-assigned and inline their usage...

Is this being worked (who is "we") on or just promised? What does "inline their usage" mean in this context - is it the same as #737 or something else?

Using global CSS selectors to override StyleX styles may break...

Global CSS selectors? I don't follow

A recent investigation suggest that inline styles are just as fast or faster than using CSS.

What's the relevance here?

@nmn
Copy link
Contributor

nmn commented Dec 9, 2024

is it the same as #737 or something else?

No, this is to detect CSS variables defined with stylex.defineVars that are never re-assigned and inlining them in the generated CSS. We're still only discussing this. (I updated my comment above). This is one of the ideas Nitish and I talked about after finding out that using too many CSS variables can cause too much RAM usage.

We obviously want to work stylex.defineConsts, but wondering if this kind of optimization might be a good thing to add that doesn't depend on you manually deciding if a value needs to be a variable or a constant ahead of time.

Global CSS selectors? I don't follow

There are some rare cases in our codebase where we use CSS files to override the "base" styles applied by StyleX.

What's the relevance here?

A lot of devs assume inline styles are slower. This is just evidence that shows that inline styles are not slower than atomic CSS.

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

No branches or pull requests

2 participants