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

[SIP-37] Proposal to implement CSS-in-JS using Emotion πŸ‘©β€πŸŽ€ #9123

Closed
rusackas opened this issue Feb 12, 2020 · 12 comments
Closed
Labels
sip Superset Improvement Proposal
Projects

Comments

@rusackas
Copy link
Member

rusackas commented Feb 12, 2020

[SIP-37] Proposal to implement CSS-in-JS using Emotion πŸ‘©β€πŸŽ€

Motivation

Superset has been built with React components utilizing a few layers of styling. Most components are build using React-Bootstrap (v0.31), which is in turn built on Bootstrap 3. Bootstrap 3 is built using LESS, and is themed/overridden in Superset with Bootswatch and a Bootswatch theme called Cosmo, also built with LESS. At this point, efforts have been made to consolidate styling with LESS variables, and cleaning up use of LESS styles throughout the codebase, but some key problems remain:

  1. Upgrading to newer versions of Bootstrap and/or React-Bootstrap are non-trivial, as they require migration to SASS from LESS.
  2. Current Cosmo/Bootswatch themes, and indeed many of the custom styles elsewhere, will not be compatible with upgraded Bootstrap/React-Bootstrap
  3. Most importantly, the development experience in customizing and styling components is difficult. Changing existing styles can have unexpected fallout in unintended views/components. It's also difficult to track down all the existing styles affecting any given component, which may be scattered in a number of different LESS files.

Migrating to CSS-in-JS accomplishes a few goals:

  • Organizes code's separation of concerns by component, rather than by language
  • Allows simpler and cleaner implementation of an atomic design system
  • Allows variants triggered by props (e.g. <Button primary> or <Card small>)
  • Simplifying, segmenting, or removing LESS usage over time, simplifying the codebase (particularly in deprecating Bootswatch/Cosmo)
  • More robust theming capabilities
  • Component library agnostic
  • Snapshot serializers for CSS testing
  • Typescript support
  • Provides an easier path to restyle React-Bootstrap, and/or upgrade it!

Proposed Change

In short, the plan is to implement components using Emotion. This can be done immediately for new components with no fallout to the existing codebase, but can also be extended to wrap or rewrite existing components in the interest of modernizing old code and libraries.

Further implementation details below.

New or Changed Public Interfaces

N/A

New dependencies

Emotion and various submodules - MIT License
babel-plugin-emotion - MIT License
emotion-ts-plugin - MIT License
jest-emotion - MIT License

Migration Plan and Compatibility

The plan to move toward Emotion-styled components would take the following path:

  1. Coping/migrating LESS variables (colors, font sizes, etc) to JS based theme file, and provide it to all components in the React component tree via Emotions ThemeProvider HOC. This will lead to duplication of these variables, but this is temporary.
  2. Create all styles for NEW components using Emotion, either with their CSS prop using "object styles", the styled pattern using "tagged template literals", or the useTheme hook.

To extend this idea toward deprecating Bootswatch/Cosmo, and eventually upgrading React-Bootstrap, the following approach may be used:

  1. Migrating React-Bootstrap based components into similarly named wrapper components, and applying custom styles and theme variables as needed to shape the atomic component toward the target design(s). Here's a sketch of how a wrapped React-Bootstrap button might look, with some arbitrary custom styles illustrating prop and theme usage.
/** @jsx jsx */

import styled from '@emotion/styled';
import { withTheme } from 'emotion-theming';
import {
    Button as React_Bootstrap_Button,
    // @ts-ignore
  } from 'react-bootstrap';

const Button = styled(React_Bootstrap_Button)`

  font-family: ${(props) => props.theme.font-family};
  border-radius: ${(props) => props.theme.borderRadius};
  background-color: ${props => props.primary ? props.theme.colors.primary : props.theme.colors.secondary};

  a {
    padding: 0;
    opacity: 0.8;
    &:active {
      opacity: 1;
      border: 2px solid ${(props) => props.theme.colors.secondary.light2};
    }
  }
`;

export default withTheme(Button);
  1. Wherever possible, migrate LESS styles from Cosmo/Bootswatch/custom styles, and put it into atomic components.
  2. When LESS code has been sufficiently migrated into Emotion components, delete Cosmo theme and Bootswatch files (hooray!)
  3. When all React-Bootstrap imports/usages have migrated to Superset-owned Emotion-styled components, it should be possible to upgrade React-Bootstrap and make relatively minor/straightforward changes to correct any unexpected style changes.

In the event of an emergency involving support or implementation of Emotion, it should be fairly straightforward to migrate to Styled-Components as an optional ejection path. They follow the same patterns.

Rejected Alternatives

  • Styled Components - The leading package, and the leading competition to Emotion. Downsides are that it is larger, less performant (they're addressing this), and that it doesn't (yet) support sourcemaps. The scale of this community is the largest, so it may be warranted to swtich to this in the future, which at this point is relatively trivial.
  • JSS - Popular, but that seems to be in large part due to its use in Material UI. It doesn't seem to provide selling points that outweigh the features, ease of use, and emerging patterns provided by Emotion (and Emotion-like) libraries
  • Glamorous - Follows similar patterns to Emotion/Styled-Components. The founder has officially recommended that users switch to Emotion (over Styled-Components) and has provided a codemod to do so.
  • Aphrodite, Radium, Styletron - These (and others) provide plenty of feature overlap in their CSS-in-JS approaches, but seem lacking in OSS adoption and community support.
@mistercrunch mistercrunch added the sip Superset Improvement Proposal label Feb 12, 2020
@rusackas rusackas changed the title [SIP] Proposal to implement CSS-in-JS using Emotion πŸ‘©β€πŸŽ€ [SIP-37] Proposal to implement CSS-in-JS using Emotion πŸ‘©β€πŸŽ€ Feb 12, 2020
@etr2460
Copy link
Member

etr2460 commented Feb 13, 2020

What sort of TypeScript guarantees do we get with Emotion? When adding the css tag to a component to we get safety of only being able to set valid CSS properties and valid values for these properties?

@rusackas
Copy link
Member Author

@etr2460 For the css prop (using object styles), Emotion uses the csstype package under the hood to check for valid CSS properties and values. There's a pragma to add at the top of the file for this to work (unless using babel plugin, apparently).

For the styled(someElement)`...` syntax, it lets you type props, and checks their validity.

The docs/examples on the matter here explain it more eloquently than I can :)

@DiggidyDave
Copy link
Contributor

the plan is to implement components using Emotion. This can be done immediately for new components with no fallout to the existing codebase, but can also be extended to wrap or rewrite existing components in the interest of modernizing old code and libraries.

It sounds like short-term (or longer) this will further fragment the approach in the codebase. Given all of the other large changes in flight and in the pipeline (including design overhauls etc), is the cost/benefit there to justify doing this now? Who is committing to converting the existing codebase rather than fragmenting it further?

@metaperl
Copy link

Hot @DiggidyDave I do agree with that ;)

@rusackas
Copy link
Member Author

rusackas commented Feb 13, 2020

@DiggidyDave this proposal is due largely to serve the goals of the design overhaul. That redesign will require a pretty massive revamp of the existing LESS codebase anyway. This also relates to the cleanup of LESS code we've been working on to move toward a set of variables (a theme, effectively). These efforts combined seem like the best way to

  • Have a more homogenous look and feel as we migrate new/updated components to the new design, and
  • Isolate the styling concerns of new or restyled components, so they don't bleed to other (older) components
  • Allow us to reduce reliance on the LESS codebase, which is built primarily around aging/deprecated libraries.
  • Make it exponentially easier to style components in service of the new design direction

So, while I don't disagree with your stance about fragmentation, I would suggest that it is the easiest means of migration to the new design system, and it is compatible with the existing setup (LESS still works on any/all components). It will also make styling (and comprehending existing styling) of components easier for future devs.

@DiggidyDave
Copy link
Contributor

Thanks @rusackas I wouldn't call my comment a "stance" so much as an expression of a concern. πŸ˜„ The benefits you've articulated seem substantial, I was just making sure there is a discussion of any potential "cons" here among stakeholders.

@graceguo-supercat
Copy link

graceguo-supercat commented Feb 14, 2020

Thanks @rusackas for making this SIP.
I happened to see this article recently. I am very interested in this feature from JSS:

JSS is a framework-agnostic CSS-in-JS library with which you can dynamically generate CSS styles based on the state of your components.

I had a small feature already in the dashboard code that can be much cleaner if i had help from a css-in-JS framework:
https://github.com/apache/incubator-superset/blob/291306392443a5a0d0e2ee0cc4a95d37c56d4589/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx#L72
In the above code, when user click on filter indicator on a chart, I want to show an animation on a dropdown in the filter_box that contains the user interested field. Without css-in-js, I need to pass props between nested React components which is kind of very troublesome (and cause many unnecessary rerendering).

Can Emotion handle this case easily? I am new to all css-in-js libraries. Once we decided, i'd like to re-write this module to use our pick. Thanks!

@kristw
Copy link
Contributor

kristw commented Feb 14, 2020

Just want to add another long-term benefit I see from using css-in-js is avoiding dead css code. Currently it is very difficult to check if a css rule is ever used at all. With css-in-js and proper linting, we could get rid of unused rules more confidently and naturally.

@rusackas
Copy link
Member Author

@graceguo-supercat It is definitely possible to style a component based on state using react hooks. I've attached a teensy demo of this using Create React App, attached here. Just npm ci and yarn start. You'll be able to click the button to increment an integer (via the useState hook), and emotion renders the css tag to be pink or teal depending on whether the number is odd or even.

EmotionalState.zip

@bugzpodder
Copy link

There are a couple of alternatives of CSS-in-JS flavors out there, just summing it up here in case they may be of some interest.

The following three libraries are being developed by Brent Jackson and the Gatsby team based around the idea of a design system.

Styled-System: Works on top of styled-components/emotion:
https://medium.com/styled-components/build-better-component-libraries-with-styled-system-4951653d54ee
https://styled-system.com/ecosystem

Rebass: Some components built on top of styled system
https://twitter.com/jxnblk/status/1158479676932853767
https://rebassjs.org/

Theme UI: Inspired by styled-system
https://twitter.com/jxnblk/status/1137028468099768320
https://jxnblk.com/blog/design-graph/

Tailwind: utility-first css library, more similar to bootstrap and easier to migrate?
https://tailwindui.com/
https://tailwindcss.com/
https://www.youtube.com/watch?v=R50q4NES6Iw

Linaria: zero-runtime css-in-js
https://github.com/callstack/linaria/blob/master/docs/BENEFITS.md

Stylex: Facebook's zero-runtime css-in-js approach. (not actually available, only demoed in react conf)
https://blog.annamalai.me/posts/new-age-css-in-js/#stylex

CSS/SCSS modules:
you probably are aware of this but just put it here in passing.

@rusackas
Copy link
Member Author

rusackas commented Mar 4, 2020

@bugzpodder Thanks for the list. I think something like Styled-System may come into play eventually, but that has a prerequisite of the adoption of Emotion or Styled-Components, which is enough scope for this SIP, I think :)

Linaria is interesting, and may be worth considering if we run into performance issues with Emotion. The good news is, it follows a largely similar pattern of code style, so it should be a not-too-painful migration if warranted. I'd still prefer to go with the front-runners for the time being, for the expanded community at the very least.

I've been long-intrigued by TailWind as well, and plan to use it for other projects, but it looks like a serious burden to migrate the codebase onto or off of it.

@rusackas
Copy link
Member Author

Voted in, work in progress, closing the SIP as an issue. More good stuff to come! Thanks all!

@suddjian suddjian mentioned this issue Apr 21, 2021
rusackas added a commit to rusackas/emotion that referenced this issue Feb 27, 2023
Apache Superset has been using Emotion for quite a while now (original proposal [here](apache/superset#9123), and best practices captured [here](https://github.com/apache/superset/wiki/Emotion-Styling-Guidelines-and-Best-Practices). We'd be proud to have our link on the roster!
Andarist pushed a commit to emotion-js/emotion that referenced this issue Feb 27, 2023
Apache Superset has been using Emotion for quite a while now (original proposal [here](apache/superset#9123), and best practices captured [here](https://github.com/apache/superset/wiki/Emotion-Styling-Guidelines-and-Best-Practices). We'd be proud to have our link on the roster!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sip Superset Improvement Proposal
Projects
Status: Implemented / Done
SIPs
APPROVED (in mailing list)
Development

No branches or pull requests

8 participants