[RFC] Add support for the css prop #172

mxstbr · 2018-10-18T08:01:12Z

This integrates babel-plugin-css-prop by @satya164 (shoutout to him for being a genius) into our own Babel plugin, thus making the css prop work for all existing users!

Click here to read more about how this works under the hood

The way this works is incredible, requires 0 changes to core. This is the crux:

const StaticString = props => <p css="flex: 1;">A</p>;

const CustomComp = props => <Comp css="flex: 1;">A</Comp>

// is turned into 👇👇👇👇👇

const _CSSP = styled.p`flex: 1;`;

const StaticString = props => <_CSSP>A</_CSSP>

const _CSSP1 = styled(Comp)`flex: 1;`;

const CustomComp = props => <_CSSP1>A</_CSSP1>;

I've also moved the entire test suite into this repo, check that out to see some of the edge cases (inc interpolations, dynamic interpolations, global interpolations,...) this covers

The big question is: do we want to have native support for the css prop?

/cc the core team (@kitten @probablyup @imbhargav5) and interested onlookers (@AndrewIngram @jxnblk).

Todos before shipping:

Write docs

Potential future optimisations, probably not necessary for v1:

Optimize object style usage (<p css={{ color: 'blue' }} />) to not use a dynamic prop
Optimize css helper usage (<p css={css\color: blue;`} />`) to not use a dynamic prop

mxstbr · 2018-10-18T08:07:33Z

I'd personally vote for yes, for the following reasons:

It's very useful in the early stages of a project, when it's not entirely clear where the boundaries of components should go. Our current API is amazing once you know where the abstraction should be, but doesn't allow you to move as quick as the css prop. To quote @AndrewIngram from here: "I find that inline methods are better when you haven't settled on the abstractions yet, it's common for me to start with large mega components and clean things up as I understand the problems (and therefore appropriate abstractions) better."
Lots of people use styled-system and/or rebass already to hack around our missing support for it, but the downside of the user-land approach is that you have to use the custom component everywhere to be able to use the prop. This allows you to put it on any JSX node
There is a large demand from users for this API, as other libraries have shown—I've often heard it's a big reason people choose another library over s-c. (see here, here and here)

What do you think?

@satya164

/cc @satya164 does this look somewhat legit? Maybe useful for babel-plugin-css-prop too

AndrewIngram · 2018-10-18T10:24:30Z

A couple of areas where the babel plugin deviates from Emotion's CSS prop (as far as I can tell, i'm looking at the readme rather than the code):

Transparently creates extra components, which is non-obvious but creates minor overhead (this problem would go away with native support)
Doesn't seem to support arrays, eg css={[margin-left: 1px;, margin-right: 1px]), this is mainly useful for composing style primitives together.

mxstbr · 2018-10-18T10:46:12Z

Doesn't seem to support arrays, eg css={[margin-left: 1px;,margin-right: 1px]), this is mainly useful for composing style primitives together.

I'm not sure about this, it would essentially transpile down to

const Bla = styled.div`
  ${['margin-left: 1px']}
`

which I think would work?

jxnblk · 2018-10-18T14:32:26Z

Personally, I really like having the css prop as an escape hatch to handle one-off style changes with a more React-like API; it helps abstract the CSS-in-JS library away as an implementation detail. I also like how Styled Components and Emotion have an almost 1:1 API, and adding this to reduce the differences between the two libraries seems like an obvious move.

That said, I'm not really a fan of syntax being too tightly coupled with build tools. It's similar to why I don't recommend using things like CSS Modules: because the syntax you write only works when you're using webpack and the CSS Modules loader. This makes the code a little less portable. For the css prop with Babel, it's a little different, but any JSX that relies on the css prop will need to have this plugin to work properly, whereas this can be done without any extra build configuration using regular React components.

But, that's just my two cents, and I generally think this would be a great addition 👍

johnnysprinkles · 2018-11-07T20:10:17Z

This is great. Only thing it doesn't cover is the scenario where you both a) don't want to create an extra component and b) you want to apply the same CSS to multiple repeated elements. That's a scenario that emotion handles wells, with css tagged template literal declared outside the JSX and used as many places as you need.

That's ok though, can't have everything! Might be possible to add later if there's enough demand.

quantizor · 2018-11-07T20:11:22Z

withComponent hasn’t been removed yet, or you can use the new “as” prop

…

On Wed, Nov 7, 2018 at 2:10 PM johnnysprinkles ***@***.***> wrote: This is great. Only thing it doesn't cover is the scenario where you both a) don't want to create an extra component and b) you want to apply the same CSS to multiple repeated elements. That's a scenario that emotion handles wells, with css tagged template literal declared outside the JSX and used as many places as you need. That's ok though, can't have everything! Might be possible to add later if there's enough demand. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#172 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAiy1kGOulkO4gKgyqes4Pl2o7gAQJ-Gks5usz4qgaJpZM4Xs4DD> .

mxstbr · 2018-11-07T20:42:58Z

@johnnysprinkles This doesn't work?

const bla = css``;

<div css={bla}>
<span css={bla}>

If not we should defo make that work, /cc @satya164

satya164 · 2018-11-07T21:03:37Z

@mxstbr it should work. let's add a test for it

johnnysprinkles · 2018-11-07T22:17:41Z

Oh cool, I didn't see any mention of using it that way in the new docs at styled-components/styled-components-website@9b6008a Would the import in that case be import { css } from styled-components? And the css prop can take either a string as documented or whatever type the css tagged template literal returns?

mxstbr · 2018-11-08T08:00:49Z

Yep @johnnysprinkles!

mxstbr · 2018-11-15T12:09:13Z

This doesn't correctly add the styled import. Even code as simple as this throws a "styled is undefined" error:

// @flow
import React from 'react';
import Card from '../../shared/components/Card';
import config from '../../../config';

export default () => (
  <div
    css={`
      width: 35em;
    `}
  >
    <Card>
      <h1>Login or Sign Up</h1>
      <p>
        <a href={config.API_URI + '/auth/google'}>
          Sign up or login with Google
        </a>
      </p>
    </Card>
  </div>
);

Any ideas why @satya164?

satya164 · 2018-11-15T12:32:58Z

I'll take a look asap

mxstbr · 2018-11-16T09:29:58Z

Just hit another bug:

TypeError: Cannot read property 'replace' of undefined
  at /node_modules/babel-plugin-styled-components/lib/visitors/transpileCssProp.js:36:75

With this code:

// @flow
import React from 'react';
import styled from 'styled-components';
import Card from '../../shared/components/Card';
import Link from '../../shared/components/Link';
import { Button } from '../../shared/components/Button';
import config from '../../../config';

export default () => (
  <div
    css={`
      width: 100%;
      height: 100%;
      display: flex;
      align-items: center;
      justify-content: center;
    `}
  >
    <div
      css={`
        max-width: 35em;
        width: 100%;
        margin: 0 auto;
      `}
    >
      <Card>
        <h1 css="margin-top: 0;">Sign Up or Log In</h1>
        <Button.Ghost
          css="display: block;"
          href={config.API_URI + '/auth/google'}
        >
          Authenticate with Google
        </Button.Ghost>
        <p css="margin-bottom: 0;">
          Your information is private and secure. By signing up, you agree to the{' '}
          <Link href="/terms">Terms of Service</Link> and{' '}
          <Link href="/privacy">Privacy Policy</Link>.
        </p>
      </Card>
    </div>
  </div>
);

Specifically, it seems to be caused by the <Button.Ghost css="" /> thing.

kopax · 2018-11-16T11:36:23Z

Hi and thanks for the amazing work you've done so far.

May i know if the css props has a specification somewhere ? I'd like to learn about it.

If this is a new API for adding css to components, it will reduce increase the code quality to css around our main component. It will reduce the amount of template literal written around it which is good thing as code will look nicer.

May I know how this will handle the props (generally used for adding logic to the css computation) and the withTheme (user for adding logic to the js/css using theme values.)

Thanks in advance, this feature is very exciting, is there a target release date yet?

mxstbr · 2018-11-16T11:41:03Z

May i know if the css props has a specification somewhere ? I'd like to learn about it.
If this is a new API for adding css to components, it will reduce the amount of tuning we need to do and reduce the amount of template literal which is good and code will look nicer.

The specification is: put whatever you want in the css prop and we'll turn it into a styled component 😉

May I know how this will handle the props (generally used for adding logic to the css computation) and the withTheme (user for adding logic to the js/css using theme values.

See the test suite in this PR for more in-depth examples, but it handles props and variables like any other styled component, with the added benefit of static interpolations:

const Bla = (props) => (
  <div css={css`
    border-radius: ${props.borderRadius};
    color: ${p => p.color};
    background: ${p => p.theme.background};
  `}
   color="blue"
 />
)

<Bla borderRadius="3px" />

I hope that makes sense!

No target release date yet, but you can test this already if you npm install --save babel-plugin-styled-components@experimental ! Note that it still has some rough edges, as my most recent comments illustrated.

satya164 · 2018-11-16T12:03:55Z

Thanks for finding the bugs @mxstbr. I'll take look over the weekend!

@satya164

/cc @satya164 does this look okay?

@satya164

/cc @satya164 another fix for the other issue, it now correctly adds `_interopRequireDefault()` around the require call, which should make it all work

mxstbr · 2018-11-24T10:04:08Z

@satya164 last two commits should fix those issues, mind giving them a double check? 🙏

mxstbr · 2018-11-24T12:37:28Z

Note: Should change the prefix of the generated display name to "Styled" rather than "CSS".

kopax · 2018-11-24T13:33:22Z

@mxstbr thanks a lot for the insight, we'll test it soon. I believe we can't use it yet for distributing a module right? What is the plugin exactly for? Does this require a plugin to be working or will styled be able to handle css props by itself?

mxstbr · 2018-11-24T15:34:34Z

. I believe we can't use it yet for distributing a module right?

You totally can, it just might be a bit buggy! npm install --save-dev babel-plugin-styled-components@experimental

What is the plugin exactly for? Does this require a plugin to be working or will styled be able to handle css props by itself?

It only requires the plugin at build-time, the users of the distributed module do not need to use the plugin. Great question!

src/visitors/transpileCssProp.js

Co-Authored-By: mxstbr <contact@mxstbr.com>

mxstbr · 2018-11-26T08:29:27Z

Going to ship this as per internal discussions!

satya164 · 2018-11-26T10:18:56Z

src/visitors/transpileCssProp.js

+  if (t.isJSXMemberExpression(node)) {
+    return `${getName(node.object, t)}.${node.property.name}`
+  }
+  throw path.buildCodeFrameError(


Oops! This function needs to accept path as an argument (or be defined inside the plugin closure to access path)

Can you submit a PR? Thanks! 🙏 (and let's hope nobody hits this anytime soon... 😉)

Will try to do it soon!

mxstbr added 4 commits October 18, 2018 09:36

Integrate (failing) tests from babel-plugin-css-prop

936eeba

Inherit from JSX syntax to make tests run

eac7e93

Integrate babel-plugin-css-prop, make tests pass

085dfcc

Refactor into separate file, add cssProp option

9c368e7

mxstbr requested review from quantizor, imbhargav5 and kitten October 18, 2018 08:01

mxstbr added 2 commits October 18, 2018 10:04

Add test for objec prop

d4ddb38

Add test for css helper prop support

77d324e

This comment has been minimized.

Sign in to view

mxstbr added 2 commits October 18, 2018 11:58

Optimize css prop usage with the css`` helper

44816f5

/cc @satya164 does this look somewhat legit? Maybe useful for babel-plugin-css-prop too

Give credit where credit is due

0f43e2e

This comment has been minimized.

Sign in to view

quantizor mentioned this pull request Oct 21, 2018

Feature request: Aphrodite-style classnames instead of wrapping styled-components/styled-components#2132

Closed

mxstbr mentioned this pull request Oct 21, 2018

Add the css prop to the API documentation styled-components/styled-components-website#365

Merged

mxstbr added 3 commits November 9, 2018 11:42

1.7.2

02dbe95

Merge branch 'master' into css-prop

2c6eea2

1.9.0-0

b186a96

mxstbr added 2 commits November 24, 2018 10:56

Fix component tags with dots in them (<Button.Ghost />)

d483afc

/cc @satya164 does this look okay?

Fix auto-importing styled-components

a636162

/cc @satya164 another fix for the other issue, it now correctly adds `_interopRequireDefault()` around the require call, which should make it all work

1.9.0-1

e9c4ed6

Change generated component prefix from CSS to Styled

f2676b2

satya164 reviewed Nov 24, 2018

View reviewed changes

src/visitors/transpileCssProp.js Show resolved Hide resolved

satya164 reviewed Nov 24, 2018

View reviewed changes

src/visitors/transpileCssProp.js Outdated Show resolved Hide resolved

src/visitors/transpileCssProp.js Outdated Show resolved Hide resolved

satya164 and others added 4 commits November 24, 2018 18:44

Update src/visitors/transpileCssProp.js

b1b5281

Co-Authored-By: mxstbr <contact@mxstbr.com>

Update transpileCssProp.js

3a56c9e

Fix passing t through

7c3f927

Improve error message for getName edge case

a4ef270

mxstbr merged commit 74d0d58 into master Nov 26, 2018

mxstbr deleted the css-prop branch November 26, 2018 08:29

Jessidhia mentioned this pull request Nov 26, 2018

Add support to the css prop from babel-plugin-styled-components DefinitelyTyped/DefinitelyTyped#30827

Merged

6 tasks

satya164 reviewed Nov 26, 2018

View reviewed changes

jxnblk mentioned this pull request Nov 30, 2018

CSS prop removal rebassjs/rebass#540

Closed

shYkiSto mentioned this pull request Dec 13, 2018

Error "insertRule` accepts only strings" when using with styled-components vercel/styled-jsx#530

Closed

akiym mentioned this pull request Dec 19, 2018

Support css prop Igorbek/typescript-plugin-styled-components#29

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] Add support for the css prop #172

[RFC] Add support for the css prop #172

mxstbr commented Oct 18, 2018 •

edited

Loading

mxstbr commented Oct 18, 2018

This comment has been minimized.

This comment has been minimized.

AndrewIngram commented Oct 18, 2018

mxstbr commented Oct 18, 2018

jxnblk commented Oct 18, 2018

johnnysprinkles commented Nov 7, 2018

quantizor commented Nov 7, 2018 via email

mxstbr commented Nov 7, 2018

satya164 commented Nov 7, 2018

johnnysprinkles commented Nov 7, 2018 •

edited

Loading

mxstbr commented Nov 8, 2018

mxstbr commented Nov 15, 2018 •

edited

Loading

satya164 commented Nov 15, 2018

mxstbr commented Nov 16, 2018 •

edited

Loading

kopax commented Nov 16, 2018 •

edited

Loading

mxstbr commented Nov 16, 2018

satya164 commented Nov 16, 2018

mxstbr commented Nov 24, 2018

mxstbr commented Nov 24, 2018

kopax commented Nov 24, 2018 •

edited

Loading

mxstbr commented Nov 24, 2018 •

edited

Loading

mxstbr commented Nov 26, 2018

satya164 Nov 26, 2018

mxstbr Nov 26, 2018

satya164 Nov 26, 2018

[RFC] Add support for the css prop #172

[RFC] Add support for the css prop #172

Conversation

mxstbr commented Oct 18, 2018 • edited Loading

mxstbr commented Oct 18, 2018

This comment has been minimized.

This comment has been minimized.

AndrewIngram commented Oct 18, 2018

mxstbr commented Oct 18, 2018

jxnblk commented Oct 18, 2018

johnnysprinkles commented Nov 7, 2018

quantizor commented Nov 7, 2018 via email

mxstbr commented Nov 7, 2018

satya164 commented Nov 7, 2018

johnnysprinkles commented Nov 7, 2018 • edited Loading

mxstbr commented Nov 8, 2018

mxstbr commented Nov 15, 2018 • edited Loading

satya164 commented Nov 15, 2018

mxstbr commented Nov 16, 2018 • edited Loading

kopax commented Nov 16, 2018 • edited Loading

mxstbr commented Nov 16, 2018

satya164 commented Nov 16, 2018

mxstbr commented Nov 24, 2018

mxstbr commented Nov 24, 2018

kopax commented Nov 24, 2018 • edited Loading

mxstbr commented Nov 24, 2018 • edited Loading

mxstbr commented Nov 26, 2018

satya164 Nov 26, 2018

Choose a reason for hiding this comment

mxstbr Nov 26, 2018

Choose a reason for hiding this comment

satya164 Nov 26, 2018

Choose a reason for hiding this comment

mxstbr commented Oct 18, 2018 •

edited

Loading

johnnysprinkles commented Nov 7, 2018 •

edited

Loading

mxstbr commented Nov 15, 2018 •

edited

Loading

mxstbr commented Nov 16, 2018 •

edited

Loading

kopax commented Nov 16, 2018 •

edited

Loading

kopax commented Nov 24, 2018 •

edited

Loading

mxstbr commented Nov 24, 2018 •

edited

Loading