eui
eui copied to clipboard
Update EUI Theme Provider to use `css` from `@emotion/react`
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?
Here just to triage.
Usages of css from @emotion/css:
packages/eui/src/components/code/code_block_line.styles.ts:16packages/eui/src/components/code/utils.tsx:23packages/eui/src/components/datagrid/data_grid_styles.stories.tsx:11packages/eui/src/components/datagrid/controls/fullscreen_selector.styles.ts:9packages/eui/src/components/date_picker/react_date_picker.styles.ts:10(note about how it needs vanillacssto pass aclassNamedirectly toreact-datepicker)packages/eui/src/components/overlay_mask/overlay_mask.styles.ts:9packages/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:25packages/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.
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 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 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 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).
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.
Absolutely! Thank you for providing more context.
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 we have a meeting today, we will discuss if and when we can fit this in and let you know!
@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
Thanks @weronikaolejniczak, I appreciate the update!
I added a spike issue for this -- let's try to figure out how feasible this is and gauge the risk and effort.