Set styled object as a whole based on props

RickEyre commented 6 years ago

Is it possible / would you accept a pull request that could accomplish something of the following:

const CaseOne = {
  color: red
};

const CaseTwo = {
  color: blue
};

const MyStyledComponent = styled('button')(
  props => props.caseOne ? CaseOne : CaseTwo
);

I know currently you can set CSS properties individually based on props, but the library doesn't support replacing the whole styled object based on props.

This is a trivial example case, but I've had a number of use cases using this library where the styled objects were so different that I'd rather just swap them in and out as a whole.

I've also had a few cases where the styled objects are coming from different node modules and it's not sensible to override them one by one; I also don't want to know what the styled object looks like because it's coming from a different node module.

lttb commented 6 years ago

hi @RickEyre

Thanks for this suggestion!

But I see some problems with this feature, and the most meaningful problem for me is possible performance downgrade for this case. So let me explain how styled-jss works now.

When we mount styled component created with styled(tag)(styles) for the first time ,jss parses styles object and creates the Rule from it, that will be used in CSSOM. So in this processjss also handles function values, applies different jss-plugins and so on. And it's quite expensive at all. But we've got many optimizations later, when we reuse or update this component, like:

styled-jss reuse this calculations, and it does not parse styles again
function values are really fast on update

That's why styled-jss is one of the fastest cssinjs solutions on rerender without inline-styles. And of course, we're trying to improve our first-mount time too.

Let's take a look at this example:

const cases = {
  one: {color: 'red'},
  two: {color: 'glue'}
}

const StyledComponent = styled('button')({
  color: props => cases[props.type].color
})

// ...

export default (props) => (
  <div>
    <StyledComponent type="one">button 1</StyledComponent>
    <StyledComponent type="two">button 2</StyledComponent>
    <StyledComponent type={props.something ? 'one' : 'two'}>button</StyledComponent>
  </div>
)

So on each rerender and mount we will just recalculate the dynamic value of created StyledComponent Rule with function color: props => cases[props.type] for each instance, and it will be cheap and fast.

And now let's check an example with suggested behavior:

const CaseOne = {
  color: red
};

const CaseTwo = {
  color: blue
};

const MyStyledComponent = styled('button')(
  props => props.caseOne ? CaseOne : CaseTwo
);

To make this thing work, we need to recalculate the whole styles object for each render and mount, that means that we should parse styles object and fully update or create the Rule every time again. It seems that it may cause some performance problems.

We will try to improve our first-mount time and make styled-jss faster at all, but nowadays, unfortunately, I don't see the common effective solution for such behavior.

Probably we may resolve this problem with shallow styles equals and smart Rule updates by diff, but it's necessary to investigate.

Maybe you have some suggestions here?

zerobias commented 6 years ago

Currently, I made this via recompose branch, because we can create static subcomponents, which represents our cases

import { branch, renderComponent } from 'recompose'

const RadioBase = styled('input')({
  content: 'some our style'
})

const RadioActive = styled(RadioBase)({
  cursor: 'pointer',
})

const RadioDisabled = styled(RadioBase)({
  cursor: 'not-allowed',
})

const Radio = branch(
  ({ disabled }) => !!disabled,
  renderComponent(RadioDisabled),
  // renderComponent means that we just render RadioDisabled
)(RadioActive)

I think that its possible to declare one united style (as in Styled) with cases referenced by choose functions. But I don't want to mess with completely untyped composes: '$baseButton', prefer to returns components directly, so it can look something like that

const Base = styled('input')({
  content: 'some our style'
})

const Result = styledSwitch({
  active: {
    cursor: 'pointer',
  },
  disabled: {
    cursor: 'not-allowed',
  },
}, (props, cases) => props.active
  ? cases.active  // Note that we switch between components, not raw props
  : cases.disabled
)( Base )

zerobias commented 6 years ago

Oops, noted that it still doesn't solve issue yet, so I hope at least it yields some good thoughts