css: fix ordering with @layer before @import

evanw · Aug 12, 2023 · f759693 · f759693
1 parent c73f0f8
commit f759693
Show file tree

Hide file tree

Showing 5 changed files with 487 additions and 269 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,30 @@
 
     Previously esbuild's generated names for local names in CSS were only unique within a given entry point (or across all entry points when code splitting was enabled). That meant that building multiple entry points with esbuild could result in local names being renamed to the same identifier even when those entry points were built simultaneously within a single esbuild API call. This problem was especially likely to happen with minification enabled. With this release, esbuild will now avoid renaming local names from two separate entry points to the same name if those entry points were built with a single esbuild API call, even when code splitting is disabled.
 
+* Fix CSS ordering bug with `@layer` before `@import`
+
+    CSS lets you put `@layer` rules before `@import` rules to define the order of layers in a stylesheet. Previously esbuild's CSS bundler incorrectly ordered these after the imported files because before the introduction of cascade layers to CSS, imported files could be bundled by removing the `@import` rules and then joining files together in the right order. But with `@layer`, CSS files may now need to be split apart into multiple pieces in the bundle. For example:
+
+    ```
+    /* Original code */
+    @layer start;
+    @import "data:text/css,@layer inner.start;";
+    @import "data:text/css,@layer inner.end;";
+    @layer end;
+
+    /* Old output (with --bundle) */
+    @layer inner.start;
+    @layer inner.end;
+    @layer start;
+    @layer end;
+
+    /* New output (with --bundle) */
+    @layer start;
+    @layer inner.start;
+    @layer inner.end;
+    @layer end;
+    ```
+
 * Unwrap nested duplicate `@media` rules ([#3226](https://github.com/evanw/esbuild/issues/3226))
 
     With this release, esbuild's CSS minifier will now automatically unwrap duplicate nested `@media` rules:

diff --git a/internal/bundler_tests/bundler_css_test.go b/internal/bundler_tests/bundler_css_test.go
@@ -2491,3 +2491,49 @@ url-token-whitespace-eof.css: WARNING: Expected ";" but found end of file
 `,
 	})
 }
+
+func TestCSSAtLayerBeforeImportNoBundle(t *testing.T) {
+	css_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.css": `
+				@layer layer1, layer2.layer3;
+				@import "a.css";
+				@import "b.css";
+				@layer layer6.layer7, layer8;
+			`,
+		},
+		entryPaths: []string{"/entry.css"},
+		options: config.Options{
+			Mode:         config.ModePassThrough,
+			AbsOutputDir: "/out",
+		},
+	})
+}
+
+func TestCSSAtLayerBeforeImportBundle(t *testing.T) {
+	css_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.css": `
+				@layer layer1, layer2.layer3;
+				@import "a.css";
+				@import "b.css";
+				@layer layer6.layer7, layer8;
+			`,
+			"/a.css": `
+				@layer layer4 {
+					a { color: red }
+				}
+			`,
+			"/b.css": `
+				@layer layer5 {
+					b { color: red }
+				}
+			`,
+		},
+		entryPaths: []string{"/entry.css"},
+		options: config.Options{
+			Mode:         config.ModeBundle,
+			AbsOutputDir: "/out",
+		},
+	})
+}
diff --git a/internal/bundler_tests/snapshots/snapshots_css.txt b/internal/bundler_tests/snapshots/snapshots_css.txt
@@ -89,7 +89,6 @@ TestCSSAtImport
 ================================================================================
 TestCSSAtImportConditionsAtLayerBundle
 ---------- /out/case1.css ----------
-/* case1-foo.css */
 @layer first.one;
 
 /* case1-foo.css */
@@ -109,7 +108,6 @@ TestCSSAtImportConditionsAtLayerBundle
 /* case1.css */
 
 ---------- /out/case2.css ----------
-/* case2-foo.css */
 @layer first.one;
 
 /* case2-bar.css */
@@ -146,7 +144,6 @@ TestCSSAtImportConditionsAtLayerBundle
 /* case3.css */
 
 ---------- /out/case4.css ----------
-/* case4-foo.css */
 @layer first {
   @layer one, one.two, one.three.four;
 }
@@ -197,7 +194,6 @@ TestCSSAtImportConditionsAtLayerBundle
 /* case5.css */
 
 ---------- /out/case6.css ----------
-/* case6-foo.css */
 @layer first;
 
 /* case6-foo.css */
@@ -235,7 +231,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerInFile
 /* case1.css */
 
 ---------- /out/case2.css ----------
-/* a.css */
 @layer first;
 
 /* b.css */
@@ -255,10 +250,7 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerInFile
 /* case2.css */
 
 ---------- /out/case3.css ----------
-/* a.css */
 @layer first;
-
-/* b.css */
 @layer last;
 
 /* a.css */
@@ -278,7 +270,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerInFile
 /* case3.css */
 
 ---------- /out/case4.css ----------
-/* a.css */
 @layer first;
 
 /* b.css */
@@ -298,10 +289,7 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerInFile
 /* case4.css */
 
 ---------- /out/case5.css ----------
-/* a.css */
 @layer first;
-
-/* b.css */
 @layer last;
 
 /* a.css */
@@ -321,7 +309,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerInFile
 /* case5.css */
 
 ---------- /out/case6.css ----------
-/* a.css */
 @layer first;
 
 /* b.css */
@@ -353,7 +340,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerOnImport
 /* case1.css */
 
 ---------- /out/case2.css ----------
-/* a.css */
 @layer first;
 
 /* b.css */
@@ -373,10 +359,7 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerOnImport
 /* case2.css */
 
 ---------- /out/case3.css ----------
-/* a.css */
 @layer first;
-
-/* b.css */
 @layer last;
 
 /* a.css */
@@ -396,7 +379,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerOnImport
 /* case3.css */
 
 ---------- /out/case4.css ----------
-/* a.css */
 @layer first;
 
 /* b.css */
@@ -416,10 +398,7 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerOnImport
 /* case4.css */
 
 ---------- /out/case5.css ----------
-/* a.css */
 @layer first;
-
-/* b.css */
 @layer last;
 
 /* a.css */
@@ -439,7 +418,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerOnImport
 /* case5.css */
 
 ---------- /out/case6.css ----------
-/* a.css */
 @layer first;
 
 /* b.css */
@@ -734,7 +712,6 @@ TestCSSAtImportConditionsFromExternalRepo
 /* at-keyframes/001/style.css */
 
 ---------- /out/at-layer/001/style.css ----------
-/* at-layer/001/a.css */
 @layer a;
 
 /* at-layer/001/b.css */
@@ -754,7 +731,6 @@ TestCSSAtImportConditionsFromExternalRepo
 /* at-layer/001/style.css */
 
 ---------- /out/at-layer/002/style.css ----------
-/* at-layer/002/a.css */
 @media print {
   @layer a;
 }
@@ -776,7 +752,6 @@ TestCSSAtImportConditionsFromExternalRepo
 /* at-layer/002/style.css */
 
 ---------- /out/at-layer/003/style.css ----------
-/* at-layer/003/a.css */
 @layer a;
 
 /* at-layer/003/b.css */
@@ -1609,6 +1584,36 @@ TestCSSAtImportExternal
 
 /* entry.css */
 
+================================================================================
+TestCSSAtLayerBeforeImportBundle
+---------- /out/entry.css ----------
+@layer layer1, layer2.layer3;
+
+/* a.css */
+@layer layer4 {
+  a {
+    color: red;
+  }
+}
+
+/* b.css */
+@layer layer5 {
+  b {
+    color: red;
+  }
+}
+
+/* entry.css */
+@layer layer6.layer7, layer8;
+
+================================================================================
+TestCSSAtLayerBeforeImportNoBundle
+---------- /out/entry.css ----------
+@layer layer1, layer2.layer3;
+@import "a.css";
+@import "b.css";
+@layer layer6.layer7, layer8;
+
 ================================================================================
 TestCSSEntryPoint
 ---------- /out.css ----------

diff --git a/internal/helpers/strings.go b/internal/helpers/strings.go
@@ -17,6 +17,18 @@ func StringArraysEqual(a []string, b []string) bool {
 	return true
 }
 
+func StringArrayArraysEqual(a [][]string, b [][]string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i, x := range a {
+		if !StringArraysEqual(x, b[i]) {
+			return false
+		}
+	}
+	return true
+}
+
 func StringArrayToQuotedCommaSeparatedString(a []string) string {
 	sb := strings.Builder{}
 	for i, str := range a {