eui icon indicating copy to clipboard operation
eui copied to clipboard

Update EUI Theme Provider to use `css` from `@emotion/react`

Open legrego opened this issue 6 months ago • 7 comments

I am starting to add nonce attributes to Kibana's style tags, in an effort to eliminate unsafe-inine from our style-src Content Security Policy directive: https://github.com/elastic/kibana/issues/135469

Emotion can automatically apply the nonce when its cache is configured with one: https://github.com/elastic/kibana/commit/bbd4a115f2c6b88045dfca84e796e958391ab3ef#diff-ad06d78885555a2bbada8ff595bf5b937b1ea3f80c0a875910346f1027025a7b

This is working well for many of our code paths, but Emotion by design does not use its cache for the @emotion/css package: https://github.com/emotion-js/emotion/issues/2409.

I've found that switching Kibana from the @emotion/css package to the @emotion/react package works as a drop-in replacement, but the latter respects the nonce.

import { css } from `@emotion/css`

to

import { css } from `@emotion/react`

Once those findings were resolved, I came across this usage of @emotion/css within EUI. This usage is less straightforward than what I've found in Kibana, and I lack the confidence/expertise to make this change without guidance:

https://github.com/elastic/eui/blob/acc56997f45f9c2491111442e713da068f6c162d/packages/eui/src/services/theme/provider.tsx#L238-L241

What is the best way to remove @emotion/css here, in favor of a cache-respecting equivalent?

legrego avatar May 28 '25 14:05 legrego

Here just to triage.

Usages of css from @emotion/css:

  • packages/eui/src/components/code/code_block_line.styles.ts:16
  • packages/eui/src/components/code/utils.tsx:23
  • packages/eui/src/components/datagrid/data_grid_styles.stories.tsx:11
  • packages/eui/src/components/datagrid/controls/fullscreen_selector.styles.ts:9
  • packages/eui/src/components/date_picker/react_date_picker.styles.ts:10 (note about how it needs vanilla css to pass a className directly to react-datepicker)
  • packages/eui/src/components/overlay_mask/overlay_mask.styles.ts:9
  • packages/eui/src/components/selectable/selectable_list/selectable_list.styles.ts:12

Cache creation:

  • packages/eui/src/services/emotion/css.ts (a note about how it's beta and not intended for a public export, so no external usage we have to worry about)

Two internal usages of css:

  • packages/eui/src/services/theme/provider.tsx:25
  • packages/eui/src/components/date_picker/react_date_picker.styles.ts:10

Suggested replacement:

@emotion/react with createCache from @emotion/cache with a prepend value for CacheProvider. Looks like a drop-in replacement but will require proper testing.

weronikaolejniczak avatar May 28 '25 15:05 weronikaolejniczak

From a first look at the provider, I don't think it's a 1:1 replacement 🤔 The appliance of styles via className vs css props has some differences and we'll need to ensure that those differences won't break usages. The main concerns would be:

  • the functionality to apply global/local theme css variables
  • potential change in class names if those are used for anything else

mgadewoll avatar May 28 '25 15:05 mgadewoll

@mgadewoll I'm not sure I follow you. Are you talking about replacing creating the cache with @emotion/css in packages/eui/src/services/emotion/css.ts with @emotion/cache? Could you expand on what you mean exactly with The appliance of styles via className vs css props and what that has to do with the CacheProvider? 🤔

Or are you only referencing the part in the description about @emotion/react being a drop-in replacement for @emotion/css? Because in that case I do agree, I even mentioned in my comment a case where that has to be investigated further than just replacing the import: packages/eui/src/components/date_picker/react_date_picker.styles.ts:10

weronikaolejniczak avatar May 29 '25 07:05 weronikaolejniczak

@weronikaolejniczak My comment was based on reviewing the actual code mentioned in the issue (in provider.tsx). It was not to your comment at all (hence no tagging). We apparently did two separate initial investigations.

Edit: To your question:

are you only referencing the part in the description about @emotion/react being a drop-in replacement for @emotion/css?

Yes exactly, that's what I was referring to.

mgadewoll avatar May 29 '25 09:05 mgadewoll

@mgadewoll fyi the css in provider.tsx is actually coming from the file path I mentioned above

https://github.com/elastic/eui/blob/acc56997f45f9c2491111442e713da068f6c162d/packages/eui/src/services/theme/provider.tsx#L25

It's not imported from @emotion/css, that's why I focused on replacing the createEmotion (which is essentially createCache from @emotion/cache).

weronikaolejniczak avatar May 29 '25 09:05 weronikaolejniczak

which is essentially createCache from @emotion/cache

@weronikaolejniczak That's true for the cache migration. But it seems it also means the API changes and the css doesn't come from that instance anymore (as it's only a cache instance) and we'll need to use css from @emotion/react.

mgadewoll avatar May 29 '25 09:05 mgadewoll

Absolutely! Thank you for providing more context.

weronikaolejniczak avatar May 29 '25 09:05 weronikaolejniczak

Hey folks - I appreciate the work you've done to triage this issue. While this isn't a critical priority, this is something we could benefit from. Would it be possible to discuss a timeline for this?

legrego avatar Nov 17 '25 17:11 legrego

@legrego we have a meeting today, we will discuss if and when we can fit this in and let you know!

weronikaolejniczak avatar Nov 18 '25 09:11 weronikaolejniczak

@legrego We discussed it, updated the estimation and pulled it into "Do Next". It doesn't mean we will get to it tomorrow, this just means that it's on our radar and whenever someone has some time from larger planned initiatives, they will take a look.

@tkajtoch mentioned we could potentially keep using @emotion/css but hook into the cache they're exporting and configure nonce. This would significantly reduce the effort.

If we do need to update @emotion/css to @emotion/react (which would be good for consistency anyway) then it can take anywhere from 2 days to a week, depending on what comes out of testing. We can make sure everything works on our side but we'd need some manual tests in Kibana.

cc @JasonStoltz @mgadewoll

weronikaolejniczak avatar Nov 19 '25 11:11 weronikaolejniczak

Thanks @weronikaolejniczak, I appreciate the update!

legrego avatar Nov 19 '25 11:11 legrego

I added a spike issue for this -- let's try to figure out how feasible this is and gauge the risk and effort.

JasonStoltz avatar Nov 19 '25 21:11 JasonStoltz