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

feat!: no-unknown-at-rules -> no-invalid-at-rules #12

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nzakas
Copy link
Member

@nzakas nzakas commented Nov 19, 2024

Prerequisites checklist

What is the purpose of this pull request?

Renamed no-unknown-at-rules to no-invalid-at-rules to cover more cases.

What changes did you make? (Give an overview)

  • Renamed source files from no-unknown-at-rules.* to no-invalid-at-rules.*
  • Added checks for invalid preludes, unknown descriptors, and invalid descriptor values.
  • Added more tests
  • Updated docs

Related Issues

Is there anything you'd like reviewers to focus on?

There are type errors that will be resolved once #11 is merged and I can rebase on top of it.

@jeddy3
Copy link

jeddy3 commented Nov 19, 2024

Looks ace!

Can I ask if the ambition is to integrate more specific validity cases (not caught by csstree) into this rule or add separate rules for those?

For example, @property requires some descriptors:

@property rules require a syntax and inherits descriptor; if either are missing, the entire rule is invalid and must be ignored. The initial-value descriptor is optional only if the syntax is the universal syntax definition, otherwise the descriptor is required; if it’s missing, the entire rule is invalid and must be ignored.

It'd catch an easy mistake for people to make.

@nzakas
Copy link
Member Author

nzakas commented Nov 19, 2024

@jeddy3

Can I ask if the ambition is to integrate more specific validity cases (not caught by csstree) into this rule or add separate rules for those?

I honestly haven't thought that far ahead yet. 😄 I could imagine wanting that check in this rule, but I'm not 100% sure yet. I'd like to just start with what csstree is providing for the first version and then think about things a bit more once we've got some feedback.

@mdjermanovic
Copy link
Member

There are merge conflicts now. Also, some tests were failing.

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

Successfully merging this pull request may close these issues.

3 participants