fix(css): improve built-in support with css prop as function (#181)

* chore: refactor getGlamorClassName

* fix(css): improve built-in support with css prop as function

**What**: This refactors things a bit and separates `css` the prop from
the overrides props.

**Why**: Otherwise, if the `css` prop happens to be a function, the
`propsAreCssOverrides` would make the props be assigned to the function
and then they'd never actually be applied. By splitting them from each
other it allows us to merge both of them.

**How**: Most of the work happened in `get-glamor-classname.js`. I first
refactored it to simplify things a bit. Then I plugged in the `cssProp`
and `cssOverrides` into their order of precidence. I also added a bit of
recursion to the `handleStyles` function which allows you to pass an
array of functions, this is useful for the `css` prop in particular. So
you can compose functions together without a `compose` helper.

src/__tests__/__snapshots__/index.js.snap

@@ -73,30 +73,30 @@ exports[`can use pre-built glamorous components with css attributes 1`] = `
 }
 
 <a
-  class="is added to classes css-1t62idy"
+  class="css-1t62idy is added to classes"
   href="/is-forwarded"
   id="is-forwarded"
 />
 `;
 
 exports[`can use pre-built glamorous components with css prop 1`] = `
-.css-1n96jtr,
-[data-css-1n96jtr] {
+.css-1d3tm0b,
+[data-css-1d3tm0b] {
   -webkit-flex-direction: column;
   -ms-flex-direction: column;
   -webkit-box-orient: vertical;
   -webkit-box-direction: normal;
-  background: blue;
   display: -webkit-box;
   display: -moz-box;
   display: -ms-flexbox;
   display: -webkit-flex;
   display: flex;
   flex-direction: column;
+  background: blue;
 }
 
 <a
-  class="is added to classes css-1n96jtr"
+  class="css-1d3tm0b is added to classes"
   href="/is-forwarded"
   id="is-forwarded"
 />
@@ -114,13 +114,15 @@ exports[`css prop with a className 1`] = `
 `;
 
 exports[`css prop with a function 1`] = `
-.css-1xnhj1e,
-[data-css-1xnhj1e] {
+.css-1ps4aij,
+[data-css-1ps4aij] {
+  font-size: 10px;
   padding: 20px;
+  margin: 30px;
 }
 
 <section
-  class="css-1xnhj1e"
+  class="css-1ps4aij"
 />
 `;
 
@@ -180,7 +182,7 @@ exports[`merges composed component styles for reasonable overrides 1`] = `
 }
 
 <div
-  class="other classes are not removed css-1k8eh41"
+  class="css-1k8eh41 other classes are not removed"
 />
 `;
 

src/__tests__/index.js

@@ -78,14 +78,21 @@ test('the css prop accepts "GlamorousStyles"', () => {
     render(<glamorous.Section css={className} />),
   ).toMatchSnapshotWithGlamor('css prop with a className')
 
-  const fn = jest.fn(() => ({padding: 20}))
-  const props = {css: fn, otherThing: 43, theme: {color: 'red'}}
+  const fn1 = jest.fn(() => ({padding: 20}))
+  const fn2 = jest.fn(() => ({margin: 30}))
+  const props = {css: [fn1, fn2], fontSize: 10, theme: {color: 'red'}}
   expect(render(<glamorous.Section {...props} />)).toMatchSnapshotWithGlamor(
     'css prop with a function',
   )
-  expect(fn).toHaveBeenCalledTimes(1)
+  expect(fn1).toHaveBeenCalledTimes(1)
+  expect(fn2).toHaveBeenCalledTimes(1)
   const context = {__glamorous__: undefined}
-  expect(fn).toHaveBeenCalledWith(
+  expect(fn1).toHaveBeenCalledWith(
+    expect.objectContaining(props),
+    expect.objectContaining(props.theme),
+    expect.objectContaining(context),
+  )
+  expect(fn2).toHaveBeenCalledWith(
     expect.objectContaining(props),
     expect.objectContaining(props.theme),
     expect.objectContaining(context),

src/create-glamorous.js

@@ -83,7 +83,7 @@ function createGlamorous(splitProps) {
           // readability to get better performance because it actually
           // matters.
           const props = this.props
-          const {toForward, cssOverrides} = splitProps(
+          const {toForward, cssOverrides, cssProp} = splitProps(
             props,
             GlamorousComponent,
           )
@@ -94,13 +94,14 @@ function createGlamorous(splitProps) {
             Object.freeze(this.state.theme)
 
           // create className to apply
-          const fullClassName = getGlamorClassName(
-            GlamorousComponent.styles,
+          const fullClassName = getGlamorClassName({
+            styles: GlamorousComponent.styles,
             props,
             cssOverrides,
+            cssProp,
             theme,
-            this.context,
-          )
+            context: this.context,
+          })
           const debugClassName = glamorous.config.useDisplayNameInClassName ?
             cleanClassname(GlamorousComponent.displayName) :
             ''

src/get-glamor-classname.js

@@ -25,31 +25,32 @@ function extractGlamorStyles(className = '') {
 
 export default getGlamorClassName
 
-function getGlamorClassName(styles, props, cssOverrides, theme, context) {
+function getGlamorClassName({
+  styles,
+  props,
+  cssOverrides,
+  cssProp,
+  theme,
+  context,
+}) {
+  const {
+    glamorStyles: parentGlamorStyles,
+    glamorlessClassName,
+  } = extractGlamorStyles(props.className)
   const {mappedArgs, nonGlamorClassNames} = handleStyles(
-    styles,
+    [...styles, parentGlamorStyles, cssOverrides, cssProp],
     props,
     theme,
     context,
   )
-  const {
-    mappedArgs: cssOverridesArgs,
-    nonGlamorClassNames: cssOverridesClassNames,
-  } = handleStyles([cssOverrides], props, theme, context)
-  const {
-    glamorStyles: parentGlamorStyles,
-    glamorlessClassName,
-  } = extractGlamorStyles(props.className)
-
-  const glamorClassName = css(
-    ...mappedArgs,
-    ...parentGlamorStyles,
-    ...cssOverridesArgs,
-  ).toString()
-  const extras = nonGlamorClassNames.concat(cssOverridesClassNames).join(' ')
-  return `${glamorlessClassName} ${glamorClassName} ${extras}`.trim()
+  const glamorClassName = css(...mappedArgs).toString()
+  const extras = [...nonGlamorClassNames, glamorlessClassName].join(' ').trim()
+  return `${glamorClassName} ${extras}`.trim()
 }
 
+// this next function is on a "hot" code-path
+// so it's pretty complex to make sure it's fast.
+// eslint-disable-next-line complexity
 function handleStyles(styles, props, theme, context) {
   let current
   const mappedArgs = []
@@ -65,6 +66,10 @@ function handleStyles(styles, props, theme, context) {
       }
     } else if (typeof current === 'string') {
       processStringClass(current, mappedArgs, nonGlamorClassNames)
+    } else if (Array.isArray(current)) {
+      const recursed = handleStyles(current, props, theme, context)
+      mappedArgs.push(...recursed.mappedArgs)
+      nonGlamorClassNames.push(...recursed.nonGlamorClassNames)
     } else {
       mappedArgs.push(current)
     }

src/split-props.js

@@ -2,7 +2,7 @@ import shouldForwardProperty from './should-forward-property'
 
 export default function splitProps(
   {
-    css: cssOverrides = {},
+    css: cssProp,
     // these are plucked off
     theme, // because they
     className, // should never
@@ -13,7 +13,7 @@ export default function splitProps(
   },
   {propsAreCssOverrides, rootEl, forwardProps},
 ) {
-  const returnValue = {toForward: {}, cssOverrides}
+  const returnValue = {toForward: {}, cssProp, cssOverrides: {}}
   if (!propsAreCssOverrides) {
     if (typeof rootEl !== 'string') {
       // if it's not a string, then we can forward everything

src/tiny.js

@@ -2,7 +2,7 @@
 import createGlamorous from './create-glamorous'
 
 function splitProps({
-  css: cssOverrides = {},
+  css: cssProp,
   // these are plucked off
   theme, // because they
   className, // should never
@@ -11,7 +11,7 @@ function splitProps({
   // component ever
   ...rest
 }) {
-  return {toForward: rest, cssOverrides}
+  return {toForward: rest, cssProp}
 }
 
 const glamorous = createGlamorous(splitProps)