postcss-import icon indicating copy to clipboard operation
postcss-import copied to clipboard

@charset matching issue in v14

Open AlecRust opened this issue 3 years ago • 7 comments

If I use postcss-import v14 I get the following warning:

postcss-import: @charset must precede all other statements

I see this was introduced in #447. Looks like my charsets aren't being merged in to one at the top.

Looking at my stylesheet, there are multiple charsets, but the only difference is the casing i.e.

@charset "utf-8";

.blah {
  color: #5200d0;
}

@charset "UTF-8";

.blah {
  color: #5200d0;
}

I wonder if this should be handled by postcss-import i.e. including just one @charset at the top without an error?

Otherwise I'm not sure what the "fix" here is, these are 3rd party stylesheets being merged in to one.

AlecRust avatar Mar 02 '22 12:03 AlecRust

Yeah, looking at this, it seems case doesn't matter in charset declarations, so we can probably merge them if that's the only difference. PR welcome.

RyanZim avatar Mar 02 '22 16:03 RyanZim

Yeah, looking at this, it seems case doesn't matter in charset declarations

Looking at #447 I think that was known at the time so I guess that "lowercase matching" isn't working correctly.

AlecRust avatar May 04 '22 14:05 AlecRust

OK, just re-reading this issue, want to make sure I'm not misunderstanding it. The example CSS you posted, is that your actual input CSS to postcss-import?

RyanZim avatar May 04 '22 15:05 RyanZim

Actually looking now, this is the only charset I'm importing:

https://github.com/basecamp/trix/blob/b6c0047f87fa078aa65beb8ba297aba32ea4bcf1/dist/trix.css#L1

No matter where the Trix import is in my stylesheet:

/*
 * Main application styles
 */

@import "trix";
@import "suitcss-base";
@import "suitcss-components-button";
@import "../styles/variables";
@import "../styles/base";

I get the same warning:

WARNING in ./app/packs/entrypoints/application.css (./app/packs/entrypoints/application.css.webpack[javascript/auto]!=!./node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[2].use[1]!./node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[2].use[2]!./app/packs/entrypoints/application.css)
Module Warning (from ./node_modules/postcss-loader/dist/cjs.js):
Warning

(300:1) postcss-import: @charset must precede all other statements
 @ ./app/packs/entrypoints/application.css

If I remove the Trix import, I get no warning.

So there's just one uppercase charset in my stylesheet, coming from this 3rd party CSS.

I suppose expected behaviour is that this is automatically moved to the top. This seems to be what it's doing already (regardless of where the import is in the list).

Output:

/*!*******************************************************************************************************************************************************************************************!*\
  !*** css ./node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[2].use[1]!./node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[2].use[2]!./app/packs/entrypoints/application.css ***!
  \*******************************************************************************************************************************************************************************************/
@charset "UTF-8";
/*
 * Main application styles
 */
/*! normalize.css v8.0.1 | MIT License | github.com/necolas/normalize.css */
/* Document
   ========================================================================== */
/**
 * 1. Correct the line height in all browsers.
 * 2. Prevent adjustments of font size after orientation changes in iOS.
 */
html {
  line-height: 1.15; /* 1 */
  -webkit-text-size-adjust: 100%; /* 2 */
}

So the output seems fine, but the error still occurs.

AlecRust avatar May 04 '22 15:05 AlecRust

I'm still not sure I understand. In the latest example you posted, where are the other charset statements?

RyanZim avatar May 04 '22 17:05 RyanZim

I think there were two charsets when I created the issue, but when looking now, there is just one charset imported via trix. Same warning though.

AlecRust avatar May 04 '22 17:05 AlecRust

A reduced test case would be useful here; there shouldn't be a warning if there's only one charset, at the top

RyanZim avatar May 04 '22 18:05 RyanZim

Given that a reduced test case has not been presented, and there have been no other reports of this issue, assuming it's some odd configuration issue, closing.

RyanZim avatar Aug 30 '22 19:08 RyanZim