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

Check the validity of the generated CSS #224

Open
Mad-Kat opened this issue Dec 3, 2024 · 3 comments
Open

Check the validity of the generated CSS #224

Mad-Kat opened this issue Dec 3, 2024 · 3 comments

Comments

@Mad-Kat
Copy link
Collaborator

Mad-Kat commented Dec 3, 2024

At the moment we don't perform any checks inside next-yak if the generated CSS is valid CSS. Until now we relied on the capabilities of Lightning CSS or PostCSS within next.js that do some form of validation.

However we noticed that certain cases like this:

import { styled } from "next-yak";

const MyDiv = styled.div`
  &:nth-child(${() => 2}) {
    color: red;
  }
`;

produce an incorrect CSS:

import { styled } from "next-yak/internal";
import __styleYak from "./index.yak.module.css!=!./index?./index.yak.module.css";
const MyDiv = /*YAK Extracted CSS:
.MyDiv {
  &:nth-child(var(--yrRH4Ik)) {
    color: red;
  }
}
*/ /*#__PURE__*/ styled.div(__styleYak.MyDiv, {
    "style": {
        "--yrRH4Ik": ()=>2
    }
});

that isn't flagged by PostCSS but is flagged by Lightning CSS as invalid CSS.

To improve the error messages and to have a consistent feel (independent if you use Lightning CSS or PostCSS) we should add a validation step after we've generated the CSS within the comments.

What do you think?

@jantimon
Copy link
Owner

jantimon commented Dec 3, 2024

I like the idea of verifying the CSS code but I actually think we should rather not add it to next-yak

dev experience is super important - especially the speed of the feedback loop - and adding another validation step would probably slow down the development flow

neither styled-components nor CSS modules extensively add a validation at the transformation stage

the minifier will (hopefully) catch these issues - so maybe all it needs is to use LightningCSS as minifier

I also think your example actually points to a more specific issue we could address:

styled.div`
  &:nth-child(${() => 2}) {  // This pattern is inherently problematic
    color: red;
  }
`;

instead of adding a full validation, we could add an error for dynamic selectors

@Mad-Kat
Copy link
Collaborator Author

Mad-Kat commented Dec 3, 2024

Yes this idea would probably harm the developer experience with more time spent on validating instead of transpiling. Are you aware of any cases where dynamic selectors make sense and work?

@jantimon
Copy link
Owner

jantimon commented Dec 3, 2024

rIght now CSS does not support dynamic selectors at all - so I guess it's always wrong to use them

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