Repository: postcss/postcss-import
Pull request #543 (2023-08-09 21:57:19)

Error on embedded imports in data urls
The current code prevents further inlining of embedded imports but it doesn't stop a browser from possible loading and applying it.

~~Adding `not all` has the intended effect.~~

Setting `stmt.children = []` prevents inlining and removes the `@import` statement.

RyanZim: If such imports are completely useless, why do we output useless imports vs. erroring?
romainmenke: A warning would help users to write more effective CSS.

But a hard error might be too much as this is perfectly valid CSS, it's just CSS that isn't applied.

This is similar to the warning for empty files.
Empty files are valid, just not very useful.
romainmenke: I've added a suggestion for a warning, please let me know if this is clear.

-----

An alternative would be to warn and completely remove the `@import` node.

But that would require a larger refactor, adding `not all` is a non-intrusive and technically correct way to ignore these `@import` statements.
RyanZim: Needs rebase.

> But a hard error might be too much as this is perfectly valid CSS, it's just CSS that isn't applied.

In other words, you're saying browsers ignore imports like these? I'm not sure if that's a valid premise to build on, because browsers will continue to parse the CSS they can, even if an import results in a 404; however, as a build tool, we'd be remiss not to error out if we can't resolve an import.
romainmenke: > we'd be remiss not to error out if we can't resolve an import.

Makes sense!
I've changed it to an error.

Thank you for reviewing these

Patch:
diff --git a/lib/parse-styles.js b/lib/parse-styles.js
index 6aaa245..130ad4a 100644
--- a/lib/parse-styles.js
+++ b/lib/parse-styles.js
@@ -92,10 +92,10 @@ async function resolveImportId(result, stmt, options, state, postcss) {
 
     return
   } else if (dataURL.isValid(stmt.from.slice(-1))) {
-    // Data urls can't be used a base url to resolve imports.
-    // When the parent statement has a data url
-    // and the current statement doesn't have a data url we ignore the statement.
-    return
+    // Data urls can't be used as a base url to resolve imports.
+    throw stmt.node.error(
+      `Unable to import '${stmt.uri}' from a stylesheet that is embedded in a data url`
+    )
   }
 
   const atRule = stmt.node


Test patch:
diff --git a/test/data-url.js b/test/data-url.js
index 13af7d8..33e469f 100644
--- a/test/data-url.js
+++ b/test/data-url.js
@@ -1,8 +1,27 @@
 "use strict"
 // external tooling
 const test = require("ava")
+const postcss = require("postcss")
+
+// plugin
+const atImport = require("..")
 
 // internal tooling
 const checkFixture = require("./helpers/check-fixture")
 
 test("should inline data urls", checkFixture, "data-url")
+
+test("should error on relative urls from stylesheets in data urls", t => {
+  return postcss()
+    .use(atImport())
+    .process(
+      "@import url(data:text/css;base64,QGltcG9ydCB1cmwoZm9vLmNzcyk7CgpwIHsKICBjb2xvcjogYmx1ZTsKfQo=);",
+      { from: undefined }
+    )
+    .catch(error =>
+      t.regex(
+        error.message,
+        /Unable to import '(?:.*?)' from a stylesheet that is embedded in a data url/
+      )
+    )
+})
diff --git a/test/fixtures/data-url.css b/test/fixtures/data-url.css
index 4b2b4b3..c26bdc0 100644
--- a/test/fixtures/data-url.css
+++ b/test/fixtures/data-url.css
@@ -2,7 +2,6 @@
 @import url("DATA:TEXT/CSS;BASE64,QGltcG9ydCB1cmwoZGF0YTp0ZXh0L2NzcztiYXNlNjQsY0NCN0lHTnZiRzl5T2lCbmNtVmxianNnZlE9PSk7CgpwIHsgY29sb3I6IGJsdWU7IH0K") layer(foo) (min-width: 320px);
 
 /* Mixed imports: */
-@import url(data:text/css;base64,QGltcG9ydCB1cmwoZm9vLmNzcyk7CgpwIHsKICBjb2xvcjogYmx1ZTsKfQo=);
 @import url(data-url.css);
 
 /* url encoded: */
diff --git a/test/fixtures/data-url.expected.css b/test/fixtures/data-url.expected.css
index af160fc..cad2495 100644
--- a/test/fixtures/data-url.expected.css
+++ b/test/fixtures/data-url.expected.css
@@ -1,6 +1,24 @@
-@import url(foo.css);p { color: green; }p { color: blue; }@media (min-width: 320px){@layer foo{
-p { color: green; } } }@media (min-width: 320px){@layer foo{
+p { color: green; }
 
-p { color: blue; } } }/* Mixed imports: */p {
-  color: blue;
-}p { color: pink; }/* url encoded: */bar { color: green }bar { color: pink }
+p { color: blue; }
+
+@media (min-width: 320px) {
+
+ @layer foo {
+p { color: green; } } }
+
+@media (min-width: 320px) {
+
+ @layer foo {
+
+p { color: blue; } } }
+
+/* Mixed imports: */
+
+p { color: pink; }
+
+/* url encoded: */
+
+bar { color: green }
+
+bar { color: pink }