proposal: styled-component api but objects not strings

[alex-wilmer]

I've been using glamor. it's great. styled-components is a thing and it's cool but all our styles are already objects.

I've implemented something that's working quite nicely for us--would something like this be cool for glamor proper?

impl: https://github.com/NCI-GDC/portal-ui/blob/relay/modules/node_modules/%40ncigdc/theme/styled.js

usage: https://github.com/NCI-GDC/portal-ui/blob/relay/modules/node_modules/%40ncigdc/components/ExploringLinks.js#L12

this works extra well for us since styled components will always have access to theme on props,. I used to have css and withTheme all over the place, but this api is nicer.

[alex-wilmer]

While I'm here, perhaps I should ask if you see any foreseeable problems with this approach.

[antoinerey]

I'm also in favor of the styled-component api for glamor. I recently draft a custom helper, pretty much like @alex-wilmer one.

IMO, the styled-component api enforces composition, and greatly divides presentational components from logical component.

[alex-wilmer]

@antoinerey yea your solution is similar to mine, but you're checking whether styles is a function, where I'm checking if the value is a function. I think that's what styled components does and seems to make sense to me

let C = styled.div({
  height: '10px',
  color: props => props.primary ? props.theme.primary : 'blue'
})

// it's also overloaded to take any el

let C = styled(Custom, styles)

what do you think?

[antoinerey]

Should calling only one function be faster than calling multiple ones ? I'm not sure about this, styled-component should have a good explanation of why they chose that way.

Also, with only one function, it's easier to compute multiple props at once.

[alex-wilmer]

I think it's because otherwise independent style fragments all need to be functions

let containerStyle = {
  color: 'red',
  height: props.height, // props is undefined
}

// must change to 

let containerStyle = props => ({
  color: 'red',
  height: props.height
})

// now if I want to spread this as a fragment

let betterContainerStyle = props => ({
  border: '1px solid black'
  ...containerStyle(props),
)}

I would be surprised if values as functions are significantly less performant if at all, though an extra recursion check needs to happen for nested objects like ':hover': {}. However values as functions is a nicer api. I don't want to have to change the surrounding object to a function just because I decided to update color to take a prop.

[alex-wilmer]

And concerning computing props.. I suspect this is better done higher up in the component tree then in the styled component

[antoinerey]

@alex-wilmer You made a point using styles fragments. 😄

[antoinerey]

@alex-wilmer How do you handle breakpoints ?

Using a helper like yours (or mine) allows to abstract away the theme variables such as colors, zIndexes, margins ... and breakpoints. I actually abstracted it away using glamor/react vars method, and it's a pleasure to only import one file (the helper) instead of multiple ones (helper + theme + ...)

I also switched to the styled-components API with multiple functions, instead of one big wrapping the whole (like I previously used). I like it, that's effectively cleaner.

const Container = styled('div', {
  background: props => props.background || props.vars.background, // works very well
})

But, how do you handle breakpoints ? Without importing the theme file all over the places.

const Container = styled('div', {
  background: props => props.background || props.vars.background,

  [ ??? ]: { // There is no way to access props.vars here
    background: 'peachpuff',
  },
})

So, I came with that solution : Accept that the whole object can also be a function, passing props.

const Container = styled('div', (props) => ({
  background: props => props.background || props.vars.background,

  [props.vars.breakpoint.desktop]: { // I can use theme variables like I wanted
    background: 'peachpuff',
  },
}))

My solution works well for the moment, but I was wondering if you had any other way to solve this problem ? 😇

[kentcdodds]

Oh, was just driving by and noticed this. Thought ya'll may be interested in glamorous 😄

[threepointone]

glamorous is the recommend way ahead. closing this issue.