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

@charset matching issue in v14 #484

Closed
AlecRust opened this issue Mar 2, 2022 · 8 comments
Closed

@charset matching issue in v14 #484

AlecRust opened this issue Mar 2, 2022 · 8 comments

Comments

@AlecRust
Copy link

AlecRust commented Mar 2, 2022

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.

@RyanZim
Copy link
Collaborator

RyanZim commented Mar 2, 2022

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.

@AlecRust
Copy link
Author

AlecRust commented May 4, 2022

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.

@RyanZim
Copy link
Collaborator

RyanZim commented May 4, 2022

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?

@AlecRust
Copy link
Author

AlecRust commented May 4, 2022

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.

@RyanZim
Copy link
Collaborator

RyanZim commented May 4, 2022

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

@AlecRust
Copy link
Author

AlecRust commented May 4, 2022

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.

@RyanZim
Copy link
Collaborator

RyanZim commented May 4, 2022

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

@RyanZim
Copy link
Collaborator

RyanZim commented Aug 30, 2022

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 RyanZim closed this as not planned Won't fix, can't repro, duplicate, stale Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants