Skip to content

Commit

Permalink
fix #2936: compare url() tokens by import record
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Feb 20, 2023
1 parent 3765e88 commit 47e54fe
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 52 deletions.
21 changes: 21 additions & 0 deletions CHANGELOG.md
View file
@@ -1,5 +1,26 @@
# Changelog

## Unreleased

* Fix cross-file CSS rule deduplication involving `url()` tokens ([#2936](https://github.com/evanw/esbuild/issues/2936))

Previously cross-file CSS rule deduplication didn't handle `url()` tokens correctly. These tokens contain references to import paths which may be internal (i.e. in the bundle) or external (i.e. not in the bundle). When comparing two `url()` tokens for equality, the underlying import paths should be compared instead of their references. This release of esbuild fixes `url()` token comparisons. One side effect is that `@font-face` rules should now be deduplicated correctly across files:

```css
/* Original code */
@import "data:text/css, \
@import 'http://example.com/style.css'; \
@font-face { src: url(http://example.com/font.ttf) }";
@import "data:text/css, \
@font-face { src: url(http://example.com/font.ttf) }";
/* Old output (with --bundle --minify) */
@import"http://example.com/style.css";@font-face{src:url(http://example.com/font.ttf)}@font-face{src:url(http://example.com/font.ttf)}

/* New output (with --bundle --minify) */
@import"http://example.com/style.css";@font-face{src:url(http://example.com/font.ttf)}
```

## 0.17.9

* Parse rest bindings in TypeScript types ([#2937](https://github.com/evanw/esbuild/issues/2937))
Expand Down
6 changes: 6 additions & 0 deletions internal/bundler_tests/bundler_css_test.go
View file
Expand Up @@ -775,6 +775,11 @@ func TestDeduplicateRules(t *testing.T) {
"/across-files-0.css": "a { color: red; color: red }",
"/across-files-1.css": "a { color: green }",
"/across-files-2.css": "a { color: red }",

"/across-files-url.css": "@import 'across-files-url-0.css'; @import 'across-files-url-1.css'; @import 'across-files-url-2.css';",
"/across-files-url-0.css": "@import 'http://example.com/some.css'; @font-face { src: url(http://example.com/some.font); }",
"/across-files-url-1.css": "@font-face { src: url(http://example.com/some.other.font); }",
"/across-files-url-2.css": "@font-face { src: url(http://example.com/some.font); }",
},
entryPaths: []string{
"/yes0.css",
Expand All @@ -790,6 +795,7 @@ func TestDeduplicateRules(t *testing.T) {
"/no6.css",

"/across-files.css",
"/across-files-url.css",
},
options: config.Options{
Mode: config.ModeBundle,
Expand Down
17 changes: 17 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_css.txt
View file
Expand Up @@ -327,6 +327,23 @@ a {

/* across-files.css */

---------- /out/across-files-url.css ----------
@import "http://example.com/some.css";

/* across-files-url-0.css */

/* across-files-url-1.css */
@font-face {
src: url(http://example.com/some.other.font);
}

/* across-files-url-2.css */
@font-face {
src: url(http://example.com/some.font);
}

/* across-files-url.css */

================================================================================
TestExternalImportURLInCSS
---------- /out/entry.css ----------
Expand Down
111 changes: 71 additions & 40 deletions internal/css_ast/css_ast.go
View file
Expand Up @@ -79,26 +79,55 @@ const (
WhitespaceAfter
)

func (a Token) Equal(b Token) bool {
if a.Kind == b.Kind && a.Text == b.Text && a.ImportRecordIndex == b.ImportRecordIndex && a.Whitespace == b.Whitespace {
// This is necessary when comparing tokens between two different files
type CrossFileEqualityCheck struct {
ImportRecordsA []ast.ImportRecord
ImportRecordsB []ast.ImportRecord
}

func (a Token) Equal(b Token, check *CrossFileEqualityCheck) bool {
if a.Kind == b.Kind && a.Text == b.Text && a.Whitespace == b.Whitespace {
// URLs should be compared based on the text of the associated import record
// (which is what will actually be printed) instead of the original text
if a.Kind == css_lexer.TURL {
if check == nil {
// If both tokens are in the same file, just compare the index
if a.ImportRecordIndex != b.ImportRecordIndex {
return false
}
} else {
// If the tokens come from separate files, compare the import records
// themselves instead of comparing the indices. This can happen when
// the linker runs a "DuplicateRuleRemover" during bundling. This
// doesn't compare the source indices because at this point during
// linking, paths inside the bundle (e.g. due to the "copy" loader)
// should have already been converted into text (e.g. the "unique key"
// string).
if check.ImportRecordsA[a.ImportRecordIndex].Path.Text !=
check.ImportRecordsB[b.ImportRecordIndex].Path.Text {
return false
}
}
}

if a.Children == nil && b.Children == nil {
return true
}

if a.Children != nil && b.Children != nil && TokensEqual(*a.Children, *b.Children) {
if a.Children != nil && b.Children != nil && TokensEqual(*a.Children, *b.Children, check) {
return true
}
}

return false
}

func TokensEqual(a []Token, b []Token) bool {
func TokensEqual(a []Token, b []Token, check *CrossFileEqualityCheck) bool {
if len(a) != len(b) {
return false
}
for i, c := range a {
if !c.Equal(b[i]) {
for i, ai := range a {
if !ai.Equal(b[i], check) {
return false
}
}
Expand All @@ -110,7 +139,9 @@ func HashTokens(hash uint32, tokens []Token) uint32 {

for _, t := range tokens {
hash = helpers.HashCombine(hash, uint32(t.Kind))
hash = helpers.HashCombineString(hash, t.Text)
if t.Kind != css_lexer.TURL {
hash = helpers.HashCombineString(hash, t.Text)
}
if t.Children != nil {
hash = HashTokens(hash, *t.Children)
}
Expand Down Expand Up @@ -262,16 +293,16 @@ type Rule struct {
}

type R interface {
Equal(rule R) bool
Equal(rule R, check *CrossFileEqualityCheck) bool
Hash() (uint32, bool)
}

func RulesEqual(a []Rule, b []Rule) bool {
func RulesEqual(a []Rule, b []Rule, check *CrossFileEqualityCheck) bool {
if len(a) != len(b) {
return false
}
for i, c := range a {
if !c.Data.Equal(b[i].Data) {
for i, ai := range a {
if !ai.Data.Equal(b[i].Data, check) {
return false
}
}
Expand All @@ -294,7 +325,7 @@ type RAtCharset struct {
Encoding string
}

func (a *RAtCharset) Equal(rule R) bool {
func (a *RAtCharset) Equal(rule R, check *CrossFileEqualityCheck) bool {
b, ok := rule.(*RAtCharset)
return ok && a.Encoding == b.Encoding
}
Expand All @@ -310,7 +341,7 @@ type RAtImport struct {
ImportRecordIndex uint32
}

func (*RAtImport) Equal(rule R) bool {
func (*RAtImport) Equal(rule R, check *CrossFileEqualityCheck) bool {
return false
}

Expand All @@ -329,7 +360,7 @@ type KeyframeBlock struct {
Rules []Rule
}

func (a *RAtKeyframes) Equal(rule R) bool {
func (a *RAtKeyframes) Equal(rule R, check *CrossFileEqualityCheck) bool {
if b, ok := rule.(*RAtKeyframes); ok && a.AtToken == b.AtToken && a.Name == b.Name && len(a.Blocks) == len(b.Blocks) {
for i, ai := range a.Blocks {
bi := b.Blocks[i]
Expand All @@ -341,7 +372,7 @@ func (a *RAtKeyframes) Equal(rule R) bool {
return false
}
}
if !RulesEqual(ai.Rules, bi.Rules) {
if !RulesEqual(ai.Rules, bi.Rules, check) {
return false
}
}
Expand Down Expand Up @@ -371,9 +402,9 @@ type RKnownAt struct {
Rules []Rule
}

func (a *RKnownAt) Equal(rule R) bool {
func (a *RKnownAt) Equal(rule R, check *CrossFileEqualityCheck) bool {
b, ok := rule.(*RKnownAt)
return ok && a.AtToken == b.AtToken && TokensEqual(a.Prelude, b.Prelude) && RulesEqual(a.Rules, b.Rules)
return ok && a.AtToken == b.AtToken && TokensEqual(a.Prelude, b.Prelude, check) && RulesEqual(a.Rules, b.Rules, check)
}

func (r *RKnownAt) Hash() (uint32, bool) {
Expand All @@ -390,9 +421,9 @@ type RUnknownAt struct {
Block []Token
}

func (a *RUnknownAt) Equal(rule R) bool {
func (a *RUnknownAt) Equal(rule R, check *CrossFileEqualityCheck) bool {
b, ok := rule.(*RUnknownAt)
return ok && a.AtToken == b.AtToken && TokensEqual(a.Prelude, b.Prelude) && TokensEqual(a.Block, b.Block)
return ok && a.AtToken == b.AtToken && TokensEqual(a.Prelude, b.Prelude, check) && TokensEqual(a.Block, b.Block, check)
}

func (r *RUnknownAt) Hash() (uint32, bool) {
Expand All @@ -409,15 +440,15 @@ type RSelector struct {
HasAtNest bool
}

func (a *RSelector) Equal(rule R) bool {
func (a *RSelector) Equal(rule R, check *CrossFileEqualityCheck) bool {
b, ok := rule.(*RSelector)
if ok && len(a.Selectors) == len(b.Selectors) && a.HasAtNest == b.HasAtNest {
for i, sel := range a.Selectors {
if !sel.Equal(b.Selectors[i]) {
for i, ai := range a.Selectors {
if !ai.Equal(b.Selectors[i], check) {
return false
}
}
return RulesEqual(a.Rules, b.Rules)
return RulesEqual(a.Rules, b.Rules, check)
}

return false
Expand Down Expand Up @@ -450,9 +481,9 @@ type RQualified struct {
Rules []Rule
}

func (a *RQualified) Equal(rule R) bool {
func (a *RQualified) Equal(rule R, check *CrossFileEqualityCheck) bool {
b, ok := rule.(*RQualified)
return ok && TokensEqual(a.Prelude, b.Prelude) && RulesEqual(a.Rules, b.Rules)
return ok && TokensEqual(a.Prelude, b.Prelude, check) && RulesEqual(a.Rules, b.Rules, check)
}

func (r *RQualified) Hash() (uint32, bool) {
Expand All @@ -470,9 +501,9 @@ type RDeclaration struct {
Important bool
}

func (a *RDeclaration) Equal(rule R) bool {
func (a *RDeclaration) Equal(rule R, check *CrossFileEqualityCheck) bool {
b, ok := rule.(*RDeclaration)
return ok && a.KeyText == b.KeyText && TokensEqual(a.Value, b.Value) && a.Important == b.Important
return ok && a.KeyText == b.KeyText && TokensEqual(a.Value, b.Value, check) && a.Important == b.Important
}

func (r *RDeclaration) Hash() (uint32, bool) {
Expand Down Expand Up @@ -500,9 +531,9 @@ type RBadDeclaration struct {
Tokens []Token
}

func (a *RBadDeclaration) Equal(rule R) bool {
func (a *RBadDeclaration) Equal(rule R, check *CrossFileEqualityCheck) bool {
b, ok := rule.(*RBadDeclaration)
return ok && TokensEqual(a.Tokens, b.Tokens)
return ok && TokensEqual(a.Tokens, b.Tokens, check)
}

func (r *RBadDeclaration) Hash() (uint32, bool) {
Expand All @@ -515,7 +546,7 @@ type RComment struct {
Text string
}

func (a *RComment) Equal(rule R) bool {
func (a *RComment) Equal(rule R, check *CrossFileEqualityCheck) bool {
b, ok := rule.(*RComment)
return ok && a.Text == b.Text
}
Expand All @@ -531,7 +562,7 @@ type RAtLayer struct {
Rules []Rule
}

func (a *RAtLayer) Equal(rule R) bool {
func (a *RAtLayer) Equal(rule R, check *CrossFileEqualityCheck) bool {
if b, ok := rule.(*RAtLayer); ok && len(a.Names) == len(b.Names) && len(a.Rules) == len(b.Rules) {
for i, ai := range a.Names {
bi := b.Names[i]
Expand All @@ -544,7 +575,7 @@ func (a *RAtLayer) Equal(rule R) bool {
}
}
}
if !RulesEqual(a.Rules, b.Rules) {
if !RulesEqual(a.Rules, b.Rules, check) {
return false
}
}
Expand All @@ -568,7 +599,7 @@ type ComplexSelector struct {
Selectors []CompoundSelector
}

func (a ComplexSelector) Equal(b ComplexSelector) bool {
func (a ComplexSelector) Equal(b ComplexSelector, check *CrossFileEqualityCheck) bool {
if len(a.Selectors) != len(b.Selectors) {
return false
}
Expand All @@ -589,7 +620,7 @@ func (a ComplexSelector) Equal(b ComplexSelector) bool {
return false
}
for j, aj := range ai.SubclassSelectors {
if !aj.Equal(bi.SubclassSelectors[j]) {
if !aj.Equal(bi.SubclassSelectors[j], check) {
return false
}
}
Expand Down Expand Up @@ -632,15 +663,15 @@ func (a NamespacedName) Equal(b NamespacedName) bool {
}

type SS interface {
Equal(ss SS) bool
Equal(ss SS, check *CrossFileEqualityCheck) bool
Hash() uint32
}

type SSHash struct {
Name string
}

func (a *SSHash) Equal(ss SS) bool {
func (a *SSHash) Equal(ss SS, check *CrossFileEqualityCheck) bool {
b, ok := ss.(*SSHash)
return ok && a.Name == b.Name
}
Expand All @@ -655,7 +686,7 @@ type SSClass struct {
Name string
}

func (a *SSClass) Equal(ss SS) bool {
func (a *SSClass) Equal(ss SS, check *CrossFileEqualityCheck) bool {
b, ok := ss.(*SSClass)
return ok && a.Name == b.Name
}
Expand All @@ -673,7 +704,7 @@ type SSAttribute struct {
MatcherModifier byte // Either 0 or one of: 'i' 'I' 's' 'S'
}

func (a *SSAttribute) Equal(ss SS) bool {
func (a *SSAttribute) Equal(ss SS, check *CrossFileEqualityCheck) bool {
b, ok := ss.(*SSAttribute)
return ok && a.NamespacedName.Equal(b.NamespacedName) && a.MatcherOp == b.MatcherOp &&
a.MatcherValue == b.MatcherValue && a.MatcherModifier == b.MatcherModifier
Expand All @@ -693,9 +724,9 @@ type SSPseudoClass struct {
IsElement bool // If true, this is prefixed by "::" instead of ":"
}

func (a *SSPseudoClass) Equal(ss SS) bool {
func (a *SSPseudoClass) Equal(ss SS, check *CrossFileEqualityCheck) bool {
b, ok := ss.(*SSPseudoClass)
return ok && a.Name == b.Name && TokensEqual(a.Args, b.Args) && a.IsElement == b.IsElement
return ok && a.Name == b.Name && TokensEqual(a.Args, b.Args, check) && a.IsElement == b.IsElement
}

func (ss *SSPseudoClass) Hash() uint32 {
Expand Down

0 comments on commit 47e54fe

Please sign in to comment.