Skip to content

Commit

Permalink
fix #3295: better avoid css local name collisions
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Aug 11, 2023
1 parent 49801f7 commit 009beb1
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 14 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

* Make renamed CSS names unique across entry points ([#3295](https://github.com/evanw/esbuild/issues/3295))

Previously esbuild's generated names for local names in CSS were only unique within a given entry point (or across all entry points when code splitting was enabled). That meant that building multiple entry points with esbuild could result in local names being renamed to the same identifier even when those entry points were built simultaneously within a single esbuild API call. This problem was especially likely to happen with minification enabled. With this release, esbuild will now avoid renaming local names from two separate entry points to the same name if those entry points were built with a single esbuild API call, even when code splitting is disabled.

## 0.19.1

* Fix a regression with `baseURL` in `tsconfig.json` ([#3307](https://github.com/evanw/esbuild/issues/3307))
Expand Down
15 changes: 11 additions & 4 deletions internal/bundler/bundler.go
Original file line number Diff line number Diff line change
Expand Up @@ -2653,8 +2653,12 @@ func (b *Bundle) Compile(log logger.Log, timer *helpers.Timer, mangleCache map[s
options := b.options

// In most cases we don't need synchronized access to the mangle cache
options.ExclusiveMangleCacheUpdate = func(cb func(mangleCache map[string]interface{})) {
cb(mangleCache)
cssUsedLocalNames := make(map[string]bool)
options.ExclusiveMangleCacheUpdate = func(cb func(
mangleCache map[string]interface{},
cssUsedLocalNames map[string]bool,
)) {
cb(mangleCache, cssUsedLocalNames)
}

files := make([]graph.InputFile, len(b.files))
Expand Down Expand Up @@ -2688,11 +2692,14 @@ func (b *Bundle) Compile(log logger.Log, timer *helpers.Timer, mangleCache map[s

// Each goroutine needs a separate options object
optionsClone := options
optionsClone.ExclusiveMangleCacheUpdate = func(cb func(mangleCache map[string]interface{})) {
optionsClone.ExclusiveMangleCacheUpdate = func(cb func(
mangleCache map[string]interface{},
cssUsedLocalNames map[string]bool,
)) {
// Serialize all accesses to the mangle cache in entry point order for determinism
serializer.Enter(i)
defer serializer.Leave(i)
cb(mangleCache)
cb(mangleCache, cssUsedLocalNames)
}

resultGroups[i] = link(&optionsClone, forked, log, b.fs, b.res, files, entryPoints,
Expand Down
30 changes: 30 additions & 0 deletions internal/bundler_tests/bundler_css_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,36 @@ func TestImportLocalCSSFromJSMinifyIdentifiersAvoidGlobalNames(t *testing.T) {
})
}

// See: https://github.com/evanw/esbuild/issues/3295
func TestImportLocalCSSFromJSMinifyIdentifiersMultipleEntryPoints(t *testing.T) {
css_suite.expectBundled(t, bundled{
files: map[string]string{
"/a.js": `
import { foo, bar } from "./a.module.css";
console.log(foo, bar);
`,
"/a.module.css": `
.foo { color: #001; }
.bar { color: #002; }
`,
"/b.js": `
import { foo, bar } from "./b.module.css";
console.log(foo, bar);
`,
"/b.module.css": `
.foo { color: #003; }
.bar { color: #004; }
`,
},
entryPaths: []string{"/a.js", "/b.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
MinifyIdentifiers: true,
},
})
}

func TestImportCSSFromJSLocalVsGlobal(t *testing.T) {
css := `
.top_level { color: #000 }
Expand Down
36 changes: 36 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_css.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3215,6 +3215,42 @@ TestImportLocalCSSFromJSMinifyIdentifiersAvoidGlobalNames
color: blue;
}

================================================================================
TestImportLocalCSSFromJSMinifyIdentifiersMultipleEntryPoints
---------- /out/a.js ----------
// a.module.css
var r = "o";
var l = "c";

// a.js
console.log(r, l);

---------- /out/a.css ----------
/* a.module.css */
.o {
color: #001;
}
.c {
color: #002;
}

---------- /out/b.js ----------
// b.module.css
var r = "l";
var l = "r";

// b.js
console.log(r, l);

---------- /out/b.css ----------
/* b.module.css */
.l {
color: #003;
}
.r {
color: #004;
}

================================================================================
TestMetafileCSSBundleTwoToOne
---------- /out/js/2PSDKYWE.js ----------
Expand Down
5 changes: 4 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,10 @@ type Options struct {
// parallel so only property mangling is serialized, which is implemented by
// this function blocking until the previous entry point's property mangling
// has finished.
ExclusiveMangleCacheUpdate func(cb func(mangleCache map[string]interface{}))
ExclusiveMangleCacheUpdate func(cb func(
mangleCache map[string]interface{},
cssUsedLocalNames map[string]bool,
))

// This is the original information that was used to generate the
// unsupported feature sets above. It's used for error messages.
Expand Down
21 changes: 12 additions & 9 deletions internal/linker/linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func Link(

// Stop now if there were errors
if c.log.HasErrors() {
c.options.ExclusiveMangleCacheUpdate(func(mangleCache map[string]interface{}) {
c.options.ExclusiveMangleCacheUpdate(func(map[string]interface{}, map[string]bool) {
// Always do this so that we don't cause other entry points when there are errors
})
return []graph.OutputFile{}
Expand All @@ -333,10 +333,13 @@ func Link(
// Merge mangled properties before chunks are generated since the names must
// be consistent across all chunks, or the generated code will break
c.timer.Begin("Waiting for mangle cache")
c.options.ExclusiveMangleCacheUpdate(func(mangleCache map[string]interface{}) {
c.options.ExclusiveMangleCacheUpdate(func(
mangleCache map[string]interface{},
cssUsedLocalNames map[string]bool,
) {
c.timer.End("Waiting for mangle cache")
c.mangleProps(mangleCache)
c.mangleLocalCSS()
c.mangleLocalCSS(cssUsedLocalNames)
})

// Make sure calls to "ast.FollowSymbols()" in parallel goroutines after this
Expand Down Expand Up @@ -444,7 +447,7 @@ func (c *linkerContext) mangleProps(mangleCache map[string]interface{}) {
}
}

func (c *linkerContext) mangleLocalCSS() {
func (c *linkerContext) mangleLocalCSS(usedLocalNames map[string]bool) {
c.timer.Begin("Mangle local CSS")
defer c.timer.End("Mangle local CSS")

Expand Down Expand Up @@ -492,14 +495,14 @@ func (c *linkerContext) mangleLocalCSS() {

for _, symbolCount := range sorted {
name := minifier.NumberToMinifiedName(nextName)
for globalNames[name] {
for globalNames[name] || usedLocalNames[name] {
nextName++
name = minifier.NumberToMinifiedName(nextName)
}

// Turn this local name into a global one
mangledProps[symbolCount.Ref] = name
globalNames[name] = true
usedLocalNames[name] = true
}
} else {
nameCounts := make(map[string]uint32)
Expand All @@ -509,7 +512,7 @@ func (c *linkerContext) mangleLocalCSS() {
name := fmt.Sprintf("%s_%s", c.graph.Files[symbolCount.Ref.SourceIndex].InputFile.Source.IdentifierName, symbol.OriginalName)

// If the name is already in use, generate a new name by appending a number
if globalNames[name] {
if globalNames[name] || usedLocalNames[name] {
// To avoid O(n^2) behavior, the number must start off being the number
// that we used last time there was a collision with this name. Otherwise
// if there are many collisions with the same name, each name collision
Expand All @@ -527,7 +530,7 @@ func (c *linkerContext) mangleLocalCSS() {
name = prefix + strconv.Itoa(int(tries))

// Make sure this new name is unused
if !globalNames[name] {
if !globalNames[name] && !usedLocalNames[name] {
// Store the count so we can start here next time instead of starting
// from 1. This means we avoid O(n^2) behavior.
nameCounts[prefix] = tries
Expand All @@ -538,7 +541,7 @@ func (c *linkerContext) mangleLocalCSS() {

// Turn this local name into a global one
mangledProps[symbolCount.Ref] = name
globalNames[name] = true
usedLocalNames[name] = true
}
}
}
Expand Down

0 comments on commit 009beb1

Please sign in to comment.