From 02e13e0854fbb8ebe39774c447adbc04812c898b Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sun, 6 Aug 2023 15:17:00 -0400 Subject: [PATCH] fix #3292: avoid generating duplicate css prefixes --- CHANGELOG.md | 27 ++++++++++++++++++ internal/css_parser/css_decls.go | 39 +++++++++++++------------- internal/css_parser/css_parser_test.go | 4 +++ 3 files changed, 51 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0331e89a6c..638e303b6f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) diff --git a/internal/css_parser/css_decls.go b/internal/css_parser/css_decls.go index 782f54dd66d..0d75f7bd084 100644 --- a/internal/css_parser/css_decls.go +++ b/internal/css_parser/css_decls.go @@ -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) { @@ -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) } } } @@ -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 diff --git a/internal/css_parser/css_parser_test.go b/internal/css_parser/css_parser_test.go index 97069a52180..6c6a71de3e0 100644 --- a/internal/css_parser/css_parser_test.go +++ b/internal/css_parser/css_parser_test.go @@ -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", "")