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

@emotion/core vs vanilla emotion #1883

Closed
colinhacks opened this issue Jun 2, 2020 · 9 comments
Closed

@emotion/core vs vanilla emotion #1883

colinhacks opened this issue Jun 2, 2020 · 9 comments

Comments

@colinhacks
Copy link

@colinhacks colinhacks commented Jun 2, 2020

UPDATE: I published final writeup on this topic here. tl;dr I think vanilla emotion is a better option for the majority of React projects: https://vriad.com/blog/emotion-core-vs-vanilla-emotion


I'm a huge fan of the emotion module but am somewhat bewildered by some of the tradeoffs made in the design of @emotion/core.

I wrote up some thoughts as a blog post but haven't published them yet. I want to give the Emotion team some time to respond to my criticisms. I don't want to put publish a post that misrepresents this package or its design goals. It's very possible I simply don't understand some of the use cases you were optimizing for. Here's a link to a Gist containing the text of my post: https://gist.github.com/vriad/23956586223e36931718399c0caa2eef

I'd hugely appreciate anyone who can shed some light on these decisions for me.

I absolutely love vanilla emotion which is why I'm sad to see it getting deprioritized in favor of core. I say this in the post but it's worth repeating: I love Emotion and I'm hugely indebted to your guys for all your great work!

@Andarist
Copy link
Member

@Andarist Andarist commented Jun 2, 2020

not the inexplicably-named @emotion/core module

Just a note - we understand how this name is confusing and we are in the process of renaming this package to @emotion/react. It will be published as such when v11 gets released.

If you're in one of those buckets, there's a fallback solution! Just copy these two lines at the top of every React component you want to style with Emotion:

Well, truth to be told - this is the preferred way of using @emotion/core. It doesn't require any additional configs and it works with the majority of the community tooling. It also makes it explicit for code readers that a particular file is using custom "createElement" instead of the regular one. For beginners, or just when exploring a different codebase, this might actually be seen as a positive thing - it's inclusive.

Yes, you have to import jsx from @emotion/core even if you don't use it.

I don't agree with this statement - you have to only add this import if you are using jsx. In the very same way that you need to import React when you are using regular JSX - it only might be less of a pain because you often have React already in the scope anyway.

Note: this part might change in the future though as React has introduced automatic transform recently (babel/babel#11154), we are waiting for the support for it to get released in React itself and for it also being supported by TypeScript team which is on their roadmap (from what I believe).

The usage of a non-native css prop kneecaps the portability/extensibility of your component; it's now unusable in any codebase that isn't configured to use @emotion/core. Bad!

The goal is to provide predictable styling - we can only do that if we control the styling, thus this actually helps us ensure that you are using emotion-generated styles. Otherwise, you can find yourself in the middle of specificity wars once again.

Also: render props?! What is this, 2018?

This really sounds very aggressive and out of place. Even if a little bit quirky render props are still valid for some use cases and this is one of them (at least for us). I'm actively thinking about a different API for this but it has to work with 0 config SSR and that's not an easy feat for this feature. We don't believe this is much of an issue because you rarely should use non-emotion styles (ideally you wouldn't have to use them at all).

don't appreciate that Emotion tries to drive people towards @emotion/core instead of tellling them how insanely easy it is to set up SSR themselves with vanilla emotion.

Many people are tired of configuring things. The configuration also spills itself on all of the consumers. Emotion is used, sort of, as an engine for multiple other libraries, and with 0config approach they don't have to educate their consumers (who might not even be aware of using Emotion under the hood) that they need to setup something for SSR. It just works.

Emotion provides no concrete guidance on how to do this, just a link to a sample Next.js project (this one).

Well - it's not our responsibility to document how to integrate Emotion with all of the fancy meta frameworks. We understand that we could maybe give guidance on how to integrate with the most popular ones, but there are already dedicated packages for this, so not sure why we should bother duplicating efforts. If you feel that their docs are lacking something you can always try to improve them by providing helpful PR. Also - those packages are only needed if you want to do something custom. 0config approach is, well, 0 config so it doesn't require any extra documentation. That's why we recommend it - we don't have to integrate in any special way with any other tool, things should just work if you stick to the 0 config approach. The time we can spare on the project is also not infinite, so we don't feel like documenting the stuff we don't really recommend.

Themeing: Touting this "feature" is preposterous

I believe many would disagree and you also sound very aggressive here which I don't find to be OK. However, we agree that builtin theming is not needed, there is an open PR which we'd like to revive with arguments laid out against it: https://github.com/emotion-js/emotion/pull/973/files . We've found out that people like the builtin theming though, so we are keeping it - but we plan to emphasize that there is a better way of doing things in v11 docs.

Perhaps there are lots of great reasons to use @emotion/core that I'm not seeing and aren't in the Emotion docs

Because we don't give you string class names back, but rather just opaque objects, the rule insertion is lazy. This might not be as interesting on its own, but it allows us to customize injection through a <CacheProvider>. Use cases for this are not as common - but they exist and once you ever come across one then you might be very thankful for the path we've chosen because otherwise re-implementing stuff to accommodate new requirements could be very hard. With vanilla Emotion to customize this stuff, you need to create a custom instance and use it everywhere. Main things that can be customized:

  • prefixing rules
  • parser plugins
  • container element (very important for rendering into iframes)
  • nonce (very important for CSP)

This allows libraries built on top of Emotion don't care about this stuff because, if needed, the final consumer can decide about this stuff.

I'll also be incredibly thankful and indebted to them for building the true "core" emotion package, which is a glorious achievement and a huge win for developers!

If the chosen tradeoffs are not for you then you are welcome to continue using vanilla Emotion. We don't plan to remove the support for it.

@Andarist Andarist changed the title @emotion/core considered harmful @emotion/core vs vanilla emotion Jun 2, 2020
@colinhacks
Copy link
Author

@colinhacks colinhacks commented Jun 2, 2020

@Andarist I really appreciate all this feedback! I want to apologize for the tone - it was far too aggressive in all the cases you mentioned. I got a little carried away. I certainly won't publish this until that's been corrected.

I'll admit upfront I haven't been following the API updates coming in v11 - I'll look more deeply into that.

I agree that

  • my complaint about importing jsx is invalid
  • it's not your job to document how to do SSR with Next/Gatsby
  • people are tired of configuring things (which is part of my problem with core!)

There's a lot I don't understand about the use cases you're optimizing for. Frankly I hope I never encounter a scenario that necessitates the use of CacheProvider 😅! Sounds hard.

I don't understand how the css prop makes styling more predictable than vanilla Emotion. Is there a resource you can direct me to that explains that a bit more? Or am I misreading that?

My primary source of my frustration is something that I only briefly alluded to in my writeup: the documentation explicitly steers people to @emotion/core over emotion, yet for a majority of cases emotion is the better choice. With core:

  1. You get theming, but theming is better handled with Context/hooks/other language features (seems we agree on this)
  2. The css prop breaks portability/encapsulation (one of the core philosophical pillars of React)
  3. You need to modify the markup to get access to cx

It seems like all of these tradeoffs were made in the name of zero-config SSR, which isn't a consideration for a hefty majority of React-based SPAs.

My recommendation would be to make these tradeoffs more explicit in a documentation, and two present these approaches as two equally valid options (instead of explicitly recommending core for React projects). Based on npm downloads, it's clear that emotion has plateaued in usage in the last 18 months, whereas core has tripled. I suspect in a majority of cases the developer would have been better served by vanilla, but didn't think to go against the officially recommended route. If that developer found Emotion via NPM or this repo's README, they may not even know about the existence of the vanilla option - it's not mentioned anywhere.

You should know that all of my thoughts are colored by the fact that I LOVE cx. My whole approach to styling is dependent on the composition of many small utility classes. Getting rid of simple, markup-free cx usage in favor of zero-config SSR isn't a tradeoff I agree with (and that's okay!). But the fact that vanilla/cx aren't the "recommended" approach (and are therefore hugely underutilized) makes me a little sad.

Thanks again for your feedback. I'll let you know if I decide this post is worth publishing. And great work - seriously.

@colinhacks
Copy link
Author

@colinhacks colinhacks commented Jun 8, 2020

After reading Andarist's feedback carefully and doing some additional research, I'm still confident in my assessment that vanilla emotion is a better choice for a hefty majority of React projects.

A small number of scenario's warrant the usage of @emotion/core: SSR (though this is quite easy to set up for Next.js or Gatsby), a need to customize the auto-prefixing, and a handful of even less common scenarios. For the average React project, vanilla emotion should really be the officially recommended approach.

I published final writeup explaining my reasoning here: https://vriad.com/blog/emotion-core-vs-vanilla-emotion

@Andarist
Copy link
Member

@Andarist Andarist commented Jun 12, 2020

There's a lot I don't understand about the use cases you're optimizing for. Frankly I hope I never encounter a scenario that necessitates the use of CacheProvider 😅! Sounds hard.

Sure, those use cases are not as common - but they happen. Just got a question regarding such use case a 1 hour ago: #760 (comment)

I don't understand how the css prop makes styling more predictable than vanilla Emotion. Is there a resource you can direct me to that explains that a bit more? Or am I misreading that?

I was referring to your previous remark of:

The usage of a non-native css prop kneecaps the portability/extensibility of your component; it's now unusable in any codebase that isn't configured to use @emotion/core. Bad!

So in here - we are not really that concerned with the ease of use in codebases not using @emotion/core (or Emotion) because integrating with external styling systems goes against the predictability of styles as we can't control external styles.

the documentation explicitly steers people to @emotion/core over emotion

Well, I'm really glad that you find vanilla Emotion useful - it certainly is. But it's hard for us to change this rhetoric in the docs as we believe that @emotion/core is better as it's more powerful and for the most of the time it can be used nearly identical to emotion. If you feel like the docs could outline cons/pros of both approaches better then please prepare a PR - I would certainly consider merging quality docs PR, but from our POV @emotion/core should still be recommended over emotion.

The css prop breaks portability/encapsulation (one of the core philosophical pillars of React)

I don't understand this point at all, could you elaborate?

You need to modify the markup to get access to cx

As mentioned above - we don't think you should integrate often with external elements, having to add ClassNames is a minor inconvenience but it also has its merits which we believe are important (working with 0config SSR).


Maybe we could think of auto-adding <ClassNames/> wrapping in our Babel plugin, which would improve the authoring side of things, but wouldn't change the generated React tree. Would something like this address your concerns a little bit? 🤔 This is just a rough idea, I don't know if we really would like to add something like this. If we would then cx would be accessible by using useCx or similar and would only be accessible from within React components and not from custom hooks.

nicl added a commit to guardian/support-dotcom-components that referenced this issue Jul 22, 2020
This is required to use the emotion cache, which Automat uses as
part of its shadow dom support.

In addition, Source uses @emotion/core and standardising helps
avoid having to load both.

Note, an interesting discussion on the differences between
emotion and @emotion/core can be found here:
emotion-js/emotion#1883.
nicl added a commit to guardian/support-dotcom-components that referenced this issue Jul 22, 2020
This is required to use the emotion cache, which Automat uses as
part of its shadow dom support.

In addition, Source uses @emotion/core and standardising helps
avoid having to load both.

Note, an interesting discussion on the differences between
emotion and @emotion/core can be found here:
emotion-js/emotion#1883.
nicl added a commit to guardian/support-dotcom-components that referenced this issue Jul 22, 2020
This is required to use the emotion cache, which Automat uses as
part of its shadow dom support.

In addition, Source uses @emotion/core and standardising helps
avoid having to load both.

Note, an interesting discussion on the differences between
emotion and @emotion/core can be found here:
emotion-js/emotion#1883.
nicl added a commit to guardian/support-dotcom-components that referenced this issue Jul 23, 2020
This is required to use the emotion cache, which Automat uses as
part of its shadow dom support.

In addition, Source uses @emotion/core and standardising helps
avoid having to load both.

Note, an interesting discussion on the differences between
emotion and @emotion/core can be found here:
emotion-js/emotion#1883.
nicl added a commit to guardian/support-dotcom-components that referenced this issue Jul 27, 2020
This is required to use the emotion cache, which Automat uses as
part of its shadow dom support.

In addition, Source uses @emotion/core and standardising helps
avoid having to load both.

Note, an interesting discussion on the differences between
emotion and @emotion/core can be found here:
emotion-js/emotion#1883.
nicl added a commit to guardian/support-dotcom-components that referenced this issue Jul 28, 2020
* Reduce globals

* Migrate to @emotion/core from vanilla emotion

This is required to use the emotion cache, which Automat uses as
part of its shadow dom support.

In addition, Source uses @emotion/core and standardising helps
avoid having to load both.

Note, an interesting discussion on the differences between
emotion and @emotion/core can be found here:
emotion-js/emotion#1883.
@Alphy11
Copy link

@Alphy11 Alphy11 commented Jul 20, 2022

Just to chime in here for a second, as somebody who went in on the typical @emotion/react approach, I am now looking to eject from all of @emotion/react for one reason: Global prop modifications have made my development cycle pretty difficult. It's hard enough to debug type mismatches when React will simply tell me not assignable to LibraryManagedAttributes without a library modifying it. I absolutely love this product, and would continue to use @emotion/react if the global prop modification was portable. But I am ejecting myself from anything that globally modifies JSX types as a first priority.

@srmagura
Copy link
Member

@srmagura srmagura commented Jul 20, 2022

Hey @Alphy11, could you explain the problem you are encountering in more detail? Or provide a CodeSandbox that shows it. I have never gotten the error not assignable to LibraryManagedAttributes from TypeScript.

@Alphy11
Copy link

@Alphy11 Alphy11 commented Jul 20, 2022

Sorry, this was sent prematurely with too little context. I want to say that I think @emotion/react is awesome, and for building an app, is a MUCH nicer developer experience, including the css prop. But, my use case has been building out a reusable component library, and so I have loads of strange patterns, and complex types. One I have been struggling with lately has been enabling Polymorphic components to be used, so you could do something like:

<Component as="button" type="submit" />
// or
<Component as={ReactRouterLink} to="/home" />

Which comes along with loads of types complexity, and digs into React's types since you're accounting for both native elements, AND components.

So I get weird error messages like this:

Type '{ style: { pointerEvents: string; }; } & Omit<ClickableProps<Tag>, "as" | "isDisabled"> & { children: boolean | ReactChild | ReactFragment | ReactPortal; ref: PolymorphicRef<...>; "aria-disabled": boolean | undefined; css: CSSProp; disabled: boolean | undefined; }' is not assignable to type 'IntrinsicAttributes & { css?: Interpolation<Theme>; } & LibraryManagedAttributes<Tag, any>'.
  Type '{ style: { pointerEvents: string; }; } & Omit<ClickableProps<Tag>, "as" | "isDisabled"> & { children: boolean | ReactChild | ReactFragment | ReactPortal; ref: PolymorphicRef<...>; "aria-disabled": boolean | undefined; css: CSSProp; disabled: boolean | undefined; }' is not assignable to type 'LibraryManagedAttributes<Tag, any>'.ts(2322)

And the problem is almost certainly not with the CSS prop here, but if I want to debug what LibraryManagedAttributes needs to be here, it's no longer a central place. This experience has felt a lot like extending native prototypes (like adding array function). It's super useful, until something goes wrong, and then you're not quite sure where everything comes from. So I would ideally like the ability to opt out of emotion's JSX replacement, and instead just use it as a hook, without any global type extensions, or "magic" while still getting all of the other react features like theme and cache in context.

Happy to spin up a sandbox if you're still curious, but seemed a bit irrelevant to the conversation since it's mostly an likely-unrelated TS error

@srmagura
Copy link
Member

@srmagura srmagura commented Jul 20, 2022

That is an advanced use case from a TypeScript perspective. To make it work, you'll probably have to dig into the .d.ts files in packages/react/types in our repo.

If you want to avoid the complexity introduced by the css prop, you could use @emotion/css or @emotion/styled.

@Alphy11
Copy link

@Alphy11 Alphy11 commented Jul 20, 2022

Yeah, totally agree it's way advanced. Just throwing a reason to separate out the react functionality from the JSX pragma. Not worth it just for my use case, but providing the feedback to give the data point!

FWIW, @emotion/styled depends on @emotion/react and therefore includes the css prop as will into the JSX namespace.

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

4 participants