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 10 #637

Closed
14 tasks done
emmatown opened this issue Apr 27, 2018 · 82 comments
Closed
14 tasks done

Emotion 10 #637

emmatown opened this issue Apr 27, 2018 · 82 comments

Comments

@emmatown
Copy link
Member

emmatown commented Apr 27, 2018

Emotion 10 is going to introduce some new APIs for React users.

The main aim with these new APIs is to move all insertion to inside the React tree, this enables a couple of things:

  • SSR just works because we can render style elements directly into the React tree.
  • Setting things like nonces, container elements and stylis plugins is much easier since it only requires a Provider at the top of the tree.
  • Global styles can change over time and be removed
  • It forces people to think in terms of styling elements instead of creating class names
  • The css prop works without the babel plugin because it's implemented as a custom createElement (this isn't necessarily a result of moving things inside the React tree but a side-effect of the new implementation)

The main new APIs are a custom createElement for the css prop and a Global component for global styles.

Example

The exact APIs are likely to change before Emotion 10 is released.

/** @jsx jsx */
import { jsx, Global } from '@emotion/core'
import css from '@emotion/css'
import { render } from 'react-dom'

render(
  <div>
    <h1
      css={css`
        font-size: 500px;
      `}
    >
      This is really big
    </h1>
    <Global
      css={{
        body: {
          backgroundColor: 'hotpink'
        }
      }}
    />
  </div>,
  document.getElementById('root')
)

If you want to try this, look at https://github.com/emotion-js/next but note that there will be breaking changes very often for now.

Another change that emotion@10 will bring is that styled.div and etc. will work without a babel plugin. The tag list will be included in @emotion/styled but if you use @emotion/styled.macro with babel-plugin-macros or the babel plugin, styled.div will be replaced with styled('div') and @emotion/styled-base will be imported instead which doesn't include the tag list.

Compatibility with Preact and non-React-like libraries

While the majority of Emotion users use React, there are some users who don't use React and we have to figure out how to cater for them.

Non-React-like libraries

Since all the packages on the @emotion scope are very modular, we can build the current emotion APIs with the packages on the @emotion scope. This should be pretty simple.

Preact

There are only a few APIs that we use that are new.

Fragment

Fragments are only used for SSR so we could have an alternative SSR API for that similar to extractCritical now.

createContext

We could tell people to use create-react-context and essentially polyfill createContext.

forwardRef

We could conditionally use forwardRef so if it's not available then we don't use it. I'm not a huge fan of this because I think it would be confusing to tell people they should use ref when using React and innerRef when using Preact but I'm not totally sure yet.

The other option to all of this is to not support the new APIs on Preact and have Preact users use the current emotion API.

Migration

This is something I'm still thinking about and I've got some ideas about but nothing concrete yet. I'm thinking of migrating emotion.sh and seeing what's difficult, how to make incremental migration easy, what can be done automatically with codemods and what warnings would be useful to provide.

Removing extractStatic

extractStatic hasn't been something that we've encouraged for a long time so @tkh44 and I think it's time to remove it. For people who like static extraction, libraries like Linaria and css-literal-loader do static extraction much better than Emotion does so those people should use those libraries.

Only do transformations when emotion is imported

babel-plugin-emotion currently transforms anything with the names of emotion's exports. While this works it transforms things for other libraries (#626 and #344) so we should only transform . I want to do this in Emotion 10 since it could be a breaking change for some people.

TODO

  • Support source maps in @emotion
  • Try to make the keyframes API more convenient
  • Reorganize some internals
  • Add a babel plugin for hoisting objects in the css prop and adding a css call around the object since we can't use a babel macro.
  • Add an alternative SSR API to get around the caveats with rendering style elements
  • Add component selectors to @emotion/styled.macro
  • Add a dev warning when :first-child and selectors like it are used and recommend replacements because of problems when rendering style elements in SSR and those selectors targeting the style elements.
  • Move https://github.com/emotion-js/next into this repo
  • Decide on how versioning should work since there are lots of packages that will likely have breaking changes that don't affect most end-users so should we do major versions really often(probably not) or do independent versioning for each package and if so should we start at 1.0.0 or 10.0.0?
  • docs. docs. docs.
  • Add a deprecation notice for extractStatic in babel-plugin-emotion and add a notice to the docs (if anyone wants to submit a PR for this, that would be great!!)
  • Remove extractStatic
  • probably more stuff
  • Make it so the new snapshot serializer doesn't mutate the object that's passed in
@tkh44
Copy link
Member

tkh44 commented Apr 28, 2018

This looks great. I like this API quite a bit. Are there any downsides that we should address?

@emmatown
Copy link
Member Author

The main downside is that it'll be hard to get class names outside of render if you're using the new APIs, that's intentional though. Another smaller thing that I haven't really figured out yet is keyframes, currently in the new API, you use it like this

/** @jsx jsx */
import jsx from '@emotion/jsx'
import keyframes from '@emotion/keyframes'
import css from '@emotion/css'

const animation = keyframes(css`
  from {
    color: green;
  }
  to {
    color: hotpink;
  }
`)

const Comp = () => (
  <div
    css={[
      {
        animation: `1s ${animation.name}`
      },
      animation.styles
    ]}
  />
)

which is pretty inconvenient since you have to pass in the name and the styles, for string styles we could interpolate the animation object and insert the animation but for object styles we can't do that since we only have a string when we interpolate the object. Any thoughts on how we could solve this?

@Andarist
Copy link
Member

Another change that emotion@10 will bring is that styled.div and etc. will work without a babel plugin. The tag list will be included in @emotion/styled but if you use @emotion/styled.macro, styled.div will be replaced with styled('div') and @emotion/styled-base will be imported instead which doesn't include the tag list.

What is the argument behind this? IMHO it brings unnecessary config right from the start when one wants to use emotion in production (assuming everyone should do it as it's less bytes without the tag list).

We could conditionally use forwardRef so if it's not available then we don't use it. I'm not a huge fan of this because I think it would be confusing to tell people they should use ref when using React and innerRef when using Preact but I'm not totally sure yet.

IMHO consistency is better, so I'd vote for keeping innerRef. In the meantime it might be good to research what preact's plans are about those new React APIs.

extractStatic hasn't been something that we've encouraged for a long time so @tkh44 and I think it's time to remove it. For people who like static extraction, libraries like Linaria and css-literal-loader do static extraction much better than Emotion does so those people should use those libraries.

Even if the feature is not actively developed and in non-ideal state, I don't quite see why should it be removed. It surely has some audience, brings some benefits to the users and having it might encourage outside contributors to make it cover more cases.

@emmatown
Copy link
Member Author

emmatown commented May 1, 2018

Another change that emotion@10 will bring is that styled.div and etc. will work without a babel plugin. The tag list will be included in but if you use .macro, styled.div will be replaced with styled('div') and will be imported instead which doesn't include the tag list.

What is the argument behind this? IMHO it brings unnecessary config right from the start when one wants to use emotion in production (assuming everyone should do it as it's less bytes without the tag list).

I'm not sure if you're referring to using a babel macro or including the tag list by default so I'll talk about both

  • I added the tag list to make the API more consistent between using the babel plugin and not using it so that the only differences between using the babel plugin and not using it are performance/size optimizations and DX improvements
  • In terms of doing the optimization with a babel macro, it was mainly because macros will be supported in react-scripts@2 and because the API made the optimization really easy. I think it would be a good idea to do it with a babel plugin as well. What I'd like to do though is make a util that accepts a certain import source and a function that transforms it with the same API as a babel macro so we can reuse all the logic and we don't have problems like [babel-plugin-emotion] Rewrites calls to foreign styled #344 and Babel plugin should only do transformations when emotion is imported #626

extractStatic hasn't been something that we've encouraged for a long time so @tkh44 and I think it's time to remove it. For people who like static extraction, libraries like Linaria and css-literal-loader do static extraction much better than Emotion does so those people should use those libraries.

Even if the feature is not actively developed and in non-ideal state, I don't quite see why should it be removed. It surely has some audience, brings some benefits to the users and having it might encourage outside contributors to make it cover more cases.

It requires some runtime code for styled so removing it would mean styled would be slightly smaller. (I know the amount of code it uses isn't very big, it still helps though) While extractStatic may have some small audience, other libraries do a better job of it so there's no reason people should use emotion over those other libraries. Also, explaining it in the docs is hard because we have to essentially say "we have this thing, it doesn't work very well, we don't recommend it and we're not planning on improving it but it's still here".

If we did keep it and contributors improved it, it would still likely require you to follow certain rules to make it statically analyzable which would limit the power of css-in-js.

@Andarist
Copy link
Member

Andarist commented May 1, 2018

If we did keep it and contributors improved it, it would still likely require you to follow certain rules to make it statically analyzable which would limit the power of css-in-js.

That is true, emotion's (and other css-in-js) true power lies in dynamic per props & theme styling, but at the same time not everything is dynamic in styles and that static part can be extracted. Keeping it would allow perfecting algorithm some day and would combine benefits of static extraction & dynamic styling at the same time. Just my 2 cents though 😉

@ntkoso
Copy link

ntkoso commented May 15, 2018

Never liked injectGlobal api in any css-in-js library.
It's a great idea to move away from globals.
Especially with React Suspense and easy streaming on server side globals will be the most annoying thing ever.
As for babel-plugin, i was avoiding trying next version before you added it. Thank you.
With glamorous deprecation the main 2 css-in-js libraries for me are Styled Components and Emotion 👍

@greggb
Copy link
Member

greggb commented May 19, 2018

Really excited about the direction of this. Had a couple of questions while thinking about what I might need to update.

If the css prop will be the favored approach do you envision most components having a bunch of outer scope variables for these styles? I really enjoy the css prop pattern, but it messes with the readability of a complexly styled components. This will somewhat involve class naming for the variables. e.g.

const borderedBox = css`<styles>`;

const Box = props => <div css={borderedBox} ...<lots of props>><a bunch of nested components></div>

Another change that emotion@10 will bring is that styled.div and etc. will work without a babel plugin. The tag list will be included in @emotion/styled but if you use @emotion/styled.macro, styled.div will be replaced with styled('div') and @emotion/styled-base will be imported instead which doesn't include the tag list.

Does this means for optimal perf we'll need to use styled('div') or

if(isProd) {
import(@emotion/styled.macro)
} else {
import(@emotion/styled)
}
styled.div

The css prop works without the babel plugin because it's implemented as a custom createElement

Does this mean doubling the number of dom elements or is it just a React implementation of the element?

@emmatown
Copy link
Member Author

If the css prop will be the favored approach do you envision most components having a bunch of outer scope variables for these styles? I really enjoy the css prop pattern, but it messes with the readability of a complexly styled components. This will somewhat involve class naming for the variables. e.g.

Could you give an example of what you would write now for that case?

Does this means for optimal perf we'll need to use styled('div') or

It only means you'll always be able to use styled.div. If you use @emotion/styled without the babel plugin, you'll include the tag list. If you use @emotion/styled.macro which requires babel-plugin-macros which will be included in create-react-app 2 or you use the babel plugin, styled.div will be converted to styled('div') and @emotion/styled-base will be imported which doesn't include the tag list.

Does this mean doubling the number of dom elements or is it just a React implementation of the element?

I'm not sure if I totally understand what you mean but the only time when there will be more dom elements is when doing SSR and style elements get rendered. You can look at how it works here

@greggb
Copy link
Member

greggb commented May 20, 2018

Here's a meaty one:

const primaryStyle = css`
    background: ${colorMap.GreenDark};
    border: 1px solid ${colorMap.GreenDark};
    color: #fff;
    font-size: ${size.default};
    &:hover {
        background: #507b32;
        border-color: ${colorMap.GreenDark};
        color: #fff;
    }
    &:active {
        ${primaryActiveStyle};
    }
    &[disabled],
    &[disabled]:hover {
        background-color: ${colorMap.slateNeutral};
        border: 1px solid #e9ebec;
        box-shadow: -2px 4px 2px -4px ${colorMap.slateLight};
        color: ${colorMap.slateLight};
    }
`;

It only means you'll always be able to use styled.div.

Ok, so nothing changes if we're using the babel plugin already. 👍 My concern was that it would be more performant in dev to use the whitelist, but fewer bytes over the wire with the macro (hence a different import depending on env.)

I'm not sure if I totally understand what you mean but the only time when there will be more dom elements is when doing SSR
Add a dev warning when :first-child and selectors like it are used and recommend replacements because of problems when rendering style elements in SSR and those selectors targeting the style elements.

Cool, then this won't affect my project atm, but my concern was adding another dom node for every css prop used on the page. For large pages I'd imagine this could add overhead.

Losing :first-child is going to be a little confusing for some of my junior devs when we start server-rending. I wonder if the babel plugin could change :first-child to :first-of-type or first-child:not(<customElement class>)?

@emmatown
Copy link
Member Author

Here's a meaty one:

Sorry, I meant one that would be different with the css prop in regards to this comment. As in, how would the readability change from how it is now and how do you have to name more than in the past?

If the css prop will be the favored approach do you envision most components having a bunch of outer scope variables for these styles? I really enjoy the css prop pattern, but it messes with the readability of a complexly styled components. This will somewhat involve class naming for the variables. e.g.

Losing :first-child is going to be a little confusing for some of my junior devs when we start server-rending. I wonder if the babel plugin could change :first-child to :first-of-type or first-child:not()?

The reason that it isn't done automatically is that if we did it in the babel plugin there would be cases where it wouldn't work and if we did it at runtime it would mean checking every single selector which would be expensive.
Also, there's no direct replacement for :first-child that ignores style elements.

@greggb
Copy link
Member

greggb commented May 20, 2018

Most of the examples and docs so far use inline styles for the css prop. It seems like the goal is to move toward that and away from styled. If that's not true than nothing is really different, I was thinking of new users who would be writing long strings of styles within a component and not knowing there are alternatives.

I thought this might work $('div :first-child:not(style)'), but realized you can't actually select style 🤦‍♂️

@andywer
Copy link

andywer commented Jun 18, 2018

Hey guys! Quick question: Wouldn‘t it be cleaner to have a customized react-dom with a reconciler that handles the css prop natively instead of patching createElement?

Just a thought 😉
Wrote down some ideas about it here when Twitter folks mentioned this issue.

Btw, great stuff, though 😊

@Ailrun
Copy link
Member

Ailrun commented Jun 18, 2018

@andywer Thank you for your opinion! However, as far as I think, it will make emotion too strongly depending on reconciling mechanism of react. The element structure of react is not that big stuff, but reconciling mechanism, especially after React@16, is quite big and complex one...

@emmatown
Copy link
Member Author

emmatown commented Jun 18, 2018

@andywer Just to be clear, we're not patching createElement, we're providing a function that you explicitly use instead of createElement.

IMO, creating a custom reconciler would be so much more complex, require a lot of work(and maintenance over time since it would have to be changed every time that react-dom was changed) and wouldn't really have any significant advantages over a custom createElement. Also, what do you mean by "cleaner" and why is having a custom reconciler "cleaner"?

Also, having a custom reconciler would make it harder to share components since you would have to make people use the custom reconciler as well rather than just exporting a component and using it with react-dom. This is especially important because one of the goals with this was to make it possible to create components with styles that can be imported from npm and just work without any configuration including with server-side rendering.

@hedgepigdaniel
Copy link

hedgepigdaniel commented Jul 16, 2018

Add an alternative SSR API to get around the caveats with rendering style elements

Can you point me in the direction of what you mean by this?

I've done some experimenting with https://github.com/emotion-js/next and its worked an absolute treat. Feels really clean/simple, and as you said, SSR just works since the style tags come out as part of the standard rendered react component tree.

One good argument in favour of using things like the extractStatic option or Linaria (i.e. extracting style information into css files) is that it avoids the needs to send style information twice on the initial render (Once before/in the server rendered HTML to ensure that the page renders correctly the first time, and once again as part of the javascript so that it can continue to keep the styles up to date after React mounts). With https://github.com/faceyspacey/extract-css-chunks-webpack-plugin and associated packages you can avoid that duplication as long as you put the styles into CSS files somewhere in your build pipeline.

Is that something that is or could be addressed with this new approach of rendering style tags within react? This has got me thinking though - already with SSR you duplicate the HTML (the SSR HTML version and the javascript/React version). I guess this is just an extension of that.

@emmatown
Copy link
Member Author

@hedgepigdaniel The caveat I'm referring to is that :first-child and pseudo classes like it are unsafe because the style elements are rendered next to other elements so the pseudo classes could target the style elements instead of the element they're intended to be targeting.

The new approach doesn't change anything with regards to static extraction. I'm not very interested in static extraction right now, I personally find the trade offs not worth the benefits. I think that the only practical way to do static extraction without big trade offs is with something like prepack.

@hedgepigdaniel
Copy link

hedgepigdaniel commented Jul 16, 2018

Ah cool. Since the box is checked... where is the alternative SSR API?

If I do ReactDom.renderToString(<App />) with the latest @emotion/core, the style tags are throughout the DOM tree in the rendered HTML, not in <head />. It sounds like this new approach is a way of SSRing the app such that the styles are all in the <head />.

@emmatown
Copy link
Member Author

@hedgepigdaniel the alternative api is using the current emotion ssr apis with the compat cache.

You can see an example of it in the tests,
https://github.com/emotion-js/emotion/blob/master/next-packages/compat-cache/__tests__/server.js

@FezVrasta
Copy link
Member

How's going to work the custom createElement with CRA?

Do they support the /** @jsx jsx */ comment and we are going to use it on every single file, or there is some way to change that setting globally?

@Andarist
Copy link
Member

I highly doubt they disallow jsx pragma, so it should "just work" with the mentioned /** @jsx jsx */ comment. If you want to change it globally you'd have to use smth like creat-react-app-rewired.

@Ailrun
Copy link
Member

Ailrun commented Sep 28, 2018

Is there a way to disable pragma with babel-plugin-transform-react-jsx? As far as I know, there is no such way...

@lifeiscontent
Copy link
Contributor

lifeiscontent commented Nov 7, 2018

@mitchellhamilton so with this refactor it looks like I can't use the component as a selector somewhere else.

e.g. css({[Button]: {...someStyle}});

how would I do that?

@FezVrasta
Copy link
Member

css`
  ${Button} {
    color: red;
  }
`

is how it works with template literals, I'm not sure about object notation

@lifeiscontent
Copy link
Contributor

@FezVrasta it doesn't work with either in this case, probably due to something special that happens when you wrap a component using styled.div, etc.

@lifeiscontent
Copy link
Contributor

@mitchellhamilton how would you use useContext with a styled(Button)?

@JesseChrestler
Copy link

@mitchellhamilton does css in Emotion 10 become the same as a styled component at render time. From what i can see in the react tab it appears to be the case, can you confirm?

@emmatown
Copy link
Member Author

emmatown commented Nov 8, 2018

does css in Emotion 10 become the same as a styled component at render time. From what i can see in the react tab it appears to be the case, can you confirm?

What do you mean by become the same as a styled component at render time?. They have different implementations if that's what you're asking.

how would you use useContext with a styled(Button)?

After hooks are stable and #967 is in, you'll be able to use hooks like useContext inside of interpolations.

@JesseChrestler
Copy link

JesseChrestler commented Nov 8, 2018

@mitchellhamilton would
Example1: const MyRedBox = <div css={css({backgroundColor:'red'})}></div>
be the same as
Example2: const MyRedBox = styled.div({backgroundColor:'red'})
I know the styles are obviously the same. From what i could tell it seemed like Example1 was wrapping itself around the <div> tag that it was assigned to.
image

This must be part of the babel transforms that rewrite the output. This changes a lot of things and the expectation around the component and how they are being used. Maybe I'm wrong so i'm just trying to verify my thoughts. Thanks.

@emmatown
Copy link
Member Author

emmatown commented Nov 8, 2018

It's wrapping the output but the examples are different, the first one is creating an element, the second is creating a component.

This changes a lot of things and the expectation around the component and how they are being used.

such as?

@JesseChrestler
Copy link

JesseChrestler commented Nov 9, 2018

such as?

The css prop becomes magical higher order components that are attaching to the ref of your element to add a class name. It seems like a lot of work just to add a class name. Is this so css will work with react-native, and other devices that don't support style sheets? What are the advantages to this approach (vs v9)

@emmatown
Copy link
Member Author

magical

Using magical to describe something is very unhelpful because it doesn’t really describe what’s wrong with something. Anything could be described as magical.

attaching to the ref of your element

We don’t use refs at all, refs are forwarded to the inner element.

What are the advantages to this approach (vs v9)

  • The css prop is a first-class citizen, not some hacky thing that breaks easily in certain cases + it works without the Babel plugin(especially great for create react app and codesandbox)
    • While this seems like a small thing, it completely changed how i use emotion, you don't have to think about classes or components, you have the smallest primitive in react, an element, and you can apply styles to it
  • Passing options like nonces, container elements, etc. via a Provider
    • this makes emotion much easier to use with iframes(while this seems like a small use, I think it could be really useful, for example, Netlify CMS shows previews in iframes) and if people want, web components
  • Zero configuration SSR, you don’t have to configure SSR at all, use react’s renderToString or renderToNodeStream and nothing else
    • The streaming performance will also be much better than the old implementation, this will be especially important when suspense SSR is out since a LOT more people will be doing streaming
    • This is especially great for component libraries since consumers don’t need to know about emotion to do ssr

Also, something that’s important to note, we’re not getting of the old APIs, if you want to keep using them, you still can, you just won’t get the advantages of the new APIs.

@lifeiscontent
Copy link
Contributor

lifeiscontent commented Nov 21, 2018

@mitchellhamilton looks like there was an option to set displayName for a styled component in the babel plugin for emotion, how would you achieve the same thing in emotion@10? Is the babel plugin still required?

@emmatown
Copy link
Member Author

Yes, the babel plugin is still required for labels(except with the css prop with function components). It's enabled by default in the v10 babel plugin.

@nonewcode
Copy link

Does this work with react-native and are there any performance concerns?

@slikts
Copy link

slikts commented Dec 17, 2018

You can literally just write a random jsx; statement anywhere in the file. Linters hate it!

Is there a workaround to having to add jsx? I'm using CRA with TS and the @jsx pragma and the unused jsx statement make for a pretty ugly look. I suppose it's related to: babel/babel#8958

@tkh44
Copy link
Member

tkh44 commented Dec 17, 2018

Unfortunately, because CRA does not allow babel configuration, you cannot update the pragma via @emotion/babel-preset-css-prop.

@jgoux
Copy link

jgoux commented Dec 18, 2018

@slikts For CRA v1 you can use https://github.com/timarney/react-app-rewired to extend the config without ejecting. For the v2 there is https://github.com/sharegate/craco which I use and it works great! 👍
I'm not using TS so I'm not sure about the compatibility.

@slikts
Copy link

slikts commented Dec 18, 2018

I added craco and babel-plugin-emotion and put this in craco.config.js:

module.exports = function({ env, paths }) {
  return {
    babel: {
      plugins: ["babel-plugin-emotion"],
    },
  };
};

Also: plugins: ["emotion"],

Then ran craco start, with no effect, --verbose just says craco: Added Babel plugins., and removing the jsx statement still causes ReferenceError: jsx is not defined.

I suppose there were reasons for these changes, but from my perspective emotion has taken a big step backward; it used to take just importing the template tag, but now I need to add 3 additional lines of noise: the babel pragma, the unused jsx import, and also an unused jsx statement.

@jgoux
Copy link

jgoux commented Dec 18, 2018

@slikts I don't think the @emotion/babel-preset-css-prop preset is included in babel-plugin-emotion.
See here : https://emotion.sh/docs/css-prop#babel-preset

@slikts
Copy link

slikts commented Dec 18, 2018

I installed @emotion/babel-preset-css-prop and added presets: ["@emotion/babel-preset-css-prop"], to craco.config.js, and now it claims to have applied the presets, but the only configuration that works is still with the 4 lines.

I liked emotion because it was minimal and elegant and allowed me to write CSS instead of object literals. Now it's Babel pragmas, unused imports, incompatibility with CRA/TS, and object literals in the main examples. It should have at least been released as -beta or -rc in the current state.

@tkh44
Copy link
Member

tkh44 commented Dec 18, 2018

@slikts while I agree this takes more work, you can still use import { css } from 'emotion' or import styled from '@emotion/styled' just like you have been before. You do not have to use the css prop.

import styled from '@emotion/styled'

const Button = styled.button`
  color: turquoise;
`

Now it's Babel pragmas, unused imports, incompatibility with CRA/TS, and object literals in the main examples.

We've always supported both objects and strings, still do. I don't know where you got this idea.

@slikts
Copy link

slikts commented Dec 18, 2018

I didn't say they're unsupported, just that the examples in the readme and elsewhere used to use template literals. It would be just:

import { css } from "emotion";

render(<div className={css`
  color: red;
`} />, root);

This was what I found elegant and why I used emotion; using CSS syntax, and having a clear model that, for example, you wouldn't have to explain to a colleague if they read your code. In contrast, the changes with pragmas and seemingly unused imports are hacky and fiddly. They're using a feature of Babel that I can't even find where it's documented, and I had to piece together what it is from a random blog post. The emotion readme just drops it there as if you'd be supposed to be familiar with it. Then, to actually use CSS syntax, I have to add an another import. I basically wouldn't even try to sell the new pattern to someone else who's possibly skeptical of CSS-in-JS as such, because it doesn't look good, requires an explanation, and even with an explanation it's like magic.

@tkh44
Copy link
Member

tkh44 commented Dec 18, 2018

This was what I found elegant and why I used emotion; using CSS syntax, and having a clear model that, for example, you wouldn't have to explain to a colleague if they read your code.

Your example still works.

In contrast, the changes with pragmas and seemingly unused imports are hacky and fiddly. They're using a feature of Babel that I can't even find where it's documented, and I had to piece together what it is from a random blog post. The emotion readme just drops it there as if you'd be supposed to be familiar with it.

While it needs to be better documented, you do not have to use the css prop if you don't want it. https://emotion.sh/docs/css-prop is the only page in the documentation that shows object styles before string styles simply because that is what I prefer to use and it does not require and additional import. In order to use emotion you've always had to import the css function just as you showed in your example. Using objects with the css prop allows you to write styles without imports.

@slikts
Copy link

slikts commented Dec 18, 2018

Your example still works.

It doesn't, the css tag function returns an object, and I'd have to know to use the vanilla package, which anyone familiarizing themselves with the library would probably miss and just see the css prop and Babel pragmas. The very first example in the readme also uses object literals.

@akullpp
Copy link

akullpp commented Dec 4, 2019

I basically wouldn't even try to sell the new pattern to someone else who's possibly skeptical of CSS-in-JS as such, because it doesn't look good, requires an explanation, and even with an explanation it's like magic.

This is a real issue, it is unacceptable to use the pragma as it seems hacky compared to styled-components. I won't introduce emotion because of this issue in a project...

@Andarist
Copy link
Member

Andarist commented Dec 4, 2019

The good thing is that no one is forcing you to do that. You also have @emotion/styled API for grabs if you prefer using styled-components-like API.

We just don't believe that jsx pragma is a hack - you are free to feel to think differently.

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