eui icon indicating copy to clipboard operation
eui copied to clipboard

chore(eui): flag dynamic styles in emotion

Open weronikaolejniczak opened this issue 5 months ago • 2 comments

[!NOTE] This is strictly a PoC. It's not meant to be the final solution. It has to be thoroughly tested.

Summary

This PR monkey-patches the cache.insert function and keeps a map of unique selectors to determine whether styles are dynamic or not.

https://github.com/user-attachments/assets/48740920-57dd-4507-97aa-1f0a1da469ed

Assumptions

  • If for the same base class (i.e., the part of the selector after the hash) multiple different hashes are generated, it is likely a dynamic style (e.g., a dynamic value passed to the css prop or a style object).
  • If for the same base class only one hash is ever generated, it is likely a static style (i.e., the style is determined solely by static props and does not change at runtime).
  • If the base class itself changes, it is likely due to a change in static props (such as color, size, etc.), which is expected and not considered a dynamic style.
  • The patch cannot directly determine which prop (e.g., css, style, color) caused the style change; it only infers dynamic styles by observing multiple hashes for the same base class.
  • The presence of multiple hashes for the same base class is a strong indicator of runtime-generated (dynamic) styles, which are best handled with the style prop for performance and maintainability.

Advantages

  • early detection - flags dynamic styles at development time, helping teams catch performance and maintainability issues before production (relies on developers actually looking at the browser console),
  • developer guidance - provides actionable warnings, encouraging best practices (e.g., using the style prop for dynamic values),
  • non-Intrusive - only runs in development (and not in production or test), so there’s no runtime or bundle size impact for end users,
  • reusable & modular - the patch can be applied to any Emotion cache, making it easy to use across projects or packages,
  • no app code changes needed - works transparently by monkey-patching Emotion’s cache, so no changes are required in component or app code.

Disadvantages

  • heuristic-based - relies on pattern matching and inference, which may produce false positives or miss some edge cases,
  • limited context - cannot tell exactly which prop css vs. color caused the style change - only that a dynamic style was generated - but could point to underlying issues in EUI as well,
  • console noise - may produce many warnings in large or complex apps, which could annoy developers if not tuned,
  • emotion-specific - tightly coupled to Emotion’s class naming and cache internals; may break if Emotion’s implementation changes,

Feasibility

The approach is feasible and practical for most Emotion-based React projects. It leverages public APIs and common patterns. Can be added to a provider or setup file with minimal effort. Since it’s dev-only, there’s no impact on production builds or performance.

Limitations

  • no prop source tracking - cannot distinguish between dynamic css prop usage and other sources of dynamic styles (e.g., dynamic theme, context, or computed props),
  • pattern dependency - assumes Emotion’s class naming convention (.css-xxxxxx-baseClass) and if this changes, the detection may fail,
  • not 100% accurate - some legitimate use cases (e.g., theming, responsive styles) may trigger warnings even if they’re not “bad” dynamic styles,
  • no automatic fixes - only warns; it does not prevent or fix dynamic style usage.

Why are we making this change?

As recommended by Emotion, you should use style for dynamic styles and css only for static styles.

By dynamic styles, we mean styles that change frequently or are unique to a single element.

Impact to users

This is not a breaking change. It doesn't affect EUI visually.

It's strictly in development to help advocate the usage of style prop for dynamic styles.

QA

Specific checklist

  • [ ] Unit tests added are passing in CI
  • [ ] There is no regression noticeable in Storybook or documentation website after monkey-patching cache.insert
  • [ ] There is no warning about dynamic styles for static styles
  • [ ] There is a warning when dynamic styles are used (e.g., generated with a function, changed on every render, changed on interaction, based on props or state, passing an object to css prop that changes reference on every render)
  • [ ] There are no warnings in a production build
  • [ ] There are warnings locally

weronikaolejniczak avatar Jun 13 '25 11:06 weronikaolejniczak

:green_heart: Build Succeeded

History

  • :green_heart: Build #520 succeeded 53c42fbc292a0597a8289aa0eb3ab40d936de15a

cc @weronikaolejniczak

elasticmachine avatar Jun 13 '25 11:06 elasticmachine

:green_heart: Build Succeeded

cc @weronikaolejniczak

elasticmachine avatar Jun 13 '25 12:06 elasticmachine

đź‘‹ Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

github-actions[bot] avatar Sep 25 '25 16:09 github-actions[bot]

Closing. We don't want this because it's not that useful considering the small amount of cases.

weronikaolejniczak avatar Sep 26 '25 07:09 weronikaolejniczak