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

Change Request: PostCSS syntax support #29

Open
1 task
privatenumber opened this issue Dec 6, 2024 · 12 comments
Open
1 task

Change Request: PostCSS syntax support #29

privatenumber opened this issue Dec 6, 2024 · 12 comments
Labels
enhancement New feature or request

Comments

@privatenumber
Copy link

privatenumber commented Dec 6, 2024

Environment

ESLint version: Latest (Possible to include the version on the Code Explorer?)
@eslint/css version: Latest (Possible to include the version on the Code Explorer?)
Node version: N/A
npm version: N/A
Operating System: mac

What problem do you want to solve?

PostCSS is an extremely popular enhancement over CSS. However, since this plugin only seems to support strict CSS syntax, it cannot be leveraged.

For example, when using https://github.com/postcss/postcss-mixins, it yields a parsing error: Code Explorer

@define-mixin icon $network, $color: blue {
    .icon.is-$(network) {
        color: $color;
        @mixin-content;
    }
    .icon.is-$(network):hover {
        color: white;
        background: $color;
    }
}

@mixin icon twitter {
    background: url(twt.png);
}
@mixin icon youtube, red {
    background: url(youtube.png);
}

This emits the error:

Error: Semicolon or block is expected(column 20)

What do you think is the correct solution?

Some ideas:

  • Looser syntax parsing, maybe behind an option?
  • PostCSS plugin

Participation

  • I am willing to submit a pull request for this change.

Additional comments

No response

@privatenumber privatenumber added the enhancement New feature or request label Dec 6, 2024
@fasttime fasttime added this to Triage Dec 6, 2024
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Dec 6, 2024
@nzakas
Copy link
Member

nzakas commented Dec 6, 2024

Sorry, I can't tell from this description what "support PostCSS syntax" means. Can you explain what you mean by that? And put some examples right in the issue?

Please keep in mind, I've never used PostCSS so I'd need a little primer to understand what exactly it is you're asking for.

@nzakas nzakas moved this from Needs Triage to Triaging in Triage Dec 6, 2024
@privatenumber
Copy link
Author

privatenumber commented Dec 7, 2024

I've inlined the PostCSS syntax example and error for your convenience (originally linked in "Code Explorer"). I'd like to point out that the ESLint Code Explorer being shareable could be even more useful if we're encouraged to link to them as examples in issues.

It looks like this plugin uses csstree for parsing CSS rather than postcss. Interestingly, csstree doesn't seem to have an issue parsing "PostCSS syntax", as demonstrated in AST Explorer.

For context, PostCSS is the most popular CSS preprocessor (think Babel for CSS), with over 65 million weekly downloads. Ensuring compatibility with its syntax is becoming increasingly important. Since the CSS parser is a fundamental piece of this tool, evaluating the feasibility of supporting PostCSS syntax early on could position the plugin to be more versatile and widely adopted without major disruption later.

@nzakas
Copy link
Member

nzakas commented Dec 9, 2024

Thanks, that's helpful. It's interesting that AST Explorer works, although, it is using CSSTree v2.x whereas this plugin uses CSSTree v3.x. I'm guessing v3.x is a bit stricter in what it is expecting. As a note: CSSTree v2.x is producing an AST that represents what the browser would see if it were parsing the code as a regular CSS stylesheet, so that representation might not necessarily be what we're looking for in the plugin.

I'm not sure what to do about this, currently. CSSTree doesn't have many options for parsing, so I'd have to dig in to the logic and see if there's a way to modify how it's working.

Another option would be to create a parser based on PostCSS that would produce a compatible AST, but that seems like a lot of extra work.

So, I'll leave this open but can't promise any sort of timeline right now.

@nzakas nzakas moved this from Triaging to Evaluating in Triage Dec 9, 2024
@nzakas
Copy link
Member

nzakas commented Dec 10, 2024

Interestingly, when I wrote up a simple repro case with CSSTree, it parsed the PostCSS syntax without any problem. This leads me to believe that there's a bug in Code Explorer. Going to dig in a bit.

@nzakas
Copy link
Member

nzakas commented Dec 10, 2024

Okay, so here's what's happening: CSSTree reports parsing errors as it goes but continues to parse unless a fatal parsing error occurs (in which case it throws an error and everything stops). For non-fatal parsing errors, it calls the onParseError() callback, which we report as a parsing error and display in Code Explorer. In this way, Code Explorer is more strict than AST Explorer.

Now is the interesting question: what to do about it? Technically, this PostCSS syntax is a syntax error in regular CSS that should be reported, so Code Explorer is doing the right thing. I think this is the expected behavior of ESLint linting CSS code with invalid syntax. However, if people want to lint PostCSS syntax, it would be nice to allow that as well.

So what we'd need is a setting or a new language configured to allow PostCSS syntax. We can think about this in two ways:

  1. We could still use CSSTree and try to figure out from the non-fatal parsing error whether this is PostCSS syntax or not. This seems error-prone at best.
  2. We could use PostCSS itself, and create a CSSTree-compatible representation of the output. This would guarantee that we'd get the same results as PostCSS, but then we'd not be able to use CSSTree-specific functionality in rules for PostCSS code. This would also mean additional bytes in the package to handle this.

I think option 2 is probably the best option, as we could allow people to specify PostCSS plugins in their config and have things work as expected.

@jeddy3 I'm curious if you have any thoughts about this?

@nzakas
Copy link
Member

nzakas commented Dec 11, 2024

Update: I dug around in the PostCSS ecosystem for a bit and I'm not sure how that could be used for this plugin. At least from what I can tell, PostCSS plugins don't always modify the AST with the additional data they've parsed. For instance, by default PostCSS only represents rule selectors as a string, and then we'd need postcss-selector-parser to parse that into an AST, but it doesn't appear that postcss-selector-parser does that automatically, so even just including the plugin won't result in an AST that can be walked.

So at the moment I'm blocked and don't know how to proceed. If anyone with PostCSS experience would like to lend their expertise, please reach out.

@pepelsbey
Copy link

It would be a mistake to consider PostCSS as a language that can be supported, like Sass or TypeScript. PostCSS is a plugin runner. You can also write your plugins, only slightly extending CSS syntax or inventing your syntax.

PostCSS is the most popular CSS preprocessor (think Babel for CSS), with over 65 million weekly downloads

While it’s true, it might sound like there’s a custom syntax used in every one of these 65 million cases. I’d say the most popular use cases are Autoprefixer and postcss-preset-env, which don’t introduce anything custom on top of the regular CSS.

I doubt that something as loosely defined as a list of custom plugins can be effectively linted.

@onigoetz
Copy link

PostCSS itself can also use alternative parsers.

The syntax tree itself is pretty basic (you are right, selectors are left as strings, values as well). The difference is in the parsers that you can use such as postcss-scss or postcss-less. They will parse the different syntaxes (such as nested selectors) and produce a compatible AST.

PostCSS itself is not a language, but provides a quite permissive parser.

As mentioned above, each plugin passed to PostCSS needs to implement its own logic to parse values over and over again (If I recall correctly, a request was made to integrate PostCSS-selector-parser directly into PostCSS core but failed to get enough traction). All that to say that each ESLint CSS rule would need to do the same or do a transformation of the AST before sending it back to ESLint

@43081j
Copy link

43081j commented Dec 11, 2024

pretty much as mentioned above

there isn't really "postcss syntax". postcss just allows you to combine one or more plugins which provide syntax on top of standard CSS

its a deep rabbit hole to go down since we'd be trying to get eslint to support an infinite amount of extended CSS syntaxes

I think if people want to use eslint to lint extended CSS, there should just be some way of writing a plugin/parser for ESLint which does what you're describing (produces a compatible AST and passes it back to ESLint). then if someone wants to write such a plugin which uses postcss itself under the hood, they can.

@romainmenke
Copy link

Another danger of adding any kind of support for non-standard syntaxes is that these squat the same syntax space as future standard features.

Mixins for example are currently being standardized in regular CSS.
The native syntax will be different from whatever mixins PostCSS plugin people happen to be using today. We can't effectively lint for both.

@jeddy3
Copy link

jeddy3 commented Dec 12, 2024

Everyone has summed it up well.

Adding to what @pepelsbey said, I believe the bulk of PostCSS's downloads come from its hidden use in tools such as Tailwind, CSSNano, and Stylelint rather than from people using plugins for language extension.

My sense is that most people have shifted toward authoring vanilla CSS. The PostCSS language extension plugins were a stepping stone for some as they moved away from monolithic language extensions like SCSS.

In addition to the danger that @romainmenke mentioned, supporting language extensions in Stylelint has been a challenge because the new syntax mascarade as standard node types. We wrote a bunch of utilities, e.g. isStandardSyntaxDeclaration, to check if a thing that claims to be a thing is actually that thing. For example, the dollar variables introduced by postcss-simple-vars will mascarade as Decl nodes. It'd be worth trying to avoid going down that rabbit hole.

CSSTree reports parsing errors as it goes but continues to parse unless a fatal parsing error occurs (in which case it throws an error and everything stops). For non-fatal parsing errors, it calls the onParseError() callback, which we report as a parsing error and display in Code Explorer. In this way, Code Explorer is more strict than AST Explorer. Technically, this PostCSS syntax is a syntax error in regular CSS that should be reported, so Code Explorer is doing the right thing. I think this is the expected behavior of ESLint linting CSS code with invalid syntax.

The linter currently hard aborts on valid CSS that CSSTree doesn't yet recognise, for example the relaxed CSS nesting spec:

a {
 b {}
}
jeddy3@mac test % npx eslint test.css

/Users/jeddy3/Projects/test/test.css
  2:5  error  Parsing error: Colon is expected

Support for relaxed nesting is on CSSTree's roadmap. I wonder if the linter should be more permissive as CSS is forgiving of syntax errors, unlike JavaScript.

Here's a fork of AST Explorer that uses CSSTree 3.x. It shows the nested rule is placed within a Raw node. The nested rule won't be checked by any lint rules, but the rest of the file can be linted.

@nzakas
Copy link
Member

nzakas commented Dec 13, 2024

Thanks everyone, this is really helpful.

I think in the short-term, we can add a tolerant option to the parsing mode that doesn't throw on recoverable errors. That should at least get us to the point where most things can be parsed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Evaluating
Development

No branches or pull requests

7 participants