Skip to content

Commit

Permalink
fix #2465: handle windows paths and sideEffects
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Aug 14, 2022
1 parent 9f0699f commit 6fd8736
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 2 deletions.
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@

Previously dependencies of a Yarn PnP virtual dependency failed to resolve on Windows. This was because Windows uses `\` instead of `/` as a path separator, and the path manipulation algorithms used for Yarn PnP expected `/`. This release converts `\` into `/` in Windows paths, which fixes this issue.

* Fix `sideEffects` patterns containing slashes on Windows ([#2465](https://github.com/evanw/esbuild/issues/2465))

The `sideEffects` field in `package.json` lets you specify an array of patterns to mark which files have side effects (which causes all other files to be considered to not have side effects by exclusion). That looks like this:

```json
"sideEffects": [
"**/index.js",
"**/index.prod.js"
]
```

However, the presence of the `/` character in the pattern meant that the pattern failed to match Windows-style paths, which broke `sideEffects` on Windows in this case. This release fixes this problem by adding additional code to handle Windows-style paths.

## 0.15.2

* Fix Yarn PnP issue with packages containing `index.js` ([#2455](https://github.com/evanw/esbuild/issues/2455), [#2461](https://github.com/evanw/esbuild/issues/2461))
Expand Down
30 changes: 30 additions & 0 deletions internal/bundler/bundler_dce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3413,3 +3413,33 @@ func TestNestedFunctionInliningWithSpread(t *testing.T) {
},
})
}

func TestPackageJsonSideEffectsFalseCrossPlatformSlash(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import "demo-pkg/foo"
import "demo-pkg/bar"
`,
"/Users/user/project/node_modules/demo-pkg/foo.js": `
console.log('foo')
`,
"/Users/user/project/node_modules/demo-pkg/bar/index.js": `
console.log('bar')
`,
"/Users/user/project/node_modules/demo-pkg/package.json": `
{
"sideEffects": [
"**/foo.js",
"bar/index.js"
]
}
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
},
})
}
9 changes: 9 additions & 0 deletions internal/bundler/snapshots/snapshots_dce.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,15 @@ var init_a = __esm({
// Users/user/project/src/entry.js
Promise.resolve().then(() => (init_a(), a_exports)).then((x) => assert(x.foo === "foo"));

================================================================================
TestPackageJsonSideEffectsFalseCrossPlatformSlash
---------- /out.js ----------
// Users/user/project/node_modules/demo-pkg/foo.js
console.log("foo");

// Users/user/project/node_modules/demo-pkg/bar/index.js
console.log("bar");

================================================================================
TestPackageJsonSideEffectsFalseIntermediateFilesChainAll
---------- /out.js ----------
Expand Down
1 change: 1 addition & 0 deletions internal/resolver/package_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ func (r resolverQuery) parsePackageJSON(inputPath string) *packageJSON {
pattern = "**/" + pattern
}
absPattern := r.fs.Join(inputPath, pattern)
absPattern = strings.ReplaceAll(absPattern, "\\", "/") // Avoid problems with Windows-style slashes
re, hadWildcard := globstarToEscapedRegexp(absPattern)

// Wildcard patterns require more expensive matching
Expand Down
5 changes: 3 additions & 2 deletions internal/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,13 +578,14 @@ func (r resolverQuery) finalizeResolve(result *ResolveResult) {
if pkgJSON := dirInfo.enclosingPackageJSON; pkgJSON != nil && *path == result.PathPair.Primary {
if pkgJSON.sideEffectsMap != nil {
hasSideEffects := false
if pkgJSON.sideEffectsMap[path.Text] {
pathLookup := strings.ReplaceAll(path.Text, "\\", "/") // Avoid problems with Windows-style slashes
if pkgJSON.sideEffectsMap[pathLookup] {
// Fast path: map lookup
hasSideEffects = true
} else {
// Slow path: glob tests
for _, re := range pkgJSON.sideEffectsRegexps {
if re.MatchString(path.Text) {
if re.MatchString(pathLookup) {
hasSideEffects = true
break
}
Expand Down

0 comments on commit 6fd8736

Please sign in to comment.