-
-
Notifications
You must be signed in to change notification settings - Fork 228
feat: add support for invalid utility class diagnostics #1496
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
base: main
Are you sure you want to change the base?
feat: add support for invalid utility class diagnostics #1496
Conversation
…e find.ts getClassNamesInClassList logic for '$' handling
e371c5e to
9cc237b
Compare
| testFixture('invalid-class/css') | ||
| testFixture('invalid-class/css-multi-prop') | ||
| testFixture('invalid-class/css-multi-rule') | ||
| testFixture('invalid-class/vue-style-lang-sass') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd write all of these using testInline now. The .json file based tests will eventually disappear when I have the time to refactor everything (I'm slowly working on a larger change right now that'll also end up moving most relevant tests to the language service instead anyway)
| let index = 0 | ||
| for (let i = 0; i < parts.length; i++) { | ||
| if (i % 2 === 0 && !blocklist.includes(parts[i])) { | ||
| if (i % 2 === 0 && parts[i] && parts[i] !== '$' && !blocklist.includes(parts[i])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very js-specific thing. It probably doesn't belong here. Not sure where though.
Will need to think about this 🤔
| let decls = getClassNameDecls(state, className) | ||
| return !!decls | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine if this diagnostic doesn't work pre-v3 imo. v0.16.x of the language server is gonna drop support for v0–v2 anyway.
| function isClassValid(state: State, className: string): boolean { | ||
| if (state.v4) { | ||
| // V4: Use design system compilation | ||
| let roots = state.designSystem.compile([className]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check should just be this I think:
return state.designSystem.compile([className])[0].nodes.length > 0
| "warning", | ||
| "error" | ||
| ], | ||
| "default": "info", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure on this but I feel like this should probably be off by default. I'd worry it'd get too unnecessarily noisy for a lot of projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other big problem is this doesn't pick up classes from stylesheets in your own project because that information isn't (currently) available.
That seems like a pretty poor DX and I think I'd want to ship this only once that problem is solved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean ship with this feature being on by default ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second comment should've really been separate comment not a reply but what I mean is this:
Given this stylesheet:
@import "tailwindcss";
.my-class {
color: red;
}
And this HTML:
<div class="my-class flex"></div>
We'd see my-class and think it is invalid because it's not a "tailwind generated" class. This is technically solvable for v4 projects — albeit with some unfortunate perf penalties at the moment — but not for v3 where do not analyze stylesheets at all (nor have we ever for v3).
Summary
Introduces a new invalidClass diagnostic to flag unrecognized Tailwind utility classes, helping catch typos and invalid utilities.
Changes
'info'option alongside'ignore','warning','error'Tests
invalidClassinbasicandv4/basicfixturespackages\tailwindcss-language-server\tests\diagnostics\diagnostics.test.jsUsage
Configure in VSCode settings:
{ "tailwindCSS.lint.invalidClass": "info" }Based on:
#872 (comment)
Linked:
#872