Emotion: Support class names

[kentcdodds]

Hi friends 👋

When I was working on glamorous, a nice feature was how easy it was to integrate a glamorous component with an existing stylesheet of class names: https://codesandbox.io/s/73W7nZ6BQ

const Alert = glamorous.div('alert', props => `alert-${props.type}`);

I really need this with emotion, but I think that react-emotion treats string arguments as css (which makes sense). I'd really love to be able to make a react-emotion component that works with class names though. Is this currently possible with some special feature of which I'm currently unaware?

Looking through the code I don't think this is currently possible. But I think that we should be able to make it work if we add a condition for if the string starts with a . (and perhaps also a condition that it doesn't include a {) then we can assume the user wants to provide a static classname they want applied. Thoughts?

Basically, here's what I want to be able to do with emotion:

const Alert = styled('div')('.alert', props => `.alert-${props.type}`)

I'm willing to make this contribution. Though I know a lot is in the air for Emotion 10 at the moment...

[mitchellhamilton]

This wouldn't be great for performance since this would mean we would have to check every single string passed to emotion with a regex and handle that which could get expensive for large apps with lots of styles.

It would also create so many ambiguous cases that could get very confusing, for example, what would happen in this case?

let SomeComp = styled.div`
  ${".some-class"} {
    color: hotpink;
  }
`;

I think it would also introduce confusion since we have to say essentially any strings passed to styles will be treated as css except if it's in styled and it begins with a ., etc... Further to that, this would be weird for composition since you couldn't create these with css. Currently we also make the assumption that styled stringifies styles in the same way as other places do, breaking that by doing this would break some build time optimizations(and more i have planned)

What I would personally recommend is doing something like this

let Alert = ({ className, type, ...props }) => (
  <div
    className={classnames(
      "alert",
      `alert-${type}`,
      className
    )}
    {...props}
  />
);

I know it's a bit boilerplatey but doing it like this also has the added advantage that people don't have to look at some documentation to learn the particulars about some API.

[mxstbr]

Just use styled-components and .attrs 😜

const Alert = styled.div.attrs({
  className: props => `alert-${props.type}`
})`
  color: blue;
`

Just kidding just kidding, sorry for being unhelpful!

[mitchellhamilton]

I know you're kidding but I strongly dislike .attrs. If someone wants to use it, they have to learn the specifics of how it works whereas if they use normal a normal react component, it's like any other React component they have so they don't have to learn something specific to a particular library. In general, I think if something requires slightly more code but is easier to understand then in general it's better to favor code that is easier to understand.

This is also one of the reasons I massively prefer the css prop to the styled api(not just referring to s-c, I include emotion's styled api in that) because I don't want to add a big abstraction that people have to learn when I can use tools that I already have access to, in this case, creating a regular react component and adding a prop to an element.

[kentcdodds]

Thanks

The reason I don't want to do this:

let Alert = ({ className, type, ...props }) => (
  <div
    className={classnames(
      "alert",
      `alert-${type}`,
      className
    )}
    {...props}
  />
);

Is because then I can't do Alert.withComponent('span') :-(

[tkh44]

You could reverse the process and wrap the component

let LegacyAlert = ({ className, type, ...props }) => (
  <div
    className={cx('alert', `alert-${type}`, className)}
    {...props}
  />
);

const Alert = styled(LegacyAlert)()

This util would help here. You could probably omit the tag arg and default to 'div' and use withComponent to change the tag at the point of use.

function makeBase(tag, ...classNames) {
  return props =>
    createElement(tag, {
      ...props,
      className: cx(
        ...classNames.map(
          cn => (typeof cn === "function" ? cn(props) : cn)
        ),
        props.className
      )
    });
}

const Alert = styled(
  makeBase("div", "alert", p => `alert-${p.type}`)
)();

[kentcdodds]

I'll probably go with something like this. Thanks @tkh44!

[tkh44]

Another issue is that style order and precedence becomes indefinite

const Alert = styled('div')('.alert', { color: 'hotpink' }, props => `.alert-${props.type}`)

Because the emotion style being inserted in the document after .alert-${props.type}, the color would be "hotpink" despite whatever color .alert-${props.type} contains.

In all other cases, the styles are composed in the expected order just like Object.assign

[kentcdodds]

I think what I'm going to do is add an emotion plugin that ensures emotion styles _always_ win (prefixing an id to all selectors or whatever). I did this with glamor and it worked nicely.

[kentcdodds]

Hmmm... Can either of you think of any way to make this work:

https://codesandbox.io/s/xoxw25nn84

const Alert = styled(makeBase('div', 'alert', p => `alert-${p.type}`))()
const SpanAlert = Alert.withComponent('span')

The <SpanAlert /> is rendered as a span, but without the class names. It makes sense to me why it doesn't work, but I'm hoping we can figure out a way to make this possible. Even if I have to do something weird I really want this to work...

[kentcdodds]

What would you all say to adding an option?

const Alert = styled('div', {classNames: ['alert', p => `alert-${p.type}`]})()

I'd be happy to code that up.

[kentcdodds]

Ugh, you know what friends, nevermind. I don't even like this. I'll make this work differently. Thanks!