Skip to content

Commit

Permalink
minify: remove unnecessary & selectors
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Mar 25, 2023
1 parent 0546cf7 commit cd62fa1
Show file tree
Hide file tree
Showing 7 changed files with 283 additions and 10 deletions.
23 changes: 23 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,29 @@

## Unreleased

* Minification now removes unnecessary `&` CSS nesting selectors

This release introduces the following CSS minification optimizations:

```css
/* Original input */
a {
font-weight: bold;
& {
color: blue;
}
& :hover {
text-decoration: underline;
}
}

/* Old output (with --minify) */
a{font-weight:700;&{color:#00f}& :hover{text-decoration:underline}}

/* New output (with --minify) */
a{font-weight:700;:hover{text-decoration:underline}color:#00f}
```

* Minification now removes duplicates from CSS selector lists

This release introduces the following CSS minification optimization:
Expand Down
35 changes: 33 additions & 2 deletions internal/bundler_tests/bundler_css_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,17 @@ func TestCSSNestingOldBrowser(t *testing.T) {
"/toplevel-hash.css": `#id { color: red; }`,
"/toplevel-plus.css": `+ b { color: red; }`,
"/toplevel-tilde.css": `~ b { color: red; }`,

"/media-ampersand-twice.css": `@media screen { &, & { color: red; } }`,
"/media-ampersand-first.css": `@media screen { &, a { color: red; } }`,
"/media-ampersand-second.css": `@media screen { a, & { color: red; } }`,
"/media-attribute.css": `@media screen { [href] { color: red; } }`,
"/media-colon.css": `@media screen { :hover { color: red; } }`,
"/media-dot.css": `@media screen { .cls { color: red; } }`,
"/media-greaterthan.css": `@media screen { > b { color: red; } }`,
"/media-hash.css": `@media screen { #id { color: red; } }`,
"/media-plus.css": `@media screen { + b { color: red; } }`,
"/media-tilde.css": `@media screen { ~ b { color: red; } }`,
},
entryPaths: []string{
"/[email protected]",
Expand All @@ -746,14 +757,32 @@ func TestCSSNestingOldBrowser(t *testing.T) {
"/toplevel-hash.css",
"/toplevel-plus.css",
"/toplevel-tilde.css",

"/media-ampersand-twice.css",
"/media-ampersand-first.css",
"/media-ampersand-second.css",
"/media-attribute.css",
"/media-colon.css",
"/media-dot.css",
"/media-greaterthan.css",
"/media-hash.css",
"/media-plus.css",
"/media-tilde.css",
},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
UnsupportedCSSFeatures: compat.Nesting,
OriginalTargetEnv: "chrome10",
},
expectedScanLog: `[email protected]: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
expectedScanLog: `media-ampersand-first.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-ampersand-second.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-ampersand-twice.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-ampersand-twice.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-greaterthan.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-plus.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-tilde.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
[email protected]: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
[email protected]: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-ampersand-first.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-ampersand-twice.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
Expand Down Expand Up @@ -821,8 +850,9 @@ func TestDeduplicateRules(t *testing.T) {
"/yes0.css": "a { color: red; color: green; color: red }",
"/yes1.css": "a { color: red } a { color: green } a { color: red }",
"/yes2.css": "@media screen { a { color: red } } @media screen { a { color: red } }",
"/yes3.css": "@media screen { a { color: red } } @media screen { & a { color: red } }",

"/no0.css": "@media screen { a { color: red } } @media screen { & a { color: red } }",
"/no0.css": "@media screen { a { color: red } } @media screen { &a { color: red } }",
"/no1.css": "@media screen { a { color: red } } @media screen { a[x] { color: red } }",
"/no2.css": "@media screen { a { color: red } } @media screen { a.x { color: red } }",
"/no3.css": "@media screen { a { color: red } } @media screen { a#x { color: red } }",
Expand All @@ -844,6 +874,7 @@ func TestDeduplicateRules(t *testing.T) {
"/yes0.css",
"/yes1.css",
"/yes2.css",
"/yes3.css",

"/no0.css",
"/no1.css",
Expand Down
93 changes: 92 additions & 1 deletion internal/bundler_tests/snapshots/snapshots_css.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,89 @@ a,
color: red;
}

---------- /out/media-ampersand-twice.css ----------
/* media-ampersand-twice.css */
@media screen {
&,
& {
color: red;
}
}

---------- /out/media-ampersand-first.css ----------
/* media-ampersand-first.css */
@media screen {
&,
a {
color: red;
}
}

---------- /out/media-ampersand-second.css ----------
/* media-ampersand-second.css */
@media screen {
a,
& {
color: red;
}
}

---------- /out/media-attribute.css ----------
/* media-attribute.css */
@media screen {
[href] {
color: red;
}
}

---------- /out/media-colon.css ----------
/* media-colon.css */
@media screen {
:hover {
color: red;
}
}

---------- /out/media-dot.css ----------
/* media-dot.css */
@media screen {
.cls {
color: red;
}
}

---------- /out/media-greaterthan.css ----------
/* media-greaterthan.css */
@media screen {
> b {
color: red;
}
}

---------- /out/media-hash.css ----------
/* media-hash.css */
@media screen {
#id {
color: red;
}
}

---------- /out/media-plus.css ----------
/* media-plus.css */
@media screen {
+ b {
color: red;
}
}

---------- /out/media-tilde.css ----------
/* media-tilde.css */
@media screen {
~ b {
color: red;
}
}

================================================================================
TestDataURLImportURLInCSS
---------- /out/entry.css ----------
Expand Down Expand Up @@ -367,6 +450,14 @@ a {
}
}

---------- /out/yes3.css ----------
/* yes3.css */
@media screen {
a {
color: red;
}
}

---------- /out/no0.css ----------
/* no0.css */
@media screen {
Expand All @@ -375,7 +466,7 @@ a {
}
}
@media screen {
& a {
&a {
color: red;
}
}
Expand Down
4 changes: 4 additions & 0 deletions internal/css_ast/css_ast.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,10 @@ type CompoundSelector struct {
HasNestingSelector bool // "&"
}

func (sel CompoundSelector) IsSingleAmpersand() bool {
return sel.HasNestingSelector && sel.Combinator == 0 && sel.TypeSelector == nil && len(sel.SubclassSelectors) == 0
}

type NameToken struct {
Text string
Kind css_lexer.T
Expand Down
29 changes: 25 additions & 4 deletions internal/css_parser/css_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ loop:
}

if context.parseSelectors {
rules = append(rules, p.parseSelectorRuleFrom(p.index, parseSelectorOpts{isTopLevel: context.isTopLevel}))
rules = append(rules, p.parseSelectorRuleFrom(p.index, context.isTopLevel, parseSelectorOpts{}))
} else {
rules = append(rules, p.parseQualifiedRuleFrom(p.index, parseQualifiedRuleOpts{isTopLevel: context.isTopLevel}))
}
Expand All @@ -297,6 +297,8 @@ loop:

func (p *parser) parseListOfDeclarations() (list []css_ast.Rule) {
list = []css_ast.Rule{}
foundNesting := false

for {
switch p.current().Kind {
case css_lexer.TWhitespace, css_lexer.TSemicolon:
Expand All @@ -306,6 +308,24 @@ func (p *parser) parseListOfDeclarations() (list []css_ast.Rule) {
list = p.processDeclarations(list)
if p.options.MinifySyntax {
list = p.mangleRules(list, false /* isTopLevel */)

// Pull out all unnecessarily-nested declarations and stick them at the end
// "a { & { b: c } d: e }" => "a { d: e; b: c; }"
if foundNesting {
var inlineDecls []css_ast.Rule
n := 0
for _, rule := range list {
if rule, ok := rule.Data.(*css_ast.RSelector); ok && len(rule.Selectors) == 1 {
if sel := rule.Selectors[0]; len(sel.Selectors) == 1 && sel.Selectors[0].IsSingleAmpersand() {
inlineDecls = append(inlineDecls, rule.Rules...)
continue
}
}
list[n] = rule
n++
}
list = append(list[:n], inlineDecls...)
}
}
return

Expand All @@ -327,7 +347,8 @@ func (p *parser) parseListOfDeclarations() (list []css_ast.Rule) {
css_lexer.TDelimGreaterThan,
css_lexer.TDelimTilde:
p.maybeWarnAboutNesting(p.current().Range)
list = append(list, p.parseSelectorRuleFrom(p.index, parseSelectorOpts{}))
list = append(list, p.parseSelectorRuleFrom(p.index, false, parseSelectorOpts{isDeclarationContext: true}))
foundNesting = true

default:
list = append(list, p.parseDeclaration())
Expand Down Expand Up @@ -1600,7 +1621,7 @@ func mangleNumber(t string) (string, bool) {
return t, t != original
}

func (p *parser) parseSelectorRuleFrom(preludeStart int, opts parseSelectorOpts) css_ast.Rule {
func (p *parser) parseSelectorRuleFrom(preludeStart int, isTopLevel bool, opts parseSelectorOpts) css_ast.Rule {
// Try parsing the prelude as a selector list
if list, ok := p.parseSelectorList(opts); ok {
selector := css_ast.RSelector{Selectors: list}
Expand All @@ -1615,7 +1636,7 @@ func (p *parser) parseSelectorRuleFrom(preludeStart int, opts parseSelectorOpts)
// Otherwise, parse a generic qualified rule
return p.parseQualifiedRuleFrom(preludeStart, parseQualifiedRuleOpts{
isAlreadyInvalid: true,
isTopLevel: opts.isTopLevel,
isTopLevel: isTopLevel,
})
}

Expand Down
60 changes: 57 additions & 3 deletions internal/css_parser/css_parser_selector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,74 @@ skip:
list = append(list, sel)
}

if p.options.MinifySyntax {
for i := 1; i < len(list); i++ {
if analyzeLeadingAmpersand(list[i], opts.isDeclarationContext) != cannotRemoveLeadingAmpersand {
list[i].Selectors = list[i].Selectors[1:]
}
}

switch analyzeLeadingAmpersand(list[0], opts.isDeclarationContext) {
case canAlwaysRemoveLeadingAmpersand:
list[0].Selectors = list[0].Selectors[1:]

case canRemoveLeadingAmpersandIfNotFirst:
for i := 1; i < len(list); i++ {
if sel := list[i].Selectors[0]; !sel.HasNestingSelector && (sel.Combinator != 0 || sel.TypeSelector == nil) {
list[0].Selectors = list[0].Selectors[1:]
list[0], list[i] = list[i], list[0]
break
}
}
}
}

ok = true
return
}

type leadingAmpersand uint8

const (
cannotRemoveLeadingAmpersand leadingAmpersand = iota
canAlwaysRemoveLeadingAmpersand
canRemoveLeadingAmpersandIfNotFirst
)

func analyzeLeadingAmpersand(sel css_ast.ComplexSelector, isDeclarationContext bool) leadingAmpersand {
if len(sel.Selectors) > 1 {
if first := sel.Selectors[0]; first.IsSingleAmpersand() {
if second := sel.Selectors[1]; second.Combinator == 0 && second.HasNestingSelector {
// ".foo { & &.bar {} }" => ".foo { & &.bar {} }"
} else if second.Combinator != 0 || second.TypeSelector == nil || !isDeclarationContext {
// "& + div {}" => "+ div {}"
// "& div {}" => "div {}"
// ".foo { & + div {} }" => ".foo { + div {} }"
// ".foo { & + &.bar {} }" => ".foo { + &.bar {} }"
// ".foo { & :hover {} }" => ".foo { :hover {} }"
return canAlwaysRemoveLeadingAmpersand
} else {
// ".foo { & div {} }"
// ".foo { .bar, & div {} }" => ".foo { .bar, div {} }"
return canRemoveLeadingAmpersandIfNotFirst
}
}
} else {
// "& {}" => "& {}"
}
return cannotRemoveLeadingAmpersand
}

type parseSelectorOpts struct {
isTopLevel bool
isDeclarationContext bool
}

func (p *parser) parseComplexSelector(opts parseSelectorOpts) (result css_ast.ComplexSelector, ok bool) {
// This is an extension: https://drafts.csswg.org/css-nesting-1/
r := p.current().Range
combinator := p.parseCombinator()
if combinator != 0 {
if opts.isTopLevel {
if !opts.isDeclarationContext {
p.maybeWarnAboutNesting(r)
}
p.eat(css_lexer.TWhitespace)
Expand Down Expand Up @@ -101,7 +155,7 @@ func (p *parser) nameToken() css_ast.NameToken {
func (p *parser) parseCompoundSelector(opts parseSelectorOpts) (sel css_ast.CompoundSelector, ok bool) {
// This is an extension: https://drafts.csswg.org/css-nesting-1/
if p.peek(css_lexer.TDelimAmpersand) {
if opts.isTopLevel {
if !opts.isDeclarationContext {
p.maybeWarnAboutNesting(p.current().Range)
}
sel.HasNestingSelector = true
Expand Down
Loading

0 comments on commit cd62fa1

Please sign in to comment.