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

Embedding the result of the css function in a styled definition. #3212

Open
thany opened this issue Jul 16, 2024 · 1 comment
Open

Embedding the result of the css function in a styled definition. #3212

thany opened this issue Jul 16, 2024 · 1 comment

Comments

@thany
Copy link

thany commented Jul 16, 2024

The problem

The css function is great for constructing reusable mixins, but for that use-case, it's important that they can be embedded in a styled definition, which they can't. At least not as-is.

Proposed solution

I'm think something like so:

const orientationMixin = (vertical: boolean) => css`
  display: flex;
  ${vertical && `
    flex-direction: column;
  `}
`;

const List = styled.div(props => `
  ${orientationMixin(true)}
  // ...
`);

The vertical boolean switch, written like that, requires being tagged with the css function, otherwise it produces the literal string false where otherwise the flex-direction would appear. I don't want to rewrite such things to a ternary statement, because it is less readable, and should be unneccesary. So the css tag is more or less required.

Alternative solutions

Currently, I have to do this in order to make it work:

const List = styled.div(props => `
  ${orientationMixin(true).styles}
  // ...
`);

Note the added .styles.

Additional context

The reason I've marked this as a feature request, is because the styles property on the result of the css function, I could not find anything about in the documentation. So this is technically an undocumented feature, or basically a hack. And the problem is that I have no way of knowing whether the above alternative solution is correct, supported, and future-proof.

It seems to me like a simple improvement to allow embedding mixins like this, by simply not blindly assuming that everything mixed in, is a string, or is blindly converted to a string.

The documentation about css does mention how to do exactly what I'm asking, but in a slightly different way, as in the use of the css fucntion must always be used inside another function that recieves props. But those props won't magically contain specific switches that I need, that are defined elsewhere, outside props. It makes me believe that it's impossible (without the above hack) to just keep statically defined mixins around - they have to be functions.

@thany
Copy link
Author

thany commented Jul 16, 2024

Addendum, if I do this:

const orientationMixin = (props: { vertical: boolean }) => {
  console.log('yohoo');
  return css`
    display: flex;
    ${vertical && `
      flex-direction: column;
    `}
  `;
};

const List = styled.div(props => `
  ${orientationMixin}
  // ...
`);

The function never even gets called, the console log never appears. It doesn't even get the chance to output anything meaningful. But this is what the documentation suggests I do.

All the more reason to get this added as a feature, because clearly it's not supported, even though somehow the documentation suggests otherwise - or I'm misreading it, as it's hard to tell what's what without type annotations in the examples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant