Skip to content

Commit 47e54fe

Browse files
committed
fix #2936: compare url() tokens by import record
1 parent 3765e88 commit 47e54fe

File tree

6 files changed

+156
-52
lines changed

6 files changed

+156
-52
lines changed

CHANGELOG.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,26 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
* Fix cross-file CSS rule deduplication involving `url()` tokens ([#2936](https://github.com/evanw/esbuild/issues/2936))
6+
7+
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:
8+
9+
```css
10+
/* Original code */
11+
@import "data:text/css, \
12+
@import 'http://example.com/style.css'; \
13+
@font-face { src: url(http://example.com/font.ttf) }";
14+
@import "data:text/css, \
15+
@font-face { src: url(http://example.com/font.ttf) }";
16+
17+
/* Old output (with --bundle --minify) */
18+
@import"http://example.com/style.css";@font-face{src:url(http://example.com/font.ttf)}@font-face{src:url(http://example.com/font.ttf)}
19+
20+
/* New output (with --bundle --minify) */
21+
@import"http://example.com/style.css";@font-face{src:url(http://example.com/font.ttf)}
22+
```
23+
324
## 0.17.9
425

526
* Parse rest bindings in TypeScript types ([#2937](https://github.com/evanw/esbuild/issues/2937))

internal/bundler_tests/bundler_css_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,11 @@ func TestDeduplicateRules(t *testing.T) {
775775
"/across-files-0.css": "a { color: red; color: red }",
776776
"/across-files-1.css": "a { color: green }",
777777
"/across-files-2.css": "a { color: red }",
778+
779+
"/across-files-url.css": "@import 'across-files-url-0.css'; @import 'across-files-url-1.css'; @import 'across-files-url-2.css';",
780+
"/across-files-url-0.css": "@import 'http://example.com/some.css'; @font-face { src: url(http://example.com/some.font); }",
781+
"/across-files-url-1.css": "@font-face { src: url(http://example.com/some.other.font); }",
782+
"/across-files-url-2.css": "@font-face { src: url(http://example.com/some.font); }",
778783
},
779784
entryPaths: []string{
780785
"/yes0.css",
@@ -790,6 +795,7 @@ func TestDeduplicateRules(t *testing.T) {
790795
"/no6.css",
791796

792797
"/across-files.css",
798+
"/across-files-url.css",
793799
},
794800
options: config.Options{
795801
Mode: config.ModeBundle,

internal/bundler_tests/snapshots/snapshots_css.txt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,23 @@ a {
327327

328328
/* across-files.css */
329329

330+
---------- /out/across-files-url.css ----------
331+
@import "http://example.com/some.css";
332+
333+
/* across-files-url-0.css */
334+
335+
/* across-files-url-1.css */
336+
@font-face {
337+
src: url(http://example.com/some.other.font);
338+
}
339+
340+
/* across-files-url-2.css */
341+
@font-face {
342+
src: url(http://example.com/some.font);
343+
}
344+
345+
/* across-files-url.css */
346+
330347
================================================================================
331348
TestExternalImportURLInCSS
332349
---------- /out/entry.css ----------

internal/css_ast/css_ast.go

Lines changed: 71 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -79,26 +79,55 @@ const (
7979
WhitespaceAfter
8080
)
8181

82-
func (a Token) Equal(b Token) bool {
83-
if a.Kind == b.Kind && a.Text == b.Text && a.ImportRecordIndex == b.ImportRecordIndex && a.Whitespace == b.Whitespace {
82+
// This is necessary when comparing tokens between two different files
83+
type CrossFileEqualityCheck struct {
84+
ImportRecordsA []ast.ImportRecord
85+
ImportRecordsB []ast.ImportRecord
86+
}
87+
88+
func (a Token) Equal(b Token, check *CrossFileEqualityCheck) bool {
89+
if a.Kind == b.Kind && a.Text == b.Text && a.Whitespace == b.Whitespace {
90+
// URLs should be compared based on the text of the associated import record
91+
// (which is what will actually be printed) instead of the original text
92+
if a.Kind == css_lexer.TURL {
93+
if check == nil {
94+
// If both tokens are in the same file, just compare the index
95+
if a.ImportRecordIndex != b.ImportRecordIndex {
96+
return false
97+
}
98+
} else {
99+
// If the tokens come from separate files, compare the import records
100+
// themselves instead of comparing the indices. This can happen when
101+
// the linker runs a "DuplicateRuleRemover" during bundling. This
102+
// doesn't compare the source indices because at this point during
103+
// linking, paths inside the bundle (e.g. due to the "copy" loader)
104+
// should have already been converted into text (e.g. the "unique key"
105+
// string).
106+
if check.ImportRecordsA[a.ImportRecordIndex].Path.Text !=
107+
check.ImportRecordsB[b.ImportRecordIndex].Path.Text {
108+
return false
109+
}
110+
}
111+
}
112+
84113
if a.Children == nil && b.Children == nil {
85114
return true
86115
}
87116

88-
if a.Children != nil && b.Children != nil && TokensEqual(*a.Children, *b.Children) {
117+
if a.Children != nil && b.Children != nil && TokensEqual(*a.Children, *b.Children, check) {
89118
return true
90119
}
91120
}
92121

93122
return false
94123
}
95124

96-
func TokensEqual(a []Token, b []Token) bool {
125+
func TokensEqual(a []Token, b []Token, check *CrossFileEqualityCheck) bool {
97126
if len(a) != len(b) {
98127
return false
99128
}
100-
for i, c := range a {
101-
if !c.Equal(b[i]) {
129+
for i, ai := range a {
130+
if !ai.Equal(b[i], check) {
102131
return false
103132
}
104133
}
@@ -110,7 +139,9 @@ func HashTokens(hash uint32, tokens []Token) uint32 {
110139

111140
for _, t := range tokens {
112141
hash = helpers.HashCombine(hash, uint32(t.Kind))
113-
hash = helpers.HashCombineString(hash, t.Text)
142+
if t.Kind != css_lexer.TURL {
143+
hash = helpers.HashCombineString(hash, t.Text)
144+
}
114145
if t.Children != nil {
115146
hash = HashTokens(hash, *t.Children)
116147
}
@@ -262,16 +293,16 @@ type Rule struct {
262293
}
263294

264295
type R interface {
265-
Equal(rule R) bool
296+
Equal(rule R, check *CrossFileEqualityCheck) bool
266297
Hash() (uint32, bool)
267298
}
268299

269-
func RulesEqual(a []Rule, b []Rule) bool {
300+
func RulesEqual(a []Rule, b []Rule, check *CrossFileEqualityCheck) bool {
270301
if len(a) != len(b) {
271302
return false
272303
}
273-
for i, c := range a {
274-
if !c.Data.Equal(b[i].Data) {
304+
for i, ai := range a {
305+
if !ai.Data.Equal(b[i].Data, check) {
275306
return false
276307
}
277308
}
@@ -294,7 +325,7 @@ type RAtCharset struct {
294325
Encoding string
295326
}
296327

297-
func (a *RAtCharset) Equal(rule R) bool {
328+
func (a *RAtCharset) Equal(rule R, check *CrossFileEqualityCheck) bool {
298329
b, ok := rule.(*RAtCharset)
299330
return ok && a.Encoding == b.Encoding
300331
}
@@ -310,7 +341,7 @@ type RAtImport struct {
310341
ImportRecordIndex uint32
311342
}
312343

313-
func (*RAtImport) Equal(rule R) bool {
344+
func (*RAtImport) Equal(rule R, check *CrossFileEqualityCheck) bool {
314345
return false
315346
}
316347

@@ -329,7 +360,7 @@ type KeyframeBlock struct {
329360
Rules []Rule
330361
}
331362

332-
func (a *RAtKeyframes) Equal(rule R) bool {
363+
func (a *RAtKeyframes) Equal(rule R, check *CrossFileEqualityCheck) bool {
333364
if b, ok := rule.(*RAtKeyframes); ok && a.AtToken == b.AtToken && a.Name == b.Name && len(a.Blocks) == len(b.Blocks) {
334365
for i, ai := range a.Blocks {
335366
bi := b.Blocks[i]
@@ -341,7 +372,7 @@ func (a *RAtKeyframes) Equal(rule R) bool {
341372
return false
342373
}
343374
}
344-
if !RulesEqual(ai.Rules, bi.Rules) {
375+
if !RulesEqual(ai.Rules, bi.Rules, check) {
345376
return false
346377
}
347378
}
@@ -371,9 +402,9 @@ type RKnownAt struct {
371402
Rules []Rule
372403
}
373404

374-
func (a *RKnownAt) Equal(rule R) bool {
405+
func (a *RKnownAt) Equal(rule R, check *CrossFileEqualityCheck) bool {
375406
b, ok := rule.(*RKnownAt)
376-
return ok && a.AtToken == b.AtToken && TokensEqual(a.Prelude, b.Prelude) && RulesEqual(a.Rules, b.Rules)
407+
return ok && a.AtToken == b.AtToken && TokensEqual(a.Prelude, b.Prelude, check) && RulesEqual(a.Rules, b.Rules, check)
377408
}
378409

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

393-
func (a *RUnknownAt) Equal(rule R) bool {
424+
func (a *RUnknownAt) Equal(rule R, check *CrossFileEqualityCheck) bool {
394425
b, ok := rule.(*RUnknownAt)
395-
return ok && a.AtToken == b.AtToken && TokensEqual(a.Prelude, b.Prelude) && TokensEqual(a.Block, b.Block)
426+
return ok && a.AtToken == b.AtToken && TokensEqual(a.Prelude, b.Prelude, check) && TokensEqual(a.Block, b.Block, check)
396427
}
397428

398429
func (r *RUnknownAt) Hash() (uint32, bool) {
@@ -409,15 +440,15 @@ type RSelector struct {
409440
HasAtNest bool
410441
}
411442

412-
func (a *RSelector) Equal(rule R) bool {
443+
func (a *RSelector) Equal(rule R, check *CrossFileEqualityCheck) bool {
413444
b, ok := rule.(*RSelector)
414445
if ok && len(a.Selectors) == len(b.Selectors) && a.HasAtNest == b.HasAtNest {
415-
for i, sel := range a.Selectors {
416-
if !sel.Equal(b.Selectors[i]) {
446+
for i, ai := range a.Selectors {
447+
if !ai.Equal(b.Selectors[i], check) {
417448
return false
418449
}
419450
}
420-
return RulesEqual(a.Rules, b.Rules)
451+
return RulesEqual(a.Rules, b.Rules, check)
421452
}
422453

423454
return false
@@ -450,9 +481,9 @@ type RQualified struct {
450481
Rules []Rule
451482
}
452483

453-
func (a *RQualified) Equal(rule R) bool {
484+
func (a *RQualified) Equal(rule R, check *CrossFileEqualityCheck) bool {
454485
b, ok := rule.(*RQualified)
455-
return ok && TokensEqual(a.Prelude, b.Prelude) && RulesEqual(a.Rules, b.Rules)
486+
return ok && TokensEqual(a.Prelude, b.Prelude, check) && RulesEqual(a.Rules, b.Rules, check)
456487
}
457488

458489
func (r *RQualified) Hash() (uint32, bool) {
@@ -470,9 +501,9 @@ type RDeclaration struct {
470501
Important bool
471502
}
472503

473-
func (a *RDeclaration) Equal(rule R) bool {
504+
func (a *RDeclaration) Equal(rule R, check *CrossFileEqualityCheck) bool {
474505
b, ok := rule.(*RDeclaration)
475-
return ok && a.KeyText == b.KeyText && TokensEqual(a.Value, b.Value) && a.Important == b.Important
506+
return ok && a.KeyText == b.KeyText && TokensEqual(a.Value, b.Value, check) && a.Important == b.Important
476507
}
477508

478509
func (r *RDeclaration) Hash() (uint32, bool) {
@@ -500,9 +531,9 @@ type RBadDeclaration struct {
500531
Tokens []Token
501532
}
502533

503-
func (a *RBadDeclaration) Equal(rule R) bool {
534+
func (a *RBadDeclaration) Equal(rule R, check *CrossFileEqualityCheck) bool {
504535
b, ok := rule.(*RBadDeclaration)
505-
return ok && TokensEqual(a.Tokens, b.Tokens)
536+
return ok && TokensEqual(a.Tokens, b.Tokens, check)
506537
}
507538

508539
func (r *RBadDeclaration) Hash() (uint32, bool) {
@@ -515,7 +546,7 @@ type RComment struct {
515546
Text string
516547
}
517548

518-
func (a *RComment) Equal(rule R) bool {
549+
func (a *RComment) Equal(rule R, check *CrossFileEqualityCheck) bool {
519550
b, ok := rule.(*RComment)
520551
return ok && a.Text == b.Text
521552
}
@@ -531,7 +562,7 @@ type RAtLayer struct {
531562
Rules []Rule
532563
}
533564

534-
func (a *RAtLayer) Equal(rule R) bool {
565+
func (a *RAtLayer) Equal(rule R, check *CrossFileEqualityCheck) bool {
535566
if b, ok := rule.(*RAtLayer); ok && len(a.Names) == len(b.Names) && len(a.Rules) == len(b.Rules) {
536567
for i, ai := range a.Names {
537568
bi := b.Names[i]
@@ -544,7 +575,7 @@ func (a *RAtLayer) Equal(rule R) bool {
544575
}
545576
}
546577
}
547-
if !RulesEqual(a.Rules, b.Rules) {
578+
if !RulesEqual(a.Rules, b.Rules, check) {
548579
return false
549580
}
550581
}
@@ -568,7 +599,7 @@ type ComplexSelector struct {
568599
Selectors []CompoundSelector
569600
}
570601

571-
func (a ComplexSelector) Equal(b ComplexSelector) bool {
602+
func (a ComplexSelector) Equal(b ComplexSelector, check *CrossFileEqualityCheck) bool {
572603
if len(a.Selectors) != len(b.Selectors) {
573604
return false
574605
}
@@ -589,7 +620,7 @@ func (a ComplexSelector) Equal(b ComplexSelector) bool {
589620
return false
590621
}
591622
for j, aj := range ai.SubclassSelectors {
592-
if !aj.Equal(bi.SubclassSelectors[j]) {
623+
if !aj.Equal(bi.SubclassSelectors[j], check) {
593624
return false
594625
}
595626
}
@@ -632,15 +663,15 @@ func (a NamespacedName) Equal(b NamespacedName) bool {
632663
}
633664

634665
type SS interface {
635-
Equal(ss SS) bool
666+
Equal(ss SS, check *CrossFileEqualityCheck) bool
636667
Hash() uint32
637668
}
638669

639670
type SSHash struct {
640671
Name string
641672
}
642673

643-
func (a *SSHash) Equal(ss SS) bool {
674+
func (a *SSHash) Equal(ss SS, check *CrossFileEqualityCheck) bool {
644675
b, ok := ss.(*SSHash)
645676
return ok && a.Name == b.Name
646677
}
@@ -655,7 +686,7 @@ type SSClass struct {
655686
Name string
656687
}
657688

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

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

696-
func (a *SSPseudoClass) Equal(ss SS) bool {
727+
func (a *SSPseudoClass) Equal(ss SS, check *CrossFileEqualityCheck) bool {
697728
b, ok := ss.(*SSPseudoClass)
698-
return ok && a.Name == b.Name && TokensEqual(a.Args, b.Args) && a.IsElement == b.IsElement
729+
return ok && a.Name == b.Name && TokensEqual(a.Args, b.Args, check) && a.IsElement == b.IsElement
699730
}
700731

701732
func (ss *SSPseudoClass) Hash() uint32 {

0 commit comments

Comments
 (0)