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 at-charset-rule-no-invalid #7492

Open
firefoxic opened this issue Jan 23, 2024 · 17 comments
Open

Add at-charset-rule-no-invalid #7492

firefoxic opened this issue Jan 23, 2024 · 17 comments
Labels
status: needs discussion triage needs further discussion

Comments

@firefoxic
Copy link
Contributor

firefoxic commented Jan 23, 2024

What is the problem you're trying to solve?

I went to fix this bug. And found out that @charset is not even an at-rule, but a special construct with much stricter syntax requirements than at-rules. And it doesn't seem to be about choosing a formatting style at all. After all, if the quotes are single, the parser will just pass it by without recognizing it as an encoding declaration. It is good if the encoding is utf-8, which is the default encoding. But if another encoding is used, it will probably cause an error.

So it's so much a matter of syntax (rather than formatting style) that the rule controlling @charset:

  1. should be in Stylelint itself, not in the plugin;
  2. should have one single option — the value true;
  3. should be added not even to the standard, but to the recommended config.

What solution would you like to see?

Add a separate rule to control the @charset syntax, which would take into account the character set of this declaration, its location in the style file, and maybe something else I may not know about yet.

@ybiquitous ybiquitous added the status: needs discussion triage needs further discussion label Jan 23, 2024
@ybiquitous
Copy link
Member

@firefoxic Thanks for the proposal. Very interesting to me.

Before looking into this issue and @charset, I didn't know the at-rule was deprecated. The following MDN documents say:

So, I wonder if a new rule should disallow the use of @charset, e.g.

@charset "UTF-8"; /* Unexpected deprecated "@charset" (no-charset) */

This rule might be expected to be put in the recommended config, as you suggest.

@firefoxic
Copy link
Contributor Author

So, I wonder if a new rule should disallow the use of @charset

I'm not sure about this. The same article https://developer.mozilla.org/en-US/docs/Web/CSS/@charset also mentions this:

but it is fully supported in all browsers.

Maybe someone can rely on such an encoding declaration.

On the other hand, it's just one way to determine encoding. And if it really is deprecated, linter would be better off not allowing it at all. Then yes, the rule would be better called no-charset.

But if it is possible to specify @charset (e.g. "no-charset": false), then we should still require clear conformance to the specification:

the first 1024 bytes of the stream begin with the hex sequence
40 63 68 61 72 73 65 74 20 22 XX* 22 3B

That is, there must be double quotes (22) around the encoding name (XX*), 1 (not 2, not 3, and not even 0) space (20) before the first quote, and ; (3B) after the second quote. If there is a @charset followed by characters that do not match the specification, and/or if they are not the first characters in the file - consider it an error.

@ybiquitous
Copy link
Member

if it really is deprecated

Yes, this matters. We want the rationale of this deprecation, maybe somewhere in CSS specs?

@romainmenke
Copy link
Member

romainmenke commented Jan 23, 2024

I think this is an error/ambiguity in the mdn docs.

In CSS 2.1 @charset ... would be parsed as an atrule contruct.
Later specifications require it to be dropped instead.

https://www.w3.org/TR/css-syntax-3/#charset-rule

So the deprecation is limited to how @charset ... is exposed or not to further parser internals and thereby to things like CSSOM.

If I am reading everything correctly

@ybiquitous
Copy link
Member

Thanks. These MDN docs seemed to be updated by PR mdn/content#31076

@firefoxic
Copy link
Contributor Author

@SelenIT maybe you can help with this vague wording?

@romainmenke
Copy link
Member

I've opened an issue to ask the spec editors directly : w3c/csswg-drafts#9838

@firefoxic
Copy link
Contributor Author

Given the answer, it seems that the rule would still make sure that @charset "..."; is spelled correctly in the presence of something like it (disallow single quotes, upper case, spaces greater than one, no semicolons, etc.). Am I understanding this correctly?

@romainmenke
Copy link
Member

And preceding content :

/* a comment */
@charset ... /* invalid because it is not at the very start */

@ybiquitous
Copy link
Member

@romainmenke Thanks for the effort. I now understand the syntax spec.

But I still wonder if @charset would be necessary or often used in modern CSS programming. 🤔
I understand the spec allows the use of @charset, but we don't always need @charset if a file's encoding is UTF-8, right?

@firefoxic
Copy link
Contributor Author

firefoxic commented Jan 24, 2024

As far as I understand it, if the developer even crookedly wrote something like charset somewhere, he wanted to rely on what he specifies. And the linter needs to see to it that it's spelled correctly (including the location of that declaration, yes).

There is no necessity for @charset of course.

@romainmenke
Copy link
Member

romainmenke commented Jan 24, 2024

Correct, the default is utf-8 anyway and I haven't used @charset in ages.

The primary use case is:

  • you do not have control over the http response headers for the stylesheet (this is not uncommon)
  • there is an environment encoding (this is extremely rare)
  • the stylesheet encoding differs from the environment encoding (this is extremely rare)

In that very specific case you can use @charset "utf-8" (or your encoding instead of utf-8) to make sure your stylesheet is correctly read.

Today it would be exceedingly rare to need this given that everything has become utf-8 by default.


I think there can be general guidance that @charset is not something you need.
While I also agree with @firefoxic that if people do include it that it must be correct.

@alexander-akait
Copy link
Member

You can have Apache and set AddDefaultCharset windows-1252, but your CSS can be in UTF-8, some services use non standard charsets to reduce size of pages, but this is extremely rare

@firefoxic
Copy link
Contributor Author

I have already started to think about how much, what and how I will have to change in the plugin in view of everything discussed here.

But I decided to take a look at the Stylelint code — it turns out that it also needs a lot of changes. For example:

image

And if in the readme files in the code examples @charset can easily be replaced by any @layer without losing much of its meaning, then the code where the logic relies on the fact that @charset can be written in different ways will also need to be corrected somehow.

Also, considering that @charset is not an at-rule at all, it might be better to ignore @charset in the linter rules that regulate the at-rules.


Looks like I've opened a closet whose junk has us all in over our heads 🤪

@ybiquitous
Copy link
Member

@firefoxic @romainmenke Thanks for your kind explanation. I understand the necessity to lint @charset. 👍🏼

However, I still wonder whether we should implement the new rule and include it in the recommended config. If it's rarely used, is it worth implementing it? Wouldn't a plugin be enough?

@firefoxic
Copy link
Contributor Author

However, I still wonder whether we should implement the new rule and include it in the recommended config. If it's rarely used, is it worth implementing it? Wouldn't a plugin be enough?

About the presence of a rule in Stylelint and in the config I think it is necessary to decide separately. Although it's not for me to decide, but my opinion is as follows: the rule should be in Stylelint itself, if only because it's about syntax, not preferences. And also because the logic of this rule should (in my opinion) affect fixes in other parts of Stylelint code (and plugins like mine).

On the config, my opinion would be even less relevant. But if the rule appears in Stylelint, it would make more sense to me to see it in stylelint-config-recommended.

@ybiquitous
Copy link
Member

Okay, there seems to be some rationale in the new rule. Additionally, I'd like to clarify a few more things:

  • Complexity in implementation (may cause bugs)
  • Performance (may slow analysis)

Of course, as far as we know now.

@Mouvedia Mouvedia changed the title Add charset-syntax-correct rule Add at-charset-rule-no-invalid Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs discussion triage needs further discussion
Development

No branches or pull requests

4 participants