check import cycles (#535)

* check cycles

* Update lib/parse-styles.js

* add tests

index.js

@@ -63,7 +63,7 @@ function AtImport(options) {
         state,
         [],
         [],
-        "",
+        [],
         postcss
       )
 

lib/parse-styles.js

@@ -91,7 +91,7 @@ async function resolveImportId(result, stmt, options, state, postcss) {
     )
 
     return
-  } else if (dataURL.isValid(stmt.from)) {
+  } 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.
@@ -148,7 +148,7 @@ async function loadImportContent(
   postcss
 ) {
   const atRule = stmt.node
-  const { media, layer } = stmt
+  const { media, layer, from } = stmt
 
   assignLayerNames(layer, atRule, state, options)
 
@@ -168,6 +168,10 @@ async function loadImportContent(
     state.importedFiles[filename][media][layer] = true
   }
 
+  if (from.includes(filename)) {
+    return
+  }
+
   const content = await options.load(filename, options)
 
   if (content.trim() === "" && options.warnOnEmpty) {
@@ -215,7 +219,7 @@ async function loadImportContent(
     state,
     media,
     layer,
-    filename,
+    [...from, filename],
     postcss
   )
 }

test/fixtures/cyclical-skip-duplicates.css

@@ -0,0 +1,4 @@
+@import "cyclical-a.css";
+@import "cyclical-b.css";
+
+@import "cyclical-screen.css" screen;

test/fixtures/cyclical-skip-duplicates.expected.css

@@ -0,0 +1,23 @@
+
+
+.b {
+  color: red;
+}
+
+.a {
+  color: blue;
+}
+
+@media screen and all {
+
+.a {
+  color: aquamarine;
+}
+}
+
+@media screen {
+
+.a {
+  color: pink;
+}
+}

test/fixtures/cyclical.css

@@ -0,0 +1,4 @@
+@import "cyclical-a.css";
+@import "cyclical-b.css";
+
+@import "cyclical-screen.css" screen;

test/fixtures/cyclical.expected.css

@@ -0,0 +1,31 @@
+
+
+.b {
+  color: red;
+}
+
+.a {
+  color: blue;
+}
+
+.a {
+  color: blue;
+}
+
+.b {
+  color: red;
+}
+
+@media screen and all {
+
+.a {
+  color: aquamarine;
+}
+}
+
+@media screen {
+
+.a {
+  color: pink;
+}
+}

test/fixtures/imports/cyclical-a.css

@@ -0,0 +1,5 @@
+@import url(cyclical-b.css);
+
+.a {
+  color: blue;
+}

test/fixtures/imports/cyclical-all.css

@@ -0,0 +1,5 @@
+@import url(cyclical-screen.css) screen;
+
+.a {
+  color: aquamarine;
+}

test/fixtures/imports/cyclical-b.css

@@ -0,0 +1,5 @@
+@import url(cyclical-a.css);
+
+.b {
+  color: red;
+}

test/fixtures/imports/cyclical-screen.css

@@ -0,0 +1,5 @@
+@import url(cyclical-all.css) all;
+
+.a {
+  color: pink;
+}

test/import.js

@@ -13,15 +13,38 @@ const atImport = require("..")
 // internal tooling
 const checkFixture = require("./helpers/check-fixture")
 
-test("should import stylsheets", checkFixture, "simple")
+test("should import stylesheets", checkFixture, "simple")
 
-test("should not import a stylsheet twice", checkFixture, "no-duplicate")
+test("should not import a stylesheet twice", checkFixture, "no-duplicate")
 
-test("should be able to import a stylsheet twice", checkFixture, "duplicates", {
-  skipDuplicates: false,
-})
+test(
+  "should be able to import a stylesheet twice",
+  checkFixture,
+  "duplicates",
+  {
+    skipDuplicates: false,
+  }
+)
+
+test(
+  "should be able to import a stylesheet with cyclical dependencies",
+  checkFixture,
+  "cyclical",
+  {
+    skipDuplicates: false,
+  }
+)
+
+test(
+  "should be able to import a stylesheet with cyclical dependencies and skip duplicates is true",
+  checkFixture,
+  "cyclical-skip-duplicates",
+  {
+    skipDuplicates: true,
+  }
+)
 
-test("should import stylsheets with same content", checkFixture, "same")
+test("should import stylesheets with same content", checkFixture, "same")
 
 test("should ignore & adjust external import", checkFixture, "ignore")