Add media-feature-range-notation autofix

[jeddy3]

What is the problem you're trying to solve?

The rule isn't autofixable.

What solution would you like to see?

Have it autofixable so that:

• we can turn it on in our standard config
• make it consistent with our other *-notation rules

See #6497 (comment)

It'll only be for the more modern "context" option as that addresses the limitations of the prefix approach.

We could reach for a new media-query-list-parser.

It won't combine media features. For example:

@media (min-width: 500px) and screen and (max-width: 1200px) {}

Becomes:

@media (width >= 500px) and screen and (width <= 1200px) {}

Rather than:

@media (500px <= width <= 500px) and screen {}

I've labelled the issue as ready to implement. Please consider contributing if anyone has time.

There are steps to add autofix to a rule in the Developer guide.

[ybiquitous]

We could reach for a new media-query-list-parser.

It sounds postcss-media-query-parser could be replaced!

$ git grep postcss-media-query-parser lib/
lib/rules/media-feature-name-allowed-list/index.js:8:const mediaParser = require('postcss-media-query-parser').default;
lib/rules/media-feature-name-case/index.js:7:const mediaParser = require('postcss-media-query-parser').default;
lib/rules/media-feature-name-disallowed-list/index.js:8:const mediaParser = require('postcss-media-query-parser').default;
lib/rules/media-feature-name-no-unknown/index.js:8:const mediaParser = require('postcss-media-query-parser').default;
lib/rules/media-feature-name-value-allowed-list/index.js:3:const mediaParser = require('postcss-media-query-parser').default;
lib/rules/no-duplicate-at-import-rules/index.js:3:const mediaParser = require('postcss-media-query-parser').default;
lib/rules/rangeContextNodeParser.js:22: * @param {import('postcss-media-query-parser').Node} node
lib/rules/unit-disallowed-list/index.js:6:const mediaParser = require('postcss-media-query-parser').default;
lib/rules/unit-disallowed-list/index.js:29: * @param {import('postcss-media-query-parser').Child} mediaFeatureNode
lib/rules/unit-no-unknown/index.js:9:const mediaParser = require('postcss-media-query-parser').default;

[romainmenke]

Let us know if you need any help using our media query list parser.
Currently it isn't very simple :/

I needed it for fallback plugins but haven't done the important bits for the wider community.

• no docs
• only a low level API

It is fully typed, so that helps a bit.

I will gladly spend time on docs and some more high level functions if there is a community need for this.

[jeddy3]

I will gladly spend time on docs and some more high level functions if there is a community need for this.

That would be amazing as we've quite a few existing and upcoming rules that would make use of it!

[romainmenke]

After reading this comment I realized that I left things vaguely here :)

It is not easy for me to provide better documentation and/or a more ergonomic API without specific use cases.

I've opened an issue to collect use cases here : csstools/postcss-plugins#763

[romainmenke]

I've been converting a few of the existing stylelint rules to our media query parser and ran into an issue.

Our parser is unable to handle sass, less and other non standard syntaxes.

For example : @media (MIN-WIDTH: ($var + 10px)) { }

This looks like a plain media feature with a name and a value but our parser will parse this as general enclosed because the value doesn't have the correct syntax.

$var for example is not one thing. It is a delim token ($) and an ident token (var).

It is possible to recover from encountering a general enclosed node but this is far from trivial. It requires you to hand of the raw token stream to another parser.

---

Was there a specific reason to replace the current parser in use in styelint?

[ybiquitous]

@romainmenke Thanks for the investigation. The reason we need another parser is that postcss-media-query-parser is outdated (last published 6 years ago) and doesn't parse new syntaxes.

[jeddy3]

I've been converting a few of the existing stylelint rules to our media query parser

That's fantastic, thank you!

(Although, be sure to avoid any of the media query rules we're deprecating as we'll be removing them in the next major release after 15.0.0).

Our parser is unable to handle sass, less and other non standard syntaxes.

That's totally OK. The built-in rules are geared towards standard CSS. We use the isStandardSyntax* utils, e.g. isStandardSyntaxMediaFeature, to ignore non-standard things. For example, we'd bail before attempting to parse the media query list if it contained a dollar variable (or anything else non-standard, like interpolation):

if (!isStandardSyntaxMediaQueryList(atRule.params) return;
const mediaQueryList = parse(atRule.params);

Was there a specific reason to replace the current parser in use in styelint?

As @ybiquitous mentioned, the outdated parser we currently use doesn't support some modern CSS syntax, specifically modern range context notation. We current hack around it by using the postcss-value-parser to parse media queries in a range context 😬 . A modern media list parser that allows to do things like walk media feature names regardless of whether it's written in prefix (min-*/max-*) or context (x > y) notation would be amazing!

[romainmenke]

Although, be sure to avoid any of the media query rules we're deprecating as we'll be removing them in the next major release after 15.0.0

That is good to know!

I am converting these anyway, but I don't plan to submit a patch for this.
I first want to see which types of things are painful with our API so that I can improve it.

A modern media list parser that allows to do things like walk media feature names regardless of whether it's written in prefix (min-/max-) or context (x > y) notation would be amazing!

This is one of the thing that immediately became clear.
A media feature with a range context has a complex AST structure and currently it is needed to do entry.node.name.name.value to get the Ident token for the feature name.

entry.node.getName() and entry.node.getNameToken() is much easier to use.
The same methods should exist in plain and boolean features and the parent object MediaFeature

That allows you to do :

if (isMediaFeature(entry.node) && entry.node.getName().toLowerCase() === 'width') {
  console.log(entry.node.getName())
  const token = entry.node.getNameToken()
  token[1] = 'height'; // tokens are arrays, so mutations just work
}

That's totally OK. The built-in rules are geared towards standard CSS. We use the isStandardSyntax* utils, e.g. isStandardSyntaxMediaFeature, to ignore non-standard things. For example, we'd bail before attempting to parse the media query list if it contained a dollar variable (or anything else non-standard, like interpolation)

Also good to know :)
As long as this is fine in the long term then I think we can proceed 🎉