Repository: postcss/postcss-import
Pull request #447 (2020-12-12 17:57:21)

Fix charset handling
OK, this is a _slightly_ naive implementation, but I think it's pretty good. If there's a `@charset` anywhere in the CSS, move it to the top of the output and apply it to everything. If there are multiple `@charset`s, compare them. If they're all the same, just use one. If they're different, there's no way we can put all of these in one file, so error out. If you legitimately need to import files with different charsets, you can use the `plugins` or `load` option to add logic to convert all your files to a single encoding that matches the entry point.

Also added checks to ensure that `@charset` comes first, warn the user if this isn't the case.

Technically a breaking change due to adding the error and warning cases.

Attn @SLKnutson 

SLKnutson: This looks right to me 👍 

Patch:
diff --git a/index.js b/index.js
index 6b78849..233a172 100755
--- a/index.js
+++ b/index.js
@@ -107,10 +107,7 @@ function AtImport(options) {
 
         // Strip additional statements.
         bundle.forEach(stmt => {
-          if (stmt.type === "import") {
-            stmt.node.parent = undefined
-            styles.append(stmt.node)
-          } else if (stmt.type === "media") {
+          if (["charset", "import", "media"].includes(stmt.type)) {
             stmt.node.parent = undefined
             styles.append(stmt.node)
           } else if (stmt.type === "nodes") {
@@ -150,15 +147,33 @@ function AtImport(options) {
             }, Promise.resolve())
           })
           .then(() => {
+            let charset
             const imports = []
             const bundle = []
 
+            function handleCharset(stmt) {
+              if (!charset) charset = stmt
+              // charsets aren't case-sensitive, so convert to lower case to compare
+              else if (
+                stmt.node.params.toLowerCase() !==
+                charset.node.params.toLowerCase()
+              ) {
+                throw new Error(
+                  `Incompatable @charset statements:
+  ${stmt.node.params} specified in ${stmt.node.source.input.file}
+  ${charset.node.params} specified in ${charset.node.source.input.file}`
+                )
+              }
+            }
+
             // squash statements and their children
             statements.forEach(stmt => {
-              if (stmt.type === "import") {
+              if (stmt.type === "charset") handleCharset(stmt)
+              else if (stmt.type === "import") {
                 if (stmt.children) {
                   stmt.children.forEach((child, index) => {
                     if (child.type === "import") imports.push(child)
+                    else if (child.type === "charset") handleCharset(child)
                     else bundle.push(child)
                     // For better output
                     if (index === 0) child.parent = stmt
@@ -169,7 +184,9 @@ function AtImport(options) {
               }
             })
 
-            return imports.concat(bundle)
+            return charset
+              ? [charset, ...imports.concat(bundle)]
+              : imports.concat(bundle)
           })
       }
 
diff --git a/lib/parse-statements.js b/lib/parse-statements.js
index ac25e92..59bb427 100644
--- a/lib/parse-statements.js
+++ b/lib/parse-statements.js
@@ -29,6 +29,7 @@ module.exports = function (result, styles) {
     if (node.type === "atrule") {
       if (node.name === "import") stmt = parseImport(result, node)
       else if (node.name === "media") stmt = parseMedia(result, node)
+      else if (node.name === "charset") stmt = parseCharset(result, node)
     }
 
     if (stmt) {
@@ -64,20 +65,34 @@ function parseMedia(result, atRule) {
   }
 }
 
+function parseCharset(result, atRule) {
+  if (atRule.prev()) {
+    return result.warn("@charset must precede all other statements", {
+      node: atRule,
+    })
+  }
+  return {
+    type: "charset",
+    node: atRule,
+    media: [],
+  }
+}
+
 function parseImport(result, atRule) {
-  let prev = getPrev(atRule)
+  let prev = atRule.prev()
   if (prev) {
     do {
       if (
-        prev.type !== "atrule" ||
-        (prev.name !== "import" && prev.name !== "charset")
+        prev.type !== "comment" &&
+        (prev.type !== "atrule" ||
+          (prev.name !== "import" && prev.name !== "charset"))
       ) {
         return result.warn(
           "@import must precede all other statements (besides @charset)",
           { node: atRule }
         )
       }
-      prev = getPrev(prev)
+      prev = prev.prev()
     } while (prev)
   }
 
@@ -128,11 +143,3 @@ function parseImport(result, atRule) {
 
   return stmt
 }
-
-function getPrev(item) {
-  let prev = item.prev()
-  while (prev && prev.type === "comment") {
-    prev = prev.prev()
-  }
-  return prev
-}


Test patch:
diff --git a/test/fixtures/charset-error.css b/test/fixtures/charset-error.css
new file mode 100644
index 0000000..3456d4a
--- /dev/null
+++ b/test/fixtures/charset-error.css
@@ -0,0 +1,2 @@
+@charset "foobar";
+@import "imports/charset.css";
diff --git a/test/fixtures/charset-import.css b/test/fixtures/charset-import.css
new file mode 100644
index 0000000..f06162a
--- /dev/null
+++ b/test/fixtures/charset-import.css
@@ -0,0 +1,4 @@
+@charset "UTF-8";
+@import "test/fixtures/imports/foo.css";
+@import "test/fixtures/imports/charset.css";
+bar{}
diff --git a/test/fixtures/charset-import.expected.css b/test/fixtures/charset-import.expected.css
new file mode 100644
index 0000000..5262c38
--- /dev/null
+++ b/test/fixtures/charset-import.expected.css
@@ -0,0 +1,3 @@
+@charset "UTF-8";
+foo{}
+bar{}
diff --git a/test/fixtures/imports/charset.css b/test/fixtures/imports/charset.css
new file mode 100644
index 0000000..9f44090
--- /dev/null
+++ b/test/fixtures/imports/charset.css
@@ -0,0 +1,1 @@
+@charset "UTF-8";
diff --git a/test/import.js b/test/import.js
index 9b637c2..4bcafdc 100644
--- a/test/import.js
+++ b/test/import.js
@@ -46,6 +46,38 @@ test("should not fail with absolute and local import", t => {
     .then(result => t.is(result.css, "@import url('http://');\nfoo{}"))
 })
 
+test("should keep @charset first", t => {
+  const base = '@charset "UTF-8";\n@import url(http://);'
+  return postcss()
+    .use(atImport())
+    .process(base, { from: undefined })
+    .then(result => {
+      t.is(result.warnings().length, 0)
+      t.is(result.css, base)
+    })
+})
+
+test(
+  "should handle multiple @charset statements",
+  checkFixture,
+  "charset-import"
+)
+
+test("should error if incompatable @charset statements", t => {
+  t.plan(2)
+  const file = "test/fixtures/charset-error.css"
+  return postcss()
+    .use(atImport())
+    .process(readFileSync(file), { from: file })
+    .catch(err => {
+      t.truthy(err)
+      t.regex(
+        err.message,
+        /Incompatable @charset statements:.+specified in.+specified in.+/s
+      )
+    })
+})
+
 test("should error when file not found", t => {
   t.plan(1)
   const file = "test/fixtures/imports/import-missing.css"
diff --git a/test/lint.js b/test/lint.js
index 6361ae7..7e65c09 100644
--- a/test/lint.js
+++ b/test/lint.js
@@ -78,6 +78,23 @@ test("should not warn when @charset or @import statement before", t => {
   })
 })
 
+test("should warn when @charset is not first", t => {
+  return Promise.all([
+    processor.process(`a {} @charset "utf-8";`, { from: undefined }),
+    processor.process(`@media {} @charset "utf-8";`, { from: undefined }),
+    processor.process(`/* foo */ @charset "utf-8";`, { from: undefined }),
+    processor.process(`@import "bar.css"; @charset "utf-8";`, {
+      from: "test/fixtures/imports/foo.css",
+    }),
+  ]).then(results => {
+    results.forEach(result => {
+      const warnings = result.warnings()
+      t.is(warnings.length, 1)
+      t.is(warnings[0].text, "@charset must precede all other statements")
+    })
+  })
+})
+
 test("should warn when a user didn't close an import with ;", t => {
   return processor
     .process(`@import url('http://') :root{}`, { from: undefined })