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

Support the css prop with the babel macro #673

Closed
kentcdodds opened this issue May 25, 2018 · 8 comments
Closed

Support the css prop with the babel macro #673

kentcdodds opened this issue May 25, 2018 · 8 comments

Comments

@kentcdodds
Copy link
Contributor

  • emotion version: N/A
  • react version: N/A

Relevant code.

import React from 'react'
import ReactDOM from 'react-dom'
import 'emotion/macro'

const App = () => <div css={{':hover': {color: 'red'}}}>Hi</div>
ReactDOM.render(<App />, document.getElementById('root'))

What you did:

Tried to use the css prop without the babel plugin but with the macro

What happened:

An object was set as the css attribute of the <div> that was rendered rather than applying the styles.

Reproduction:

https://codesandbox.io/s/2xonnkmzpy

Problem description:

I know that the babel macro explicitly lists the css prop as NOT supported, but I was thinking about it and I'm pretty sure there's no reason it couldn't be.

Suggested solution:

What if we had something like this:

import React from 'react'
import ReactDOM from 'react-dom'
import {cssProp} from 'emotion/macro'

cssProp()

const App = () => <div css={{':hover': {color: 'red'}}}>Hi</div>
ReactDOM.render(<App />, document.getElementById('root'))

Then in the macro:

module.exports = createMacro(emotionMacro)

function emotionMacro({ references, state, babel }) {
  const programPath = references.cssProp[0].findParent(babel.types.isProgram)
  // traverse program path to apply css prop changes and stuff
}

Here's a start: https://astexplorer.net/#/gist/bf43182c1966a9bdbb00e2285f298727/5cd938716db45cf3ddad4fe49905507a090dbd2c

I would have done more, but it's the kiddo's graduation and I gotta run! What do y'all think!?

@Andarist
Copy link
Member

It seems to me that the whole premise of macros is to apply transforms more explicitly (at call sites only) and the proposed cssProp is rather implicit & targeting whole file.

What do you think about similar, but IMHO slightly more explicit version of this:

/** @jsx jsx */
import {jsx} from 'emotion/macro'

@kentcdodds
Copy link
Contributor Author

It seems to me that the whole premise of macros is to apply transforms more explicitly

Yes, this is a bit out of the spirit of a macro, but I really want the css prop 😅

If your example works, then I'd be good with that 👍

@Andarist
Copy link
Member

It would certainly be doable, WDYT @mitchellhamilton ?

@emmatown
Copy link
Member

I like the explicit version but it can't be implemented with babel-plugin-macros because babel-plugin-macros runs macros inside the visitor of where it's imported so the jsx hasn't been transformed by the time the macros run.

It would be great if we could change babel-plugin-macros so that it works but I'm not really sure of the best way to do it and how to do it without changing the API for macros.

Also, btw @kentcdodds, we're planning on making the css prop work everywhere by making it a custom jsx function anyway in emotion@10 (#637)

@kentcdodds
Copy link
Contributor Author

That will be satisfactory 👍 Thanks!

@Andarist
Copy link
Member

I had no idea about how macros work under the hood until yesterday. It's a bummer that it wouldn't catch transformed jsx 😢

@kentcdodds
Copy link
Contributor Author

Glad you know now!

@kentcdodds
Copy link
Contributor Author

Also, if you can think of a way to make it work that'd be neat!

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

3 participants