Skip to content

Commit d917038

Browse files
committed
fix #4144: node path resolution edge case
1 parent 7ed1684 commit d917038

File tree

4 files changed

+102
-16
lines changed

4 files changed

+102
-16
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@
4848

4949
This fix was contributed by [@mxschmitt](https://github.com/mxschmitt).
5050

51+
* Fix path resolution edge case ([#4144](https://github.com/evanw/esbuild/issues/4144))
52+
53+
This fixes an edge case where esbuild's path resolution algorithm could deviate from node's path resolution algorithm. It involves a confusing situation where a directory shares the same file name as a file (but without the file extension). See the linked issue for specific details. This appears to be a case where esbuild is correctly following [node's published resolution algorithm](https://nodejs.org/api/modules.html#all-together) but where node itself is doing something different. Specifically the step `LOAD_AS_FILE` appears to be skipped when the input ends with `..`. This release changes esbuild's behavior for this edge case to match node's behavior.
54+
5155
* Update Go from 1.23.7 to 1.23.8 ([#4133](https://github.com/evanw/esbuild/issues/4133), [#4134](https://github.com/evanw/esbuild/pull/4134))
5256

5357
This should have no effect on existing code as this version change does not change Go's operating system support. It may remove certain reports from vulnerability scanners that detect which version of the Go compiler esbuild uses, such as for CVE-2025-22871.

internal/bundler_tests/bundler_packagejson_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3035,3 +3035,37 @@ func TestPackageJsonExportsDefaultWarningIssue3887(t *testing.T) {
30353035
debugLogs: true,
30363036
})
30373037
}
3038+
3039+
// https://github.com/evanw/esbuild/issues/4144
3040+
func TestConfusingNameCollisionsIssue4144(t *testing.T) {
3041+
packagejson_suite.expectBundled(t, bundled{
3042+
files: map[string]string{
3043+
"/entry.js": `
3044+
import { it } from 'mydependency'
3045+
console.log(it())
3046+
`,
3047+
"/node_modules/mydependency/package.json": `
3048+
{
3049+
"main": "./package/index.js"
3050+
}
3051+
`,
3052+
"/node_modules/mydependency/package/index.js": `
3053+
export { it } from './utils'
3054+
export let works = true
3055+
`,
3056+
"/node_modules/mydependency/package/utils/index.js": `
3057+
export { it } from './utils'
3058+
`,
3059+
"/node_modules/mydependency/package/utils/utils.js": `
3060+
// This should resolve to "../index.js" not "../../package.json"
3061+
import { works } from '..'
3062+
export function it() { return works }
3063+
`,
3064+
},
3065+
entryPaths: []string{"/entry.js"},
3066+
options: config.Options{
3067+
Mode: config.ModeBundle,
3068+
AbsOutputFile: "/out.js",
3069+
},
3070+
})
3071+
}

internal/bundler_tests/snapshots/snapshots_packagejson.txt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,20 @@ TestCommonJSVariableInESMTypeModule
33
// entry.js
44
module.exports = null;
55

6+
================================================================================
7+
TestConfusingNameCollisionsIssue4144
8+
---------- /out.js ----------
9+
// node_modules/mydependency/package/utils/utils.js
10+
function it() {
11+
return works;
12+
}
13+
14+
// node_modules/mydependency/package/index.js
15+
var works = true;
16+
17+
// entry.js
18+
console.log(it());
19+
620
================================================================================
721
TestPackageJsonBadExportsDefaultWarningIssue3867
822
---------- /out.js ----------

internal/resolver/resolver.go

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,27 +1000,48 @@ func (r resolverQuery) resolveWithoutSymlinks(sourceDir string, sourceDirInfo *d
10001000
return &ResolveResult{PathPair: PathPair{Primary: logger.Path{Text: absPath, Namespace: "file"}, IsExternal: true}}
10011001
}
10021002

1003-
// Check the "browser" map
1004-
if importDirInfo := r.dirInfoCached(r.fs.Dir(absPath)); importDirInfo != nil {
1005-
if remapped, ok := r.checkBrowserMap(importDirInfo, absPath, absolutePathKind); ok {
1006-
if remapped == nil {
1007-
return &ResolveResult{PathPair: PathPair{Primary: logger.Path{Text: absPath, Namespace: "file", Flags: logger.PathDisabled}}}
1008-
}
1009-
if remappedResult, ok, diffCase, sideEffects := r.resolveWithoutRemapping(importDirInfo.enclosingBrowserScope, *remapped); ok {
1010-
result = ResolveResult{PathPair: remappedResult, DifferentCase: diffCase, PrimarySideEffectsData: sideEffects}
1011-
checkRelative = false
1012-
checkPackage = false
1013-
}
1014-
}
1015-
}
1016-
1017-
if checkRelative {
1018-
if absolute, ok, diffCase := r.loadAsFileOrDirectory(absPath); ok {
1003+
// Node's actual behavior deviates from its published algorithm by not
1004+
// running the "LOAD_AS_FILE" step if the import path looks like it
1005+
// resolves to a directory instead of a file. Attempt to replicate that
1006+
// behavior here. This really only matters for intentionally confusing
1007+
// edge cases where a directory is named the same thing as a file.
1008+
// Unfortunately people actually create situations like this.
1009+
hasTrailingSlash := importPath == "." ||
1010+
importPath == ".." ||
1011+
strings.HasSuffix(importPath, "/") ||
1012+
strings.HasSuffix(importPath, "/.") ||
1013+
strings.HasSuffix(importPath, "/..")
1014+
1015+
if hasTrailingSlash {
1016+
if absolute, ok, diffCase := r.loadAsDirectory(absPath); ok {
10191017
checkPackage = false
10201018
result = ResolveResult{PathPair: absolute, DifferentCase: diffCase}
10211019
} else if !checkPackage {
10221020
return nil
10231021
}
1022+
} else {
1023+
// Check the "browser" map
1024+
if importDirInfo := r.dirInfoCached(r.fs.Dir(absPath)); importDirInfo != nil {
1025+
if remapped, ok := r.checkBrowserMap(importDirInfo, absPath, absolutePathKind); ok {
1026+
if remapped == nil {
1027+
return &ResolveResult{PathPair: PathPair{Primary: logger.Path{Text: absPath, Namespace: "file", Flags: logger.PathDisabled}}}
1028+
}
1029+
if remappedResult, ok, diffCase, sideEffects := r.resolveWithoutRemapping(importDirInfo.enclosingBrowserScope, *remapped); ok {
1030+
result = ResolveResult{PathPair: remappedResult, DifferentCase: diffCase, PrimarySideEffectsData: sideEffects}
1031+
checkRelative = false
1032+
checkPackage = false
1033+
}
1034+
}
1035+
}
1036+
1037+
if checkRelative {
1038+
if absolute, ok, diffCase := r.loadAsFileOrDirectory(absPath); ok {
1039+
checkPackage = false
1040+
result = ResolveResult{PathPair: absolute, DifferentCase: diffCase}
1041+
} else if !checkPackage {
1042+
return nil
1043+
}
1044+
}
10241045
}
10251046
}
10261047

@@ -1899,6 +1920,19 @@ func (r resolverQuery) loadAsFileOrDirectory(path string) (PathPair, bool, *fs.D
18991920
return PathPair{Primary: logger.Path{Text: absolute, Namespace: "file"}}, true, diffCase
19001921
}
19011922

1923+
return r.loadAsDirectory(path)
1924+
}
1925+
1926+
func (r resolverQuery) loadAsDirectory(path string) (PathPair, bool, *fs.DifferentCase) {
1927+
extensionOrder := r.options.ExtensionOrder
1928+
if r.kind.MustResolveToCSS() {
1929+
// Use a special import order for CSS "@import" imports
1930+
extensionOrder = r.cssExtensionOrder
1931+
} else if helpers.IsInsideNodeModules(path) {
1932+
// Use a special import order for imports inside "node_modules"
1933+
extensionOrder = r.nodeModulesExtensionOrder
1934+
}
1935+
19021936
// Is this a directory?
19031937
if r.debugLogs != nil {
19041938
r.debugLogs.addNote(fmt.Sprintf("Attempting to load %q as a directory", path))

0 commit comments

Comments
 (0)