Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Streamline using dynamic values inside styled() #743

Closed
IgnusG opened this issue Apr 27, 2022 · 3 comments
Closed

Streamline using dynamic values inside styled() #743

IgnusG opened this issue Apr 27, 2022 · 3 comments

Comments

@IgnusG
Copy link

IgnusG commented Apr 27, 2022

I was looking for a way to have styled() components but with dynamic values inside their css -> for examle MUI's theme utilities:

import { useTheme } from '@mui/material';

// Where to place this?
const { spacing } = useTheme();

const MyButton = styled('button')`
  padding-inline: ${spacing(2)}
`

I saw that it's currently possible to "process" custom props in the attrs callback, so I though that's cool, what if we use that to get our dynamic props and pass the css prop that we generate with astroturf:

import { useTheme } from '@mui/material';

// Proposed use
const MyButton = styled('button').attrs(props => {
  const { spacing } = useTheme();

  return {
    // Astroturf complains about incorrect css usage
    css: css`
      padding-inline: ${spacing(2)}
    `
  }
})`
  background-color: blue;
` // Astroturf complains that there are 2 css props defined

Now this doesn't work (least because of the validation check that looks if we do use css correctly. I've played around with it a bit (disabling checks here and there, messing around in node_modules), but so far without luck - maybe I'm missed something (or this goes against how the library works right now).

You can of course do something more traditional like but this adds overhead in terms of typing the props and we either have to write every component like this (even ones without dynamic CSS properties, which adds a lot of extra bloat like in MyButtonStatic) or we'd have to rewrite the static component the moment we add a dynamic property and back if we remove them to keep everything lean.

// Here we have to use this otherwise we cannot interpolate the spacing from MUI
const MyButton = (props: JSX.IntrinsicElements['div']) => {
  const { spacing } = useTheme();
  return (
    <div
      {...props}
      css={css`
        padding-inline: ${spacing(2)};
      `}
    />
  );
};

// This is a lot of bloat compared to
// const MyButtonStatic = styled('div')`
//   padding-inline: 2em;
// `
const MyButtonStatic = (props: JSX.IntrinsicElements['div']) => {
  return (
    <div
      {...props}
      css={css`
        padding-inline: 2em;
      `}
    />
  );
};

Anyway, I wanted to see if this might be something to that would find its place in astroturf 🙂

@IgnusG
Copy link
Author

IgnusG commented Apr 27, 2022

You can also do this, which allows us to use dynamic properties together with static ones but it is a lot of boilerplate and you have to come up with CSS variable names:

const MyButton= styled('div').attrs(({ style = {}, ...props }) => {
  const { spacing } = useTheme();

  return {
    ...props,
    style: {
      ...style,
      '--x-spacing-2': spacing(2),
    },
  };
})`
  padding-inline: var(--x-spacing-2);
  background-color: blue;
`;

@IgnusG
Copy link
Author

IgnusG commented Apr 28, 2022

Another Idea I played around with would be similar to #730 so I'm not sure whether this wouldn't complicate things too much:

const MyButton= styled('div')`
  ${() => {
    const { spacing } = useTheme();

    return `
      padding-inline: ${spacing(2)};
    `;
  }}

 background-color: blue;
`;

The benefit would be that we don't have two sources of styles (additional css call) and it's clearer how the user can get dynamic styles into their styled components in an easy to follow way.

Or to make the VS Code extension work and make the style extraction easier:

const MyButton= styled('div')`
  ${() => {
    const { spacing } = useTheme();

    return css`
      padding-inline: ${spacing(2)};
    `;
  }}

 background-color: blue;
`;

@jquense
Copy link
Contributor

jquense commented Apr 29, 2022

hey thanks for the suggestions. Some of these are feasible but complex, and in general i've come to the opinion that the styled API is generally worse than using the css prop. It's slower, harder to type, and it's overall composition model doesn't work well with the static constraints of astroturf. I still thinks valuable for simple components, but one you are into dynamic values based on props i would say it's worth it to use the css prop instead.

@IgnusG IgnusG closed this as not planned Won't fix, can't repro, duplicate, stale Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants