Repository: postcss/postcss-import
Pull request #496 (2022-06-27 17:34:29)

fix import with layer
fixes : https://github.com/postcss/postcss-import/issues/495

@RyanZim it still feels a bit like the complexity (which I added) is exploding and the overal shape of the code base isn't optimal. But I also do not dislike this fix :)

Important spec bits : https://www.w3.org/TR/css-cascade-5/#at-import

> the optional layer keyword or layer() function assigns the contents of the style sheet into its own anonymous [cascade layer](https://www.w3.org/TR/css-cascade-5/#cascade-layers) or into the named cascade layer.
>
>The layer is added to the [layer order](https://www.w3.org/TR/css-cascade-5/#layer-ordering) even if the import fails to load the stylesheet, but is subject to any [import conditions](https://www.w3.org/TR/css-cascade-5/#import-conditions) (just as if declared by an [@layer](https://www.w3.org/TR/css-cascade-5/#at-ruledef-layer) rule wrapped in the appropriate [conditional group rules](https://www.w3.org/TR/css3-conditional/#conditional-group-rule)).the optional layer keyword or layer() function assigns the contents of the style sheet into its own anonymous [cascade layer](https://www.w3.org/TR/css-cascade-5/#cascade-layers) or into the named cascade layer.
>
>The layer is added to the [layer order](https://www.w3.org/TR/css-cascade-5/#layer-ordering) even if the import fails to load the stylesheet, but is subject to any [import conditions](https://www.w3.org/TR/css-cascade-5/#import-conditions) (just as if declared by an [@layer](https://www.w3.org/TR/css-cascade-5/#at-ruledef-layer) rule wrapped in the appropriate [conditional group rules](https://www.w3.org/TR/css3-conditional/#conditional-group-rule)).

Which means that `@import url('foo.css') layer(A) (min-width: 320px)` becomes :

```css
@media (min-width: 320px) {
  @layer A {
    /* imported styles */
  }
}

romainmenke: @RyanZim do you have time one of the following days to review this? 🙇 
RyanZim: @romainmenke My apologies for my slowness here; I've been quite busy. Then, I was in a fairly serious accident last week, so that's put things even more on behind. I have not forgotten; I will try to review in the coming days.
romainmenke: @RyanZim Thank you for the review and happy to hear things are getting better on your end 🎉 
RyanZim: I'm hesitant about the naming of anonymous layers, and what unexpected problems that may create. I'm not super familiar with `@layer` myself, so I don't know all the edge cases.
romainmenke: > I'm hesitant about the naming of anonymous layers, and what unexpected problems that may create. I'm not super familiar with @layer myself, so I don't know all the edge cases.

I actually am also unsure about this.
Thank you for calling this out.

We could make anonymous layered imports unsupported for now.
That way we avoid unforeseen edge cases while still shipping an improved version.

We can iterate on this later to add support for anonymous layers.
This would only affect `@import "foo.css" layer;` not `@layer {}`.

The current version might be incorrect when an author has multiple stylesheets, all with anonymous layered imports. There is no way for this plugin to be aware of that and the counter approach doesn't produce truly unique layer names.

I did however want to avoid using true random values as these make builds non-reproducible.

RyanZim: OK, compromise proposal: we don't support anonymous layers by default. To enable support, you must pass an option which is a function, that is passed the index, and must return the layer name. That way, if you want our current index solution, you could pass:

```js
(index) => `imported-anon-layer-${index}`
```

If you have multiple stylesheets, you could use different prefixes. Maybe we should also pass the root filename we're processing, for those who want to incorporate that into the name. Alternately, if someone doesn't care about deterministic builds, they could simply provide a random string generator (with or without a prefix).

I realize this is a bit complicated, but it also provides maximum flexibility; thoughts? Also, sane naming suggestions for such an option?
romainmenke: I like this!
I will try to implement this in the next few days :)
romainmenke: @RyanZim I think this is ready for review (when you have time).
For the moment I have settled on `nameLayer` for the plugin option.

Other canidates :

- `nameLayer`
- `deAnonymizeLayer`
- `generateLayerName`

-----

I wasn't sure how to pass the root file name considering that it might not be set.
At the moment I am using the import file name as a second argument.
romainmenke: @RyanZim Thank you for the feedback.
I've made the necessary changes.

Hopefully all good now :)

Patch:
diff --git a/README.md b/README.md
index f7e2ac2..ac0148e 100644
--- a/README.md
+++ b/README.md
@@ -190,6 +190,17 @@ This option is only for adding additional directories to default resolver. If
 you provide your own resolver via the `resolve` configuration option above, then
 this value will be ignored.
 
+#### `nameLayer`
+
+Type: `Function`
+Default: `null`
+
+You can provide a custom naming function for anonymous layers (`@import 'baz.css' layer;`).
+This function gets `(index, rootFilename)` arguments and should return a unique string.
+
+This option only influences imports without a layer name.
+Without this option the plugin will warn on anonymous layers.
+
 #### Example with some options
 
 ```js
diff --git a/index.js b/index.js
index e6a3dc3..6cb912e 100755
--- a/index.js
+++ b/index.js
@@ -19,6 +19,7 @@ function AtImport(options) {
     load: loadContent,
     plugins: [],
     addModulesDirectories: [],
+    nameLayer: null,
     ...options,
   }
 
@@ -37,9 +38,12 @@ function AtImport(options) {
       const state = {
         importedFiles: {},
         hashFiles: {},
+        rootFilename: null,
+        anonymousLayerCounter: 0,
       }
 
       if (styles.source && styles.source.input && styles.source.input.file) {
+        state.rootFilename = styles.source.input.file
         state.importedFiles[styles.source.input.file] = {}
       }
 
@@ -47,6 +51,10 @@ function AtImport(options) {
         throw new Error("plugins option must be an array")
       }
 
+      if (options.nameLayer && typeof options.nameLayer !== "function") {
+        throw new Error("nameLayer option must be a function")
+      }
+
       return parseStyles(result, styles, options, state, [], []).then(
         bundle => {
           applyRaws(bundle)
@@ -81,7 +89,30 @@ function AtImport(options) {
           if (stmt.type === "import") {
             stmt.node.params = `${stmt.fullUri} ${stmt.media.join(", ")}`
           } else if (stmt.type === "media") {
-            stmt.node.params = stmt.media.join(", ")
+            if (stmt.layer.length) {
+              const layerNode = atRule({
+                name: "layer",
+                params: stmt.layer.filter(layer => layer !== "").join("."),
+                source: stmt.node.source,
+              })
+
+              if (stmt.parentMedia && stmt.parentMedia.length) {
+                const mediaNode = atRule({
+                  name: "media",
+                  params: stmt.parentMedia.join(", "),
+                  source: stmt.node.source,
+                })
+
+                mediaNode.append(layerNode)
+                layerNode.append(stmt.node)
+                stmt.node = mediaNode
+              } else {
+                layerNode.append(stmt.node)
+                stmt.node = layerNode
+              }
+            } else {
+              stmt.node.params = stmt.media.join(", ")
+            }
           } else {
             const { nodes } = stmt
             const { parent } = nodes[0]
@@ -170,6 +201,7 @@ function AtImport(options) {
             return stmts.reduce((promise, stmt) => {
               return promise.then(() => {
                 stmt.media = joinMedia(media, stmt.media || [])
+                stmt.parentMedia = media
                 stmt.layer = joinLayer(layer, stmt.layer || [])
 
                 // skip protocol base uri (protocol://url) or protocol-relative
@@ -283,18 +315,38 @@ function AtImport(options) {
       function loadImportContent(result, stmt, filename, options, state) {
         const atRule = stmt.node
         const { media, layer } = stmt
+        layer.forEach((layerPart, i) => {
+          if (layerPart === "") {
+            if (options.nameLayer) {
+              layer[i] = options
+                .nameLayer(state.anonymousLayerCounter++, state.rootFilename)
+                .toString()
+            } else {
+              throw atRule.error(
+                `When using anonymous layers in @import you must also set the "nameLayer" plugin option`
+              )
+            }
+          }
+        })
+
         if (options.skipDuplicates) {
           // skip files already imported at the same scope
           if (
             state.importedFiles[filename] &&
-            state.importedFiles[filename][media]
+            state.importedFiles[filename][media] &&
+            state.importedFiles[filename][media][layer]
           ) {
             return
           }
 
           // save imported files to skip them next time
-          if (!state.importedFiles[filename]) state.importedFiles[filename] = {}
-          state.importedFiles[filename][media] = true
+          if (!state.importedFiles[filename]) {
+            state.importedFiles[filename] = {}
+          }
+          if (!state.importedFiles[filename][media]) {
+            state.importedFiles[filename][media] = {}
+          }
+          state.importedFiles[filename][media][layer] = true
         }
 
         return Promise.resolve(options.load(filename, options)).then(
@@ -305,8 +357,13 @@ function AtImport(options) {
             }
 
             // skip previous imported files not containing @import rules
-            if (state.hashFiles[content] && state.hashFiles[content][media])
+            if (
+              state.hashFiles[content] &&
+              state.hashFiles[content][media] &&
+              state.hashFiles[content][media][layer]
+            ) {
               return
+            }
 
             return processContent(
               result,
@@ -324,8 +381,13 @@ function AtImport(options) {
                 })
                 if (!hasImport) {
                   // save hash files to skip them next time
-                  if (!state.hashFiles[content]) state.hashFiles[content] = {}
-                  state.hashFiles[content][media] = true
+                  if (!state.hashFiles[content]) {
+                    state.hashFiles[content] = {}
+                  }
+                  if (!state.hashFiles[content][media]) {
+                    state.hashFiles[content][media] = {}
+                  }
+                  state.hashFiles[content][media][layer] = true
                 }
               }
 


Test patch:
diff --git a/test/fixtures/imports/layer-anonymous.css b/test/fixtures/imports/layer-anonymous.css
new file mode 100644
index 0000000..589aeeb
--- /dev/null
+++ b/test/fixtures/imports/layer-anonymous.css
@@ -0,0 +1,33 @@
+@import url("layer-level-2-anonymous.css") layer;
+
+body {
+  order: 1;
+}
+
+@media (min-width: 50rem) {
+  body {
+    order: 2;
+  }
+}
+
+@keyframes RED_TO_BLUE {
+  0% {
+    background-color: red;
+  }
+  100% {
+    background-color: blue;
+  }
+}
+
+@supports (display: grid) {
+  body {
+    display: grid;
+    order: 3;
+  }
+}
+
+@layer A {
+  body {
+    order: 4;
+  }
+}
diff --git a/test/fixtures/imports/layer-level-2-anonymous.css b/test/fixtures/imports/layer-level-2-anonymous.css
new file mode 100644
index 0000000..36d69c2
--- /dev/null
+++ b/test/fixtures/imports/layer-level-2-anonymous.css
@@ -0,0 +1,7 @@
+@import url("layer-level-3.css") layer (min-width: 320px);
+
+@layer Y {
+  body {
+    color: purple;
+  }
+}
diff --git a/test/fixtures/imports/layer-level-2.css b/test/fixtures/imports/layer-level-2.css
new file mode 100644
index 0000000..8ec5e25
--- /dev/null
+++ b/test/fixtures/imports/layer-level-2.css
@@ -0,0 +1,7 @@
+@import url("layer-level-3.css") layer(level-3) (min-width: 320px);
+
+@layer Y {
+  body {
+    color: purple;
+  }
+}
diff --git a/test/fixtures/imports/layer-level-3.css b/test/fixtures/imports/layer-level-3.css
new file mode 100644
index 0000000..1567ae9
--- /dev/null
+++ b/test/fixtures/imports/layer-level-3.css
@@ -0,0 +1,5 @@
+@layer Z {
+  body {
+    color: cyan;
+  }
+}
diff --git a/test/fixtures/imports/layer-rule-grouping.css b/test/fixtures/imports/layer-rule-grouping.css
new file mode 100644
index 0000000..c3febe5
--- /dev/null
+++ b/test/fixtures/imports/layer-rule-grouping.css
@@ -0,0 +1,13 @@
+rule-one {}
+
+rule-two {}
+
+@media (min-width: 50rem) {
+  rule-three {}
+
+  rule-four {}
+}
+
+rule-five {}
+
+rule-six {}
diff --git a/test/fixtures/imports/layer.css b/test/fixtures/imports/layer.css
new file mode 100644
index 0000000..9e554b7
--- /dev/null
+++ b/test/fixtures/imports/layer.css
@@ -0,0 +1,33 @@
+@import url("layer-level-2.css") layer(level-2);
+
+body {
+  order: 1;
+}
+
+@media (min-width: 50rem) {
+  body {
+    order: 2;
+  }
+}
+
+@keyframes RED_TO_BLUE {
+  0% {
+    background-color: red;
+  }
+  100% {
+    background-color: blue;
+  }
+}
+
+@supports (display: grid) {
+  body {
+    display: grid;
+    order: 3;
+  }
+}
+
+@layer A {
+  body {
+    order: 4;
+  }
+}
diff --git a/test/fixtures/layer-import-atrules-anonymous.css b/test/fixtures/layer-import-atrules-anonymous.css
new file mode 100644
index 0000000..f2d9356
--- /dev/null
+++ b/test/fixtures/layer-import-atrules-anonymous.css
@@ -0,0 +1,3 @@
+@import url("layer-anonymous.css") layer screen;
+
+@import url("layer-anonymous.css") layer;
diff --git a/test/fixtures/layer-import-atrules-anonymous.expected.css b/test/fixtures/layer-import-atrules-anonymous.expected.css
new file mode 100644
index 0000000..be1df27
--- /dev/null
+++ b/test/fixtures/layer-import-atrules-anonymous.expected.css
@@ -0,0 +1,127 @@
+@media screen and (min-width: 320px) {
+@layer import-anon-layer-0.import-anon-layer-1.import-anon-layer-2 {
+@layer Z {
+  body {
+    color: cyan;
+  }
+}
+}
+}
+
+@media screen {
+@layer import-anon-layer-0.import-anon-layer-1 {
+
+@layer Y {
+  body {
+    color: purple;
+  }
+}
+}
+}
+
+@media screen {
+@layer import-anon-layer-0 {
+
+body {
+  order: 1;
+}
+}
+}
+
+@media screen {
+@layer import-anon-layer-0 {
+
+@media (min-width: 50rem) {
+  body {
+    order: 2;
+  }
+}
+}
+}
+
+@media screen {
+@layer import-anon-layer-0 {
+
+@keyframes RED_TO_BLUE {
+  0% {
+    background-color: red;
+  }
+  100% {
+    background-color: blue;
+  }
+}
+
+@supports (display: grid) {
+  body {
+    display: grid;
+    order: 3;
+  }
+}
+
+@layer A {
+  body {
+    order: 4;
+  }
+}
+}
+}
+
+@media (min-width: 320px) {
+@layer import-anon-layer-3.import-anon-layer-4.import-anon-layer-5 {
+@layer Z {
+  body {
+    color: cyan;
+  }
+}
+}
+}
+
+@layer import-anon-layer-3.import-anon-layer-4 {
+
+@layer Y {
+  body {
+    color: purple;
+  }
+}
+}
+
+@layer import-anon-layer-3 {
+
+body {
+  order: 1;
+}
+}
+
+@layer import-anon-layer-3 {
+
+@media (min-width: 50rem) {
+  body {
+    order: 2;
+  }
+}
+}
+
+@layer import-anon-layer-3 {
+
+@keyframes RED_TO_BLUE {
+  0% {
+    background-color: red;
+  }
+  100% {
+    background-color: blue;
+  }
+}
+
+@supports (display: grid) {
+  body {
+    display: grid;
+    order: 3;
+  }
+}
+
+@layer A {
+  body {
+    order: 4;
+  }
+}
+}
diff --git a/test/fixtures/layer-import-atrules.css b/test/fixtures/layer-import-atrules.css
new file mode 100644
index 0000000..dab4c0e
--- /dev/null
+++ b/test/fixtures/layer-import-atrules.css
@@ -0,0 +1,3 @@
+@import url("layer.css") layer(imported-with-media) screen;
+
+@import url("layer.css") layer(imported-without-media);
diff --git a/test/fixtures/layer-import-atrules.expected.css b/test/fixtures/layer-import-atrules.expected.css
new file mode 100644
index 0000000..e4d6c19
--- /dev/null
+++ b/test/fixtures/layer-import-atrules.expected.css
@@ -0,0 +1,127 @@
+@media screen and (min-width: 320px) {
+@layer imported-with-media.level-2.level-3 {
+@layer Z {
+  body {
+    color: cyan;
+  }
+}
+}
+}
+
+@media screen {
+@layer imported-with-media.level-2 {
+
+@layer Y {
+  body {
+    color: purple;
+  }
+}
+}
+}
+
+@media screen {
+@layer imported-with-media {
+
+body {
+  order: 1;
+}
+}
+}
+
+@media screen {
+@layer imported-with-media {
+
+@media (min-width: 50rem) {
+  body {
+    order: 2;
+  }
+}
+}
+}
+
+@media screen {
+@layer imported-with-media {
+
+@keyframes RED_TO_BLUE {
+  0% {
+    background-color: red;
+  }
+  100% {
+    background-color: blue;
+  }
+}
+
+@supports (display: grid) {
+  body {
+    display: grid;
+    order: 3;
+  }
+}
+
+@layer A {
+  body {
+    order: 4;
+  }
+}
+}
+}
+
+@media (min-width: 320px) {
+@layer imported-without-media.level-2.level-3 {
+@layer Z {
+  body {
+    color: cyan;
+  }
+}
+}
+}
+
+@layer imported-without-media.level-2 {
+
+@layer Y {
+  body {
+    color: purple;
+  }
+}
+}
+
+@layer imported-without-media {
+
+body {
+  order: 1;
+}
+}
+
+@layer imported-without-media {
+
+@media (min-width: 50rem) {
+  body {
+    order: 2;
+  }
+}
+}
+
+@layer imported-without-media {
+
+@keyframes RED_TO_BLUE {
+  0% {
+    background-color: red;
+  }
+  100% {
+    background-color: blue;
+  }
+}
+
+@supports (display: grid) {
+  body {
+    display: grid;
+    order: 3;
+  }
+}
+
+@layer A {
+  body {
+    order: 4;
+  }
+}
+}
diff --git a/test/fixtures/layer-rule-grouping.css b/test/fixtures/layer-rule-grouping.css
new file mode 100644
index 0000000..afa489e
--- /dev/null
+++ b/test/fixtures/layer-rule-grouping.css
@@ -0,0 +1,6 @@
+@import url("layer-rule-grouping.css") screen;
+
+@import url("layer-rule-grouping.css") layer;
+@import url("layer-rule-grouping.css") layer;
+
+@import url("layer-rule-grouping.css") layer(named);
diff --git a/test/fixtures/layer-rule-grouping.expected.css b/test/fixtures/layer-rule-grouping.expected.css
new file mode 100644
index 0000000..a4e34d0
--- /dev/null
+++ b/test/fixtures/layer-rule-grouping.expected.css
@@ -0,0 +1,88 @@
+@media screen {
+
+rule-one {}
+
+rule-two {}
+}
+
+@media screen and (min-width: 50rem) {
+  rule-three {}
+
+  rule-four {}
+}
+
+@media screen {
+
+rule-five {}
+
+rule-six {}
+}
+
+@layer import-anon-layer-0 {
+
+rule-one {}
+
+rule-two {}
+}
+
+@layer import-anon-layer-0 {
+
+@media (min-width: 50rem) {
+  rule-three {}
+
+  rule-four {}
+}
+}
+
+@layer import-anon-layer-0 {
+
+rule-five {}
+
+rule-six {}
+}
+
+@layer import-anon-layer-1 {
+
+rule-one {}
+
+rule-two {}
+}
+
+@layer import-anon-layer-1 {
+
+@media (min-width: 50rem) {
+  rule-three {}
+
+  rule-four {}
+}
+}
+
+@layer import-anon-layer-1 {
+
+rule-five {}
+
+rule-six {}
+}
+
+@layer named {
+
+rule-one {}
+
+rule-two {}
+}
+
+@layer named {
+
+@media (min-width: 50rem) {
+  rule-three {}
+
+  rule-four {}
+}
+}
+
+@layer named {
+
+rule-five {}
+
+rule-six {}
+}
diff --git a/test/fixtures/layer.css b/test/fixtures/layer.css
index 1d1c59a..b9894da 100644
--- a/test/fixtures/layer.css
+++ b/test/fixtures/layer.css
@@ -1,9 +1,9 @@
 @layer layer-alpha, layer-beta.one;
 
-@import "foo.css" layer;
+@import "foo.css" layer(foo);
 @import 'bar.css' layer(bar);
 @import 'bar.css' layer(bar) level-1 and level-2;
-@import url(baz.css) layer;
+@import url(baz.css) layer(baz);
 @import url("foobar.css") layer(foobar);
 @import url("foo-layered.css") layer(foo-layered);
 
diff --git a/test/fixtures/layer.expected.css b/test/fixtures/layer.expected.css
index 0a18986..bc73a55 100644
--- a/test/fixtures/layer.expected.css
+++ b/test/fixtures/layer.expected.css
@@ -1,5 +1,5 @@
 @layer layer-alpha, layer-beta.one;
-@layer{
+@layer foo{
 foo{}
 }
 @layer bar{
@@ -10,7 +10,7 @@ bar{}
 bar{}
 }
 }
-@layer{
+@layer baz{
 baz{}
 }
 @layer foobar{
diff --git a/test/layer.js b/test/layer.js
index 1b50f4c..69d27af 100644
--- a/test/layer.js
+++ b/test/layer.js
@@ -1,8 +1,80 @@
 "use strict"
 // external tooling
 const test = require("ava")
+const postcss = require("postcss")
+
+const crypto = require("crypto")
+const cwd = process.cwd()
+
+// plugin
+const atImport = require("..")
 
 // internal tooling
 const checkFixture = require("./helpers/check-fixture")
 
 test("should resolve layers of import statements", checkFixture, "layer")
+
+test(
+  "should correctly wrap imported at rules in layers",
+  checkFixture,
+  "layer-import-atrules",
+  {
+    nameLayer: hashLayerName,
+  }
+)
+
+test(
+  "should correctly wrap imported at rules in anonymous layers",
+  checkFixture,
+  "layer-import-atrules-anonymous",
+  {
+    nameLayer: hashLayerName,
+  }
+)
+
+test("should group rules", checkFixture, "layer-rule-grouping", {
+  nameLayer: hashLayerName,
+})
+
+test("should pass the root file name to the nameLayer function", t => {
+  return postcss()
+    .use(atImport({ path: "test/fixtures/imports", nameLayer: hashLayerName }))
+    .process('@import "foo.css" layer;', { from: "layer.css" })
+    .then(result => {
+      t.is(result.css, `@layer import-anon-layer-52ff1597784c{\nfoo{}\n}`)
+    })
+})
+
+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"))
+})
+
+test("should throw when using anonymous layers without the nameLayer plugin option", t => {
+  return postcss()
+    .use(atImport({ path: "test/fixtures/imports" }))
+    .process('@import "foo.css" layer;', { from: undefined })
+    .catch(err => {
+      t.is(
+        err.message,
+        'postcss-import: <css input>:1:1: When using anonymous layers in @import you must also set the "nameLayer" plugin option'
+      )
+    })
+})
+
+function hashLayerName(index, rootFilename) {
+  if (!rootFilename) {
+    return `import-anon-layer-${index}`
+  }
+
+  // A stable, deterministic and unique layer name:
+  // - layer index
+  // - relative rootFilename to current working directory
+  return `import-anon-layer-${crypto
+    .createHash("sha256")
+    .update(`${index}-${rootFilename.split(cwd)[1]}`)
+    .digest("hex")
+    .slice(0, 12)}`
+}