Add sx prop for theme-aware ad-hoc styles

[BinaryMuse]

The sx prop is used by some UI libraries to allow users to apply inline, ad-hoc styles to a component while still making the styles theme-aware. I experimented a bit this afternoon and was able to come up with a proof-of-concept:

You can see the basic implementation in this PR, which is based around @styled-system/css, but it is essentially the following:

const withSx = Component => {
  return function SxWrapper({sx, children, ...props}) {
    const ctxTheme = useTheme()
    if (!sx) {
      return <Component {...props}>{children}</Component>
    }

    const extraProps = {}
    const compiled = css(sx)(props.theme || ctxTheme)
    if (props.style) {
      extraProps.style = Object.assign({}, props.style, compiled)
    } else {
      extraProps.style = compiled
    }

    return (
      <Component {...props} {...extraProps}>
        {children}
      </Component>
    )
  }
}

const Heading = WithSx(styled.h1`
// ... normal styled-component stuff here

While exploring this PoC and the implementations from other libraries, I've identified the following constraints and challenges:

Compatibility with plain DOM nodes

This implementation only works for components explicitly wrapped with the HoC, so it wouldn't be possible to do something like <span sx={{color: 'red.5'}}>...</span>. To enable that, we'd have to create a JSX pragma, similar to Theme UI, which would need to be used in all our components and any JS file a consumer writes that wants to use the sx prop on a plain DOM node. This pragma function would then generate the correct element tree to power the prop. This would of course need some JSX intrinsics types for TypeScript.

Another option is to introduce a hook that can produce style-prop-compatible objects with themeable values, à la useStyles({ color: 'red.5' }).

However, neither option addresses the following concern:

Nested styles

styled-systems's css utility, which powers this implementation, allows for more than just plain CSS properties in the sx prop object. For example, this is a valid value for the prop:

{
  fontSize: [ 4, 5, 6 ],
  color: 'primary',
  bg: 'gray',
  '&:hover': {
    color: 'secondary',
  },
}

However, since this generates style objects with media queries and selectors, it can't be passed directly to any DOM node.

One solution is to use styled-component's css prop; however, the use of this prop necessitates the use of the Babel plugin (or macro) to work, and as a result, I don't think we'd be able to implement the sx prop in Primer Components (because the Babel plugin won't run). Instead, a user could use the option that @styled-system/css demonstrates:

import React from 'react'
import css from '@styled-system/css'

// ...

<div
  css={css({
  fontSize: [ 4, 5, 6 ],
  color: 'primary',
  bg: 'gray',
  '&:hover': {
    color: 'secondary',
  },
})}
/>

Since the user would have to set up the Babel plugin anyway in this case, they could simply write the above code themselves, instead of us providing a special sx prop.

Part of me wonders if one could generate a styled-components-wrapped component on the fly using the sx prop, but my guess is no, because if so then why does styled-components require a Babel plugin for the css prop?

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/primer/primer-components/76xp9rlse
✅ Preview: https://primer-components-git-mkt-sx-prop.primer.now.sh

[colebemis]

@BinaryMuse What if we did something like this?

import styled from "styled-components";
import css from "@styled-system/css";
import {COMMON} from "./constants";

const sx = props => css(props.sx);

const Label = styled.div`
  background: red;
  ${COMMON};
  ${sx};
`;

export default function App() {
  return (
    <div>
      <Label sx={{ bg: "lightblue", "&:hover": { bg: "pink" } }}>Foo</Label>
    </div>
  );
}

We'd add the sx prop like we add other system props—in the styled component definition. This approach would also support pseudo selectors.

[BinaryMuse]

We'd add the sx prop like we add other system props—in the styled component definition. This approach would also support pseudo selectors.

Thank you, Cole; for some reason I didn't consider this, I think it's the ideal solution for that problem. I changed up the abstraction a bit and managed:

with only this change to the components:

diff --git a/src/BorderBox.js b/src/BorderBox.js
index 2cdde7b5..778fd593 100644
--- a/src/BorderBox.js
+++ b/src/BorderBox.js
@@ -1,10 +1,10 @@
-import styled from 'styled-components'
+import sx from './sx'
 import PropTypes from 'prop-types'
 import Box from './Box'
 import theme from './theme'
 import {BORDER} from './constants'
 
-const BorderBox = styled(Box)(BORDER)
+const BorderBox = sx.styled(Box)(BORDER)
 
 BorderBox.defaultProps = {
   theme,
diff --git a/src/Heading.js b/src/Heading.js
index f75ff14c..81705fd4 100644
--- a/src/Heading.js
+++ b/src/Heading.js
@@ -1,9 +1,9 @@
-import styled from 'styled-components'
+import sx from './sx'
 import PropTypes from 'prop-types'
 import {TYPOGRAPHY, COMMON, get} from './constants'
 import theme from './theme'
 
-const Heading = styled.h1`
+const Heading = sx.styled.h1`
   font-weight: ${get('fontWeights.bold')};
   font-size: ${get('fontSizes.5')};
   margin: 0;

Can you think of any downsides to this strategy?

[colebemis]

Oh, interesting. What are the benefits of sx.styled.div() over styled.div(sx)?

[BinaryMuse]

What are the benefits of sx.styled.div() over styled.div(sx)?

That only works if you don't want to define any additional styles, right? What about inside the library itself? styled(BaseComponent) with more styles tacked on is a common pattern; in those cases you'd have to remember to drop ${sx} into the styles themselves.

By wrapping styled we can export our own styled from the sx module and make all our components sx-enabled at once:

-import styled from 'styled-components'
+import styled from './sx-styled'

[colebemis]

styled(BaseComponent) with more styles tacked on is a common pattern; in those cases you'd have to remember to drop ${sx} into the styles themselves.

Isn't that what we already do with other system props?

const Heading = styled.h1`
  font-weight: ${get('fontWeights.bold')};
  font-size: ${get('fontSizes.5')};
  margin: 0;
  ${TYPOGRAPHY};
  ${COMMON};
+ ${sx};
`

I'm not sure with approach I prefer yet. I'm just trying to understand the differences. I'd love to hear @jxnblk's thoughts :)

[BinaryMuse]

I'm just trying to understand the differences.

Gotcha. I don't think there are any, as appending sxProps to the end of the tag function args is (I hope) the same as including it at the end of the tagged template literal.

One advantage of having to add it to the literal itself is that one is less likely to forget to add the appropriate prop types...

[jxnblk]

@colebemis most people tend to use the @theme-ui/css (note the @styled-system/css package is to be deprecated) package as you've shown in your example when using the styled HOC pattern. I suspect you could wrap it up into your COMMON props to make including it in each component easier.

[colebemis]

@jxnblk Oh, I like the idea of including sx with the COMMON props. Is there a good way to do that? I assume that we can't pass props => css(props.sx) to the compose() function.

[BinaryMuse]

@colebemis Despite what the docs say, not every component currently gets COMMON props, although we could fix this. Also, I believe sx would need to consistently go at the end of the style list. I think I would be more in favor of including it at the end automatically whenever creating a new component (as implemented in this PR or similar) or having it included by hand (like you originally proposed).

[colebemis]

@BinaryMuse Yeah, that makes sense. I’m fine with either. I think I prefer the more explicit approach of listing it the styled definition. I’m interested in @emplums thoughts on this too.

[BinaryMuse]

@emplums I've updated the CONTRIBUTING doc and also started on a document for the sx prop. I would highly value your input esp. on the latter if you have thoughts.

https://primer-components-git-mkt-sx-prop.primer.now.sh/components/overriding-styles

[colebemis]

@BinaryMuse The documentation looks good! It might be nice to also include an example of responsive values with the sx prop. For example, sx={{ borderWidth: [0, 1] }}

[emplums]

left a couple more notes on the documentation!

[BinaryMuse]

I believe I've addressed all the comments so far; please feel free to keep the feedback coming if you see anything else.

[colebemis]

Awesome, @BinaryMuse! 🎉 Thanks for going the extra mile on this (e.g. writing great documentation, streamlining tests). Ship it 🚢

Changes addressed