Skip to content

Commit

Permalink
fix #1184: implicit "**/" in "sideEffects"
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Apr 24, 2021
1 parent d354bbb commit e79a747
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 10 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@

However, doing this is very un-ergonomic and exporting something as an arbitrary name is impossible outside of `export * from`. So this proposal is designed to fully fill out the possibility matrix and make arbitrary alias names a proper first-class feature.

* Implement more accurate `sideEffects` behavior from Webpack ([#1184](https://github.com/evanw/esbuild/issues/1184))

This release adds support for the implicit `**/` prefix that must be added to paths in the `sideEffects` array in `package.json` if the path does not contain `/`. Another way of saying this is if `package.json` contains a `sideEffects` array with a string that doesn't contain a `/` then it should be treated as a file name instead of a path. Previously esbuild treated all strings in this array as paths, which does not match how Webpack behaves. The result of this meant that esbuild could consider files to have no side effects while Webpack would consider the same files to have side effects. This bug should now be fixed.

## 0.11.13

* Implement ergonomic brand checks for private fields
Expand Down
57 changes: 47 additions & 10 deletions internal/resolver/package_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,13 @@ func (r resolverQuery) parsePackageJSON(inputPath string) *packageJSON {
continue
}

absPattern := r.fs.Join(inputPath, js_lexer.UTF16ToString(item.Value))
re, hadWildcard := globToEscapedRegexp(absPattern)
// Reference: https://github.com/webpack/webpack/blob/ed175cd22f89eb9fecd0a70572a3fd0be028e77c/lib/optimize/SideEffectsFlagPlugin.js
pattern := js_lexer.UTF16ToString(item.Value)
if !strings.ContainsRune(pattern, '/') {
pattern = "**/" + pattern
}
absPattern := r.fs.Join(inputPath, pattern)
re, hadWildcard := globstarToEscapedRegexp(absPattern)

// Wildcard patterns require more expensive matching
if hadWildcard {
Expand Down Expand Up @@ -290,27 +295,59 @@ func (r resolverQuery) parsePackageJSON(inputPath string) *packageJSON {
return packageJSON
}

func globToEscapedRegexp(glob string) (string, bool) {
// Reference: https://github.com/fitzgen/glob-to-regexp/blob/2abf65a834259c6504ed3b80e85f893f8cd99127/index.js
func globstarToEscapedRegexp(glob string) (string, bool) {
sb := strings.Builder{}
sb.WriteByte('^')
hadWildcard := false
n := len(glob)

for _, c := range glob {
for i := 0; i < n; i++ {
c := glob[i]
switch c {
case '\\', '^', '$', '.', '+', '|', '(', ')', '[', ']', '{', '}':
sb.WriteByte('\\')
sb.WriteRune(c)

case '*':
sb.WriteString(".*")
hadWildcard = true
sb.WriteByte(c)

case '?':
sb.WriteByte('.')
hadWildcard = true

case '*':
// Move over all consecutive "*"'s.
// Also store the previous and next characters
prevChar := -1
if i > 0 {
prevChar = int(glob[i-1])
}
starCount := 1
for i+1 < n && glob[i+1] == '*' {
starCount++
i++
}
nextChar := -1
if i+1 < n {
nextChar = int(glob[i+1])
}

// Determine if this is a globstar segment
isGlobstar := starCount > 1 && // multiple "*"'s
(prevChar == '/' || prevChar == -1) && // from the start of the segment
(nextChar == '/' || nextChar == -1) // to the end of the segment

if isGlobstar {
// It's a globstar, so match zero or more path segments
sb.WriteString("(?:[^/]*(?:/|$))*")
i++ // Move over the "/"
} else {
// It's not a globstar, so only match one path segment
sb.WriteString("[^/]*")
}

hadWildcard = true

default:
sb.WriteRune(c)
sb.WriteByte(c)
}
}

Expand Down
12 changes: 12 additions & 0 deletions scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -2349,6 +2349,18 @@
'entry.js': `import {foo, bar} from './foo'; let unused = foo; if (bar) throw 'expected "foo" to be tree-shaken'`,
'foo.js': `module.exports = {get foo() { module.exports.bar = 1 }, bar: 0}`,
}),

// Test for an implicit and explicit "**/" prefix (see https://github.com/evanw/esbuild/issues/1184)
test(['--bundle', 'entry.js', '--outfile=node.js'], {
'entry.js': `import './foo'; if (global.dce6 !== 123) throw 'fail'`,
'foo/dir/x.js': `global.dce6 = 123`,
'foo/package.json': `{ "main": "dir/x", "sideEffects": ["x.*"] }`,
}),
test(['--bundle', 'entry.js', '--outfile=node.js'], {
'entry.js': `import './foo'; if (global.dce6 !== 123) throw 'fail'`,
'foo/dir/x.js': `global.dce6 = 123`,
'foo/package.json': `{ "main": "dir/x", "sideEffects": ["**/x.*"] }`,
}),
)

// Test obscure CommonJS symbol edge cases
Expand Down

0 comments on commit e79a747

Please sign in to comment.