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

Working directly with classNames using createEmotion post-Emotion v10 #1264

Closed
DesignerDave opened this issue Mar 11, 2019 · 11 comments
Closed

Comments

@DesignerDave
Copy link

  • emotion version: 10.0.7
  • react version: 16.4.2

Problem description:
I'm a developer on a UI library that uses Emotion. We use it because we want styles to be bundled and applied with each component when they're imported, so we don't add any implementation complexity (importing more than one file to use a component, adding a context provider to the consuming app, etc.) to the consuming app.

We have had to work through a few issues with Emotion:

  1. Styles were being appended, rather than prepended to the document head, which overwrote the consuming application's styles. We solved this by creating a custom instance of Emotion that prepends a <div> to the document head, and configuring the instance to use that as the container. It is less than ideal, but works in all the browsers we support.
  2. The className composition method, cx(), wasn't handling certain uses of the & + & selector that the library seems to support otherwise. We now combine classNames with a simple (...args) => args.join(' '); (simplistic, but gets the job done effectively).

We're undertaking the upgrade from Emotion 9 -> 10 now, and it seems like classNames are being abstracted away, and createEmotion is being phased out for CacheProvider.

Quite simply, while we have managed to upgrade to version 10 of the original create-emotion package, we haven't been able to get our individually-consumable components working using CacheProvider + the scoped @emotion/x packages.

Suggested solution:
Ideally, we would continue to create classNames through the css() template literal processor function, and work without abstracting classNames away in favor of the css prop / composition. I'm aware of the new ClassNames component, but given the sheer length of the styles of some of our components, it would be horrible to compose those styles in the render method of the components.

Additionally, having some way to more easily have style tags prepended, rather than appended to the document head would be excellent.

What we would like clarity on is: are you planning on continuing support of the emotion and create-emotion packages?

@hswolff
Copy link

hswolff commented Mar 12, 2019

(on same team)

I'll also extend this question

Additionally, having some way to more easily have style tags prepended, rather than appended to the document head would be excellent.

And wonder if you'd be open to providing more configuration to control how style elements are added to the document? Primarily we'd like to prepend the style tags to the head element rather than append, so that they come first in the page.

Also a general meta question is: Do you think Emotion is a good tool to use for library authors in addition to application authors?

@jcharry
Copy link
Contributor

jcharry commented Aug 14, 2019

+1 We are currently experiencing this same issue - with an older component library that we're trying to slowly bring emotion into. Styles being appended to head is problematic in that we have a lot of usages of classNames being used to override local component styles. Appending styles makes it potentially impossible to fully bring emotion in as it may break lots of legacy usages.

An option to be able to simply choose to prepend vs append would be really nice.

@Andarist
Copy link
Member

Andarist commented Nov 3, 2019

The className composition method, cx(), wasn't handling certain uses of the & + & selector that the library seems to support otherwise. We now combine classNames with a simple (...args) => args.join(' '); (simplistic, but gets the job done effectively).

By this sole description, I'm not sure what is not working correctly. I see that you have reported a dedicated issue for this and it's basically about this #1417 . I'm afraid that there is not much that we can do regarding that right now.

Quite simply, while we have managed to upgrade to version 10 of the original create-emotion package, we haven't been able to get our individually-consumable components working using CacheProvider + the scoped @emotion/x packages.

The answer to this depends on why do you have to use custom cache/instance. If you can't expect the consumer to wrap their app with your CacheProvider then you can still use create-emotion package and just use vanilla emotion APIs, that won't let you use css prop though but I have no idea how we could make it work for your use case.

I'm aware of the new ClassNames component, but given the sheer length of the styles of some of our components, it would be horrible to compose those styles in the render method of the components.

With ClassNames you still can construct your styles on the top of the file (in example), you only have to actually call provided css/cx in your render.

Additionally, having some way to more easily have style tags prepended, rather than appended to the document head would be excellent.

We'll give it a thought, thanks for the feedback.

What we would like clarity on is: are you planning on continuing support of the emotion and create-emotion packages?

Definitely, emotion is composable and we plan to support vanilla flavor indefinitely (AFAIK).

Also a general meta question is: Do you think Emotion is a good tool to use for library authors in addition to application authors?

Definitely yes, I personally work on an emotion-based UI kit library. It comes with a few caveats though (mainly around cache/instance customization mentioned above).

@DesignerDave
Copy link
Author

By this sole description, I'm not sure what is not working correctly. I see that you have reported a dedicated issue for this and it's basically about this #1417 . I'm afraid that there is not much that we can do regarding that right now.

Understood.

The answer to this depends on why do you have to use custom cache/instance. If you can't expect the consumer to wrap their app with your CacheProvider then you can still use create-emotion package and just use vanilla emotion APIs, that won't let you use css prop though but I have no idea how we could make it work for your use case.

We need a custom instance to ensure that styles are prepended, not appended to the document <head>. When the styles are appended, it prevents applications consuming the library from overwriting Emotion styles, unless they over-qualify their CSS selectors.

With ClassNames you still can construct your styles on the top of the file (in example), you only have to actually call provided css/cx in your render.

Makes sense. Since this issue was written, we've adopted patterns to work with Emotion a bit more, rather than against it.

We'll give it a thought, thanks for the feedback.

Thank you – this is currently our largest pain-point with Emotion, and otherwise we love using Emotion!

Definitely, emotion is composable and we plan to support vanilla flavor indefinitely (AFAIK).

Good to hear 😁

@Andarist
Copy link
Member

Andarist commented Nov 4, 2019

When the styles are appended, it prevents applications consuming the library from overwriting Emotion styles, unless they over-qualify their CSS selectors.

Out of curiosity - how applications are supposed to overwrite your emotion styles which have hashes in then?

Makes sense. Since this issue was written, we've adopted patterns to work with Emotion a bit more, rather than against it.

Great to hear that! I hope you like it.

@DesignerDave
Copy link
Author

Out of curiosity - how applications are supposed to overwrite your emotion styles which have hashes in then?

We allow passing a string for a className to our components and compose them with cx, passing them last. If they use Emotion, the styles overwrite our emotion styles due to the order they're being composed in, and otherwise their className is simply added to the classlist normally. If a consuming application is using a standard CSS stylesheet, they would need to over-qualify the selectors if we didn't have a custom emotion instance that ensured styles were prepended, rather than appended.

@sneridagh
Copy link

+1 to include an option for prepending rather than appending the styles.
We are building an extensilble CMS in React (https://github.com/plone/volto) and we would like for the integrators be able to override via CSS the vainilla components provided by the CMS. So users that are not that into JS would also extend and customize the CMS look and feel using plain CSS (that would be applied after Emotion ones).

@Andarist have you already thought into this? Is it difficult to implement?

@sneridagh
Copy link

@Andarist Would be here? given an option, prepend/append?
https://github.com/emotion-js/emotion/blob/master/packages/cache/src/index.js#L80

@Andarist
Copy link
Member

prepend option has been implemented in the upcoming v11

@DesignerDave
Copy link
Author

Cheers @Andarist! Looking forward to it

@Andarist
Copy link
Member

Closing this issue as I believe the OP has found a way to work around this limitation.

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

5 participants