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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

New rule: no-invalid-css #198

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

koddsson
Copy link

Hello 馃憢!

I thought it would be cool to have a complementary CSS rule to no-invalid-html. It could also be crucial to tools that operate on the CSS in Lit components such as @lit-labs/rollup-plugin-minify-html-literals.

Let me know what you think!

node.tag.name === 'css'
) {
const errors = validateCSS(
node.quasi.quasis.map((quasi) => quasi.value.raw).join('')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to why i'm not hugely active maintaining postcss-lit - this stuff becomes very finicky

think about:

css`
  .foo {
    color: ${someColor};
  }
`;

this'll be validated as:

.foo {
  color: ;
}

which presumably fails validation (if it doesn't, there'll be similar cases which do)

in postcss-lit we just tried our best to choose a sensible placeholder, but there's still endless edge cases 馃槥

maybe thats what we could do here though as "best effort"?

you can see what we do here:
https://github.com/43081j/postcss-lit/blob/b19673062b3f0a63f4be3eaa1845018d1460f156/src/util.ts#L81-L100

but maybe we can be looser than that and just tell consumers to turn it off if it breaks

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's a huge PITA. I was thinking that one could turn on this rule if they wanted to strictly not allow template interpolation like this. Or it could emit a warning if plugins like rollup-plugin-minify-html-templates won't be able to minify the given template.

https://github.com/43081j/postcss-lit/blob/b19673062b3f0a63f4be3eaa1845018d1460f156/src/util.ts#L125-L191

is something that's needed in `rollup-plugin-minify-html-templates!

Maybe I'll put this back into draft mode so I can document this better and maybe work around some of the edge cases as a best effort.

@43081j
Copy link
Owner

43081j commented May 10, 2024

I have read through FYI but this introduces a deep dependency im trying to get rid of in the ecosystem cleanup
So I'm trying to contribute upstream to remove that first

@koddsson koddsson marked this pull request as draft May 12, 2024 15:20
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

Successfully merging this pull request may close these issues.

None yet

2 participants