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

Add no-unknown-custom-media #6362

Closed
jeddy3 opened this issue Sep 26, 2022 · 10 comments · Fixed by #7594
Closed

Add no-unknown-custom-media #6362

jeddy3 opened this issue Sep 26, 2022 · 10 comments · Fixed by #7594
Labels
status: wip is being worked on by someone type: new rule an entirely new rule

Comments

@jeddy3
Copy link
Member

jeddy3 commented Sep 26, 2022

What is the problem you're trying to solve?

I'd like to disallow unknown custom media queries. For example:

@custom-media --foo (max-width: 30em);
@media (--bar) {}

What solution would you like to see?

A new rule to catch this. Like #6361, it's currently available as a plugin but it (perhaps) meets our criteria to be a built-in rule. Unlike #6361, it's for a draft spec. However, we already have a custom media queries rule (custom-media-pattern) and both PostCSS Preset Env and Lightening CSS offer syntax lowering for it.

Interestingly, the plugin also enforces using custom media queries through its always and always-known primary options. Shall we take the same approach or create two rules?:

  • one that checks for unknown custom media queries
  • and another that ensures matching queries use the corresponding name

I'm leaning towards the latter.

  • Name: no-unknown-custom-media
  • Primary option: true
  • Secondary options: importForm & resolver?
  • Autofixable: No
  • Message: - Unexpected unknown custom media
  • Description: "Disallow unknown custom media queries."
  • Extended description: "This rule considers custom media queries defined within the same source to be known. Use the importForm secondary option to specify more sources."
  • Section: "Avoid errors" -> "General"
@jeddy3 jeddy3 added the status: needs discussion triage needs further discussion label Sep 26, 2022
@ybiquitous
Copy link
Member

@jeddy3 Thanks for the proposal. The new rule sounds good to me. 👍🏼

Shall we take the same approach or create two rules?:

I agree with creating two rules; one is no-unknown-custom-media (avoiding errors), while another would be something like custom-media-*** (enforcing conventions). Although, it seems unclear whether the latter rule would be needed now.

@jeddy3 jeddy3 changed the title Add no-unknown-custom-media Add no-unknown-custom-media Sep 28, 2022
@jeddy3
Copy link
Member Author

jeddy3 commented Sep 28, 2022

I agree with creating two rules; one is no-unknown-custom-media (avoiding errors), while another would be something like custom-media-*** (enforcing conventions). Although, it seems unclear whether the latter rule would be needed now.

SGTM.

  • Name: no-unknown-custom-media
  • Primary option: true
  • Secondary options: importFrom: [] - array of glob paths to .css files
  • Autofixable: No
  • Message: - Unexpected unknown custom media
  • Description: "Disallow unknown custom media queries."
  • Extended description: "This rule considers custom media queries defined within the same source to be known. You can use the importFrom secondary option to specify more sources."
  • Section: "Avoid errors" -> "Unknown"

Labelling as ready to implement.

If anyone fancies contributing this rule, there are steps on how to add a new rule in the Developer guide.

@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: new rule an entirely new rule and removed status: needs discussion triage needs further discussion labels Sep 28, 2022
@romainmenke
Copy link
Member

We will provide a parser function for custom media in the media query parser.

parseCustomMedia('--foo screen and (min-width: 300px)')

This returns false or a CustomMedia instance.

CustomMedia has methods/properties for :

  • the extension name (--foo)
  • custom media has true or false keywords
  • media query list

This should make it possible to handle custom media in the same way as regular media query lists.

Rules that handle @media should be easily updated to also handle @custom-media without changing the AST visitors.

Copy link
Contributor

This issue is older than one month. Please ask before opening a pull request, as it may no longer be relevant.

@github-actions github-actions bot added status: ask to implement ask before implementing as may no longer be relevant and removed status: ready to implement is ready to be worked on by someone labels Jan 22, 2024
@fpetrakov fpetrakov added the status: wip is being worked on by someone label Apr 6, 2024
@ybiquitous ybiquitous removed the status: ask to implement ask before implementing as may no longer be relevant label Apr 6, 2024
@ybiquitous
Copy link
Member

@fpetrakov Thank you for starting this issue! @romainmenke's package seems to provide parseCustomMedia(), so I believe there are no blockers. 👍🏼

@ybiquitous
Copy link
Member

FYI, we cannot provide the importFrom secondary option because of a resolver problem. See also:

@silverwind
Copy link

@custom-media seems to not be part of any CSS standard, I wonder why stylelint chose to even support it.

@romainmenke
Copy link
Member

romainmenke commented Apr 23, 2024

@custom-media seems to not be part of any CSS standard

It is part of this standard : https://www.w3.org/TR/mediaqueries-5/#custom-mq
Browsers are however not very eager to implement this (yet).

@silverwind
Copy link

silverwind commented Apr 23, 2024

Yeah, I guess they are more likely to just make variables work inside media queries than to implement another at-rule, I suppose 😆

@romainmenke
Copy link
Member

No, they are even less eager to do that :D

But I should take the time in the near future to check if it can get unstuck. It is also the 4th most visited page on the postcss-preset-env repo.

Screenshot 2024-04-23 at 19 45 48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: new rule an entirely new rule
Development

Successfully merging a pull request may close this issue.

5 participants