Skip to content

feat: add no-invalid-import-placement rule #156

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

Closed

Conversation

thecalamiity
Copy link
Contributor

Prerequisites checklist

What is the purpose of this pull request?

This pull request introduces a new rule no-invalid-import-placement that enforces correct placement of @import rules within stylesheets.

What changes did you make? (Give an overview)

  • Implemented the no-invalid-import-placement rule to enforce correct placement of @import rules
  • Added documentation for the rule
  • Added tests

Related Issues

Fixes #151

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

@github-project-automation github-project-automation bot moved this to Needs Triage in Triage May 30, 2025
@lumirlumir lumirlumir added the accepted There is consensus among the team that this change meets the criteria for inclusion label May 31, 2025
Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, the code mostly LGTM. Just a few suggestions

Comment on lines +50 to +52
if (IGNORED_AT_RULES.has(name)) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it won't report @import if it is placed before @charset and @layer, which is also an invalid placement. Can you fix that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not an invalid placement though?

},

Rule() {
hasSeenNonImportRule = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we set it to false again once the traversing is complete?

recommended: true,
url: "https://github.com/eslint/css/blob/main/docs/rules/no-invalid-import-placement.md",
},

Copy link
Contributor

@snitin315 snitin315 May 31, 2025

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered it but didn’t find a need. Want it added anyway?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, we should add it for easy migration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We've decided to expand no-invalid-import-placement to no-invalid-at-rule-placement. Given this change, should we rename the ignoreAtRules option to something more specific, like ignoreAtRulesForImport?

@lumirlumir lumirlumir moved this from Needs Triage to Implementing in Triage Jun 4, 2025
@thecalamiity
Copy link
Contributor Author

Superseded by #171

@github-project-automation github-project-automation bot moved this from Implementing to Complete in Triage Jun 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion feature
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

New Rule: no-invalid-import-position
4 participants