Skip to content

Commit

Permalink
fix #3292: avoid generating duplicate css prefixes
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Aug 6, 2023
1 parent 7469ead commit 02e13e0
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 19 deletions.
27 changes: 27 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,33 @@

The hash algorithm (currently [XXH64](https://xxhash.com/)) is implementation-dependent and may be changed at any time in between esbuild versions. If you don't like esbuild's choice of hash algorithm then you are welcome to hash the contents yourself instead. As with any hash algorithm, note that while two different hashes mean that the contents are different, two equal hashes do not necessarily mean that the contents are equal. You may still want to compare the contents in addition to the hashes to detect with certainty when output files have been changed.

* Avoid generating duplicate prefixed declarations in CSS ([#3292](https://github.com/evanw/esbuild/issues/3292))

There was a request for esbuild's CSS prefixer to avoid generating a prefixed declaration if a declaration by that name is already present in the same rule block. So with this release, esbuild will now avoid doing this:

```css
/* Original code */
body {
backdrop-filter: blur(30px);
-webkit-backdrop-filter: blur(45px);
}

/* Old output (with --target=safari12) */
body {
-webkit-backdrop-filter: blur(30px);
backdrop-filter: blur(30px);
-webkit-backdrop-filter: blur(45px);
}

/* New output (with --target=safari12) */
body {
backdrop-filter: blur(30px);
-webkit-backdrop-filter: blur(45px);
}
```

This can result in a visual difference in certain cases (for example if the browser understands `blur(30px)` but not `blur(45px)`, it will be able to fall back to `blur(30px)`). But this change means esbuild now matches the behavior of [Autoprefixer](https://autoprefixer.github.io/) which is probably a good representation of how people expect this feature to work.

## 0.18.18

* Fix asset references with the `--line-limit` flag ([#3286](https://github.com/evanw/esbuild/issues/3286))
Expand Down
39 changes: 20 additions & 19 deletions internal/css_parser/css_decls.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func (p *parser) processDeclarations(rules []css_ast.Rule) (rewrittenRules []css
inset := boxTracker{key: css_ast.DInset, keyText: "inset", allowAuto: true}
borderRadius := borderRadiusTracker{}
rewrittenRules = make([]css_ast.Rule, 0, len(rules))
var declarationKeys map[string]struct{}

// Don't automatically generate the "inset" property if it's not supported
if p.options.unsupportedCSSFeatures.Has(compat.InsetProperty) {
Expand Down Expand Up @@ -281,20 +282,29 @@ func (p *parser) processDeclarations(rules []css_ast.Rule) (rewrittenRules []css
}

if prefixes, ok := p.options.cssPrefixData[decl.Key]; ok {
if declarationKeys == nil {
// Only generate this map if it's needed
declarationKeys = make(map[string]struct{})
for _, rule := range rules {
if decl, ok := rule.Data.(*css_ast.RDeclaration); ok {
declarationKeys[decl.KeyText] = struct{}{}
}
}
}
if (prefixes & compat.WebkitPrefix) != 0 {
rewrittenRules = p.insertPrefixedDeclaration(rewrittenRules, "-webkit-", rule.Loc, decl)
rewrittenRules = p.insertPrefixedDeclaration(rewrittenRules, "-webkit-", rule.Loc, decl, declarationKeys)
}
if (prefixes & compat.KhtmlPrefix) != 0 {
rewrittenRules = p.insertPrefixedDeclaration(rewrittenRules, "-khtml-", rule.Loc, decl)
rewrittenRules = p.insertPrefixedDeclaration(rewrittenRules, "-khtml-", rule.Loc, decl, declarationKeys)
}
if (prefixes & compat.MozPrefix) != 0 {
rewrittenRules = p.insertPrefixedDeclaration(rewrittenRules, "-moz-", rule.Loc, decl)
rewrittenRules = p.insertPrefixedDeclaration(rewrittenRules, "-moz-", rule.Loc, decl, declarationKeys)
}
if (prefixes & compat.MsPrefix) != 0 {
rewrittenRules = p.insertPrefixedDeclaration(rewrittenRules, "-ms-", rule.Loc, decl)
rewrittenRules = p.insertPrefixedDeclaration(rewrittenRules, "-ms-", rule.Loc, decl, declarationKeys)
}
if (prefixes & compat.OPrefix) != 0 {
rewrittenRules = p.insertPrefixedDeclaration(rewrittenRules, "-o-", rule.Loc, decl)
rewrittenRules = p.insertPrefixedDeclaration(rewrittenRules, "-o-", rule.Loc, decl, declarationKeys)
}
}
}
Expand All @@ -314,23 +324,14 @@ func (p *parser) processDeclarations(rules []css_ast.Rule) (rewrittenRules []css
return
}

func (p *parser) insertPrefixedDeclaration(rules []css_ast.Rule, prefix string, loc logger.Loc, decl *css_ast.RDeclaration) []css_ast.Rule {
func (p *parser) insertPrefixedDeclaration(rules []css_ast.Rule, prefix string, loc logger.Loc, decl *css_ast.RDeclaration, declarationKeys map[string]struct{}) []css_ast.Rule {
keyText := prefix + decl.KeyText

// Don't insert a prefixed declaration if there already is one
for i := len(rules) - 2; i >= 0; i-- {
if prev, ok := rules[i].Data.(*css_ast.RDeclaration); ok && prev.Key == css_ast.DUnknown {
if prev.KeyText == keyText {
// We found a previous declaration with a matching prefixed property.
// The value is ignored, which matches the behavior of "autoprefixer".
return rules
}
if p, d := len(prev.KeyText), len(decl.KeyText); p > d && prev.KeyText[p-d-1] == '-' && prev.KeyText[p-d:] == decl.KeyText {
// Continue through a run of prefixed properties with the same name
continue
}
}
break
if _, ok := declarationKeys[keyText]; ok {
// We found a previous declaration with a matching prefixed property.
// The value is ignored, which matches the behavior of "autoprefixer".
return rules
}

// Additional special cases for when the prefix applies
Expand Down
4 changes: 4 additions & 0 deletions internal/css_parser/css_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2292,6 +2292,10 @@ func TestPrefixInsertion(t *testing.T) {
"a {\n -webkit-"+key+": url(x.png);\n "+key+": url(y.png);\n}\n",
"a {\n -webkit-"+key+": url(x.png);\n "+key+": url(y.png);\n}\n", "")

expectPrintedWithAllPrefixes(t,
"a {\n "+key+": url(y.png);\n -webkit-"+key+": url(x.png);\n}\n",
"a {\n "+key+": url(y.png);\n -webkit-"+key+": url(x.png);\n}\n", "")

expectPrintedWithAllPrefixes(t,
"a { "+key+": url(x.png); "+key+": url(y.png) }",
"a {\n -webkit-"+key+": url(x.png);\n "+key+": url(x.png);\n -webkit-"+key+": url(y.png);\n "+key+": url(y.png);\n}\n", "")
Expand Down

0 comments on commit 02e13e0

Please sign in to comment.