astroturf icon indicating copy to clipboard operation
astroturf copied to clipboard

[Feature Request] Adapting based on props

Open benjycui opened this issue 5 years ago • 5 comments

I think that we can support Adapting based on props by compiling it to style prop.

// input
const Color = styled('span')`
  color: ${(props) => props.color};
`;

// output
const Color = (props) => {
  return (
    <span className={...} style={{ color: props.color }}>
      {props.children}
    </span>
  );
}

benjycui avatar Oct 10 '18 03:10 benjycui

I don't really see this as being all that useful, given how limited this would be, and the additional complexity involved. This would be much more easily accomplished with something like withProps from recompose.

taion avatar Oct 10 '18 15:10 taion

I think ${(props) => props.color} is not really readable syntax. CSS should be declarative.

It is nice to have this feature. But not a high priority at least for me.

ai avatar Oct 10 '18 16:10 ai

It is just a simple case. If we can read variables from props, it means that we can use context to pass theme variables.

const Button = styled('button')`
  background-color: ${(props) => props.theme.color};
`;

<ThemeProvider>
  <Button />
</ThemeProvider>

benjycui avatar Oct 17 '18 03:10 benjycui

@benjycui theme properties on context is something that css-in-js libraries like emotion need to support because they don't allow for css preprocessor pipelines. Astroturf is intentionally taking a different approach, and deciding to lean on existing tooling. In 99% (in our experience) there isn't any additional value to storing theme variables in js. In most cases a theme.scss file of sass variables (or postcss values, etc) is sufficient and preferable to having to figure out how to convert the value from js to css.

In other words, we feel like variables in css are used the vast majority of the time only in css, and so it makes more sense to keep them in css, and occasionally convert to js instead of starting in a format that is the less common case.

jquense avatar Oct 17 '18 13:10 jquense

I think that we can support Adapting based on props by compiling it to style prop.

It'll be very limited. For example, you can't use this interpolation inside pseudo-selector or media queries. This also brings specificity issues because it won't possible to override this inline style easily. IMO you're better off using inline style directly for such simple stuff.

There are some other approaches you can use, like CSS variables or currentColor, where it makes sense.

satya164 avatar Oct 17 '18 22:10 satya164