Skip to content

feat: Add support for media conditions in require-baseline rule#49

Merged
nzakas merged 3 commits into
eslint:mainfrom
ryo-manba:feat-require-baseline-media-conditions
Feb 19, 2025
Merged

feat: Add support for media conditions in require-baseline rule#49
nzakas merged 3 commits into
eslint:mainfrom
ryo-manba:feat-require-baseline-media-conditions

Conversation

@ryo-manba

Copy link
Copy Markdown
Contributor

Prerequisites checklist

What is the purpose of this pull request?

This pull request adds support for media conditions in the require-baseline rule.

What changes did you make? (Give an overview)

  • Added checks for media conditions in @media queries.
  • Updated baseline-data.js to include media conditions.
  • Adjusted test cases to cover media conditions.
  • Updated documentation to reflect the changes.

Related Issues

fixes #48

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

@linux-foundation-easycla

linux-foundation-easycla Bot commented Feb 16, 2025

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

@nzakas nzakas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the submission, this is looking really good. Just left one note about an edge case.

continue;
}

const conditionLevel = mediaConditions.get(child.name);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

conditionLevel can be undefined if someone is using an unknown media condition. In that case, we should just exit early because no-invalid-at-rules should catch it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point! I've fixed it.
Add check for invalid at-rules

@ryo-manba ryo-manba requested a review from nzakas February 17, 2025 23:34
node,
) {
for (const child of node.children) {
// ignore unknown media conditions - no-invalid-at-rules already catches this

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you also a add a test that has an invalid media condition to verify that it's ignored?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ryo-manba ryo-manba requested a review from nzakas February 19, 2025 14:10

@nzakas nzakas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks so much!

@nzakas nzakas merged commit ad9ddd7 into eslint:main Feb 19, 2025
@ryo-manba ryo-manba deleted the feat-require-baseline-media-conditions branch February 20, 2025 00:56
@nzakas

nzakas commented Mar 6, 2025

Copy link
Copy Markdown
Member

@ryo-manba we'd like to pay you for this contribution, but I couldn't find an email address to contact you. Please send an email to contact (at) eslint (dot) org.

@ryo-manba

Copy link
Copy Markdown
Contributor Author

@nzakas
Thanks! I just sent an email.

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

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

Bug: require-baseline does not properly validate media conditions in @media rules

2 participants