Skip to content

Commit

Permalink
css: fix ordering with @layer before @import
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Aug 12, 2023
1 parent c73f0f8 commit f759693
Show file tree
Hide file tree
Showing 5 changed files with 487 additions and 269 deletions.
24 changes: 24 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
46 changes: 46 additions & 0 deletions internal/bundler_tests/bundler_css_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
})
}
55 changes: 30 additions & 25 deletions internal/bundler_tests/snapshots/snapshots_css.txt
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ TestCSSAtImport
================================================================================
TestCSSAtImportConditionsAtLayerBundle
---------- /out/case1.css ----------
/* case1-foo.css */
@layer first.one;

/* case1-foo.css */
Expand All @@ -109,7 +108,6 @@ TestCSSAtImportConditionsAtLayerBundle
/* case1.css */

---------- /out/case2.css ----------
/* case2-foo.css */
@layer first.one;

/* case2-bar.css */
Expand Down Expand Up @@ -146,7 +144,6 @@ TestCSSAtImportConditionsAtLayerBundle
/* case3.css */

---------- /out/case4.css ----------
/* case4-foo.css */
@layer first {
@layer one, one.two, one.three.four;
}
Expand Down Expand Up @@ -197,7 +194,6 @@ TestCSSAtImportConditionsAtLayerBundle
/* case5.css */

---------- /out/case6.css ----------
/* case6-foo.css */
@layer first;

/* case6-foo.css */
Expand Down Expand Up @@ -235,7 +231,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerInFile
/* case1.css */

---------- /out/case2.css ----------
/* a.css */
@layer first;

/* b.css */
Expand All @@ -255,10 +250,7 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerInFile
/* case2.css */

---------- /out/case3.css ----------
/* a.css */
@layer first;

/* b.css */
@layer last;

/* a.css */
Expand All @@ -278,7 +270,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerInFile
/* case3.css */

---------- /out/case4.css ----------
/* a.css */
@layer first;

/* b.css */
Expand All @@ -298,10 +289,7 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerInFile
/* case4.css */

---------- /out/case5.css ----------
/* a.css */
@layer first;

/* b.css */
@layer last;

/* a.css */
Expand All @@ -321,7 +309,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerInFile
/* case5.css */

---------- /out/case6.css ----------
/* a.css */
@layer first;

/* b.css */
Expand Down Expand Up @@ -353,7 +340,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerOnImport
/* case1.css */

---------- /out/case2.css ----------
/* a.css */
@layer first;

/* b.css */
Expand All @@ -373,10 +359,7 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerOnImport
/* case2.css */

---------- /out/case3.css ----------
/* a.css */
@layer first;

/* b.css */
@layer last;

/* a.css */
Expand All @@ -396,7 +379,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerOnImport
/* case3.css */

---------- /out/case4.css ----------
/* a.css */
@layer first;

/* b.css */
Expand All @@ -416,10 +398,7 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerOnImport
/* case4.css */

---------- /out/case5.css ----------
/* a.css */
@layer first;

/* b.css */
@layer last;

/* a.css */
Expand All @@ -439,7 +418,6 @@ TestCSSAtImportConditionsAtLayerBundleAlternatingLayerOnImport
/* case5.css */

---------- /out/case6.css ----------
/* a.css */
@layer first;

/* b.css */
Expand Down Expand Up @@ -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 */
Expand All @@ -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;
}
Expand All @@ -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 */
Expand Down Expand Up @@ -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 ----------
Expand Down
12 changes: 12 additions & 0 deletions internal/helpers/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit f759693

Please sign in to comment.