Repository: postcss/postcss-import
Pull request #540 (2023-07-23 09:36:41)

use `node.error` for CSS relevant errors
fixes https://github.com/postcss/postcss-import/issues/518

I've kept the `new Error()` when checking plugin options.
I don't think these are CSS syntax errors : https://github.com/postcss/postcss/blob/main/docs/guidelines/plugin.md#41-use-nodeerror-on-css-relevant-errors


Patch:
diff --git a/README.md b/README.md
index b698874..aba46ef 100644
--- a/README.md
+++ b/README.md
@@ -164,7 +164,7 @@ Type: `Function`
 Default: `null`
 
 You can provide a custom path resolver with this option. This function gets
-`(id, basedir, importOptions)` arguments and should return a path, an array of
+`(id, basedir, importOptions, astNode)` arguments and should return a path, an array of
 paths or a promise resolving to the path(s). If you do not return an absolute
 path, your path will be resolved to an absolute path using the default
 resolver.
diff --git a/lib/join-media.js b/lib/join-media.js
index 0eff3c9..04ee827 100644
--- a/lib/join-media.js
+++ b/lib/join-media.js
@@ -20,6 +20,8 @@ module.exports = function joinMedia(parentMedia, childMedia) {
         } else {
           media.push(`${parentItem} and ${childItem}`)
         }
+      } else {
+        media.push(parentItem)
       }
     })
   })
diff --git a/lib/parse-styles.js b/lib/parse-styles.js
index d021dd4..4c3c893 100644
--- a/lib/parse-styles.js
+++ b/lib/parse-styles.js
@@ -27,8 +27,7 @@ async function parseStyles(
     stmt.parentMedia = media
     stmt.layer = joinLayer(layer, stmt.layer || [])
 
-    // skip protocol base uri (protocol://url) or protocol-relative
-    if (stmt.type !== "import" || /^(?:[a-z]+:)?\/\//i.test(stmt.uri)) {
+    if (stmt.type !== "import" || !isProcessableURL(stmt.uri)) {
       continue
     }
 
@@ -50,8 +49,8 @@ async function parseStyles(
     else if (
       stmt.node.params.toLowerCase() !== charset.node.params.toLowerCase()
     ) {
-      throw new Error(
-        `Incompatable @charset statements:
+      throw stmt.node.error(
+        `Incompatible @charset statements:
   ${stmt.node.params} specified in ${stmt.node.source.input.file}
   ${charset.node.params} specified in ${charset.node.source.input.file}`
       )
@@ -108,12 +107,14 @@ async function resolveImportId(result, stmt, options, state, postcss) {
     ? path.dirname(atRule.source.input.file)
     : options.root
 
-  const paths = [await options.resolve(stmt.uri, base, options)].flat()
+  const paths = [await options.resolve(stmt.uri, base, options, atRule)].flat()
 
   // Ensure that each path is absolute:
   const resolved = await Promise.all(
     paths.map(file => {
-      return !path.isAbsolute(file) ? resolveId(file, base, options) : file
+      return !path.isAbsolute(file)
+        ? resolveId(file, base, options, atRule)
+        : file
     })
   )
 
@@ -219,4 +220,27 @@ async function loadImportContent(
   )
 }
 
+function isProcessableURL(uri) {
+  // skip protocol base uri (protocol://url) or protocol-relative
+  if (/^(?:[a-z]+:)?\/\//i.test(uri)) {
+    return false
+  }
+
+  // check for fragment or query 
+  try {
+    // needs a base to parse properly
+    const url = new URL(uri, "https://example.com")
+
+    if (url.hash) {
+      return false
+    }
+
+    if (url.search) {
+      return false
+    }
+  } catch {} // Ignore
+
+  return true
+}
+
 module.exports = parseStyles
diff --git a/lib/resolve-id.js b/lib/resolve-id.js
index d6d1668..aca3002 100644
--- a/lib/resolve-id.js
+++ b/lib/resolve-id.js
@@ -11,7 +11,7 @@ function resolveModule(id, opts) {
   })
 }
 
-module.exports = function resolveId(id, base, options) {
+module.exports = function resolveId(id, base, options, node) {
   const paths = options.path
 
   const resolveOpts = {
@@ -32,10 +32,10 @@ module.exports = function resolveId(id, base, options) {
     .catch(() => {
       if (paths.indexOf(base) === -1) paths.unshift(base)
 
-      throw new Error(
+      throw node.error(
         `Failed to find '${id}'
   in [
-    ${paths.join(",\n        ")}
+    ${paths.join(",\n    ")}
   ]`
       )
     })


Test patch:
diff --git a/test/fixtures/filter-ignore.css b/test/fixtures/filter-ignore.css
index d134503..923d64d 100644
--- a/test/fixtures/filter-ignore.css
+++ b/test/fixtures/filter-ignore.css
@@ -12,4 +12,6 @@
 @import url("//css");
 @import url('//css');
 @import url(//css);
+@import url('foo.css?query=string');
+@import url('foo.css#hash');
 content{}
diff --git a/test/fixtures/filter-ignore.expected.css b/test/fixtures/filter-ignore.expected.css
index a979122..03093e4 100644
--- a/test/fixtures/filter-ignore.expected.css
+++ b/test/fixtures/filter-ignore.expected.css
@@ -13,6 +13,8 @@
 @import url("//css");
 @import url('//css');
 @import url(//css);
+@import url('foo.css?query=string');
+@import url('foo.css#hash');
 @media (min-width: 25em){
 ignore{}
 }
diff --git a/test/fixtures/imports/media-query-screen.css b/test/fixtures/imports/media-query-screen.css
new file mode 100644
index 0000000..e96a63d
--- /dev/null
+++ b/test/fixtures/imports/media-query-screen.css
@@ -0,0 +1,1 @@
+@import "foo.css" screen;
diff --git a/test/fixtures/media-join.css b/test/fixtures/media-join.css
index b9d595a..28ca38a 100644
--- a/test/fixtures/media-join.css
+++ b/test/fixtures/media-join.css
@@ -1,3 +1,4 @@
 @import "media-join-nested.css" one-1;
 @import "media-join-nested.css" or-left-1, or-right-1;
 @import "media-join-nested.css" and-left-1 and and-right-1;
+@import "media-query-screen.css" screen;
diff --git a/test/fixtures/media-join.expected.css b/test/fixtures/media-join.expected.css
index 0dd4562..b733b0b 100644
--- a/test/fixtures/media-join.expected.css
+++ b/test/fixtures/media-join.expected.css
@@ -15,3 +15,7 @@
 @media and-left-1 and and-right-1 and or-left-2, and-left-1 and and-right-1 and or-right-2 {}
 
 @media and-left-1 and and-right-1 and and-left-2 and and-right-2 {}
+
+@media screen {
+foo{}
+}
diff --git a/test/import.js b/test/import.js
index c13bd76..ea4a9c1 100644
--- a/test/import.js
+++ b/test/import.js
@@ -63,7 +63,7 @@ test(
   "charset-import"
 )
 
-test("should error if incompatable @charset statements", t => {
+test("should error if incompatible @charset statements", t => {
   t.plan(2)
   const file = "test/fixtures/charset-error.css"
   return postcss()
@@ -73,7 +73,7 @@ test("should error if incompatable @charset statements", t => {
       t.truthy(err)
       t.regex(
         err.message,
-        /Incompatable @charset statements:.+specified in.+specified in.+/s
+        /Incompatible @charset statements:.+specified in.+specified in.+/s
       )
     })
 })
diff --git a/test/layer.js b/test/layer.js
index 69d27af..3414ff0 100644
--- a/test/layer.js
+++ b/test/layer.js
@@ -49,7 +49,9 @@ test("should error when value is not a function", t => {
   return postcss()
     .use(atImport({ nameLayer: "not a function" }))
     .process("", { from: undefined })
-    .catch(error => t.is(error.message, "nameLayer option must be a function"))
+    .catch(error =>
+      t.regex(error.message, /nameLayer option must be a function/)
+    )
 })
 
 test("should throw when using anonymous layers without the nameLayer plugin option", t => {
diff --git a/test/plugins.js b/test/plugins.js
index 20cee88..31df1ff 100644
--- a/test/plugins.js
+++ b/test/plugins.js
@@ -35,7 +35,7 @@ test("should error when value is not an array", t => {
   return postcss()
     .use(atImport({ plugins: "foo" }))
     .process("", { from: undefined })
-    .catch(error => t.is(error.message, "plugins option must be an array"))
+    .catch(error => t.regex(error.message, /plugins option must be an array/))
 })
 
 test("should remain silent when value is an empty array", t => {