Skip to content

Commit

Permalink
fix #2752: respect exports in NODE_PATHS
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Dec 15, 2022
1 parent d28646b commit 14cc802
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 37 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -6,6 +6,10 @@

The change in version 0.16.0 to give control over `</script>` escaping via `--supported:inline-script=false` or `--supported:inline-script=true` accidentally broke automatic escaping of `</script>` when an explicit `target` setting is specified. This release restores the correct automatic escaping of `</script>` (which should not depend on what `target` is set to).

* Enable the `exports` field with `NODE_PATHS` ([#2752](https://github.com/evanw/esbuild/issues/2752))

Node has a rarely-used feature where you can extend the set of directories that node searches for packages using the `NODE_PATHS` environment variable. While esbuild supports this too, previously it only supported the old `main` field path resolution but did not support the new `exports` field package resolution. This release makes the path resolution rules the same again for both `node_modules` directories and `NODE_PATHS` directories.

## 0.16.7

* Include `file` loader strings in metafile imports ([#2731](https://github.com/evanw/esbuild/issues/2731))
Expand Down
32 changes: 32 additions & 0 deletions internal/bundler/bundler_packagejson_test.go
View file
Expand Up @@ -2731,3 +2731,35 @@ NOTE: Node's package format requires that CommonJS files in a "type": "module" p
`,
})
}

func TestPackageJsonNodePathsIssue2752(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
"/src/entry.js": `
import "pkg1"
import "pkg2"
import "@scope/pkg3/baz"
import "@scope/pkg4"
`,
"/usr/lib/pkg/pkg1/package.json": `{ "main": "./foo.js" }`,
"/usr/lib/pkg/pkg1/foo.js": `console.log('pkg1')`,
"/lib/pkg/pkg2/package.json": `{ "exports": { ".": "./bar.js" } }`,
"/lib/pkg/pkg2/bar.js": `console.log('pkg2')`,
"/var/lib/pkg/@scope/pkg3/package.json": `{ "browser": { "./baz.js": "./baz-browser.js" } }`,
"/var/lib/pkg/@scope/pkg3/baz-browser.js": `console.log('pkg3')`,
"/tmp/pkg/@scope/pkg4/package.json": `{ "exports": { ".": { "import": "./bat.js" } } }`,
"/tmp/pkg/@scope/pkg4/bat.js": `console.log('pkg4')`,
},
entryPaths: []string{"/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
AbsNodePaths: []string{
"/usr/lib/pkg",
"/lib/pkg",
"/var/lib/pkg",
"/tmp/pkg",
},
},
})
}
15 changes: 15 additions & 0 deletions internal/bundler/snapshots/snapshots_packagejson.txt
View file
Expand Up @@ -815,6 +815,21 @@ var require_main = __commonJS({
var import_demo_pkg = __toESM(require_main());
console.log((0, import_demo_pkg.default)());

================================================================================
TestPackageJsonNodePathsIssue2752
---------- /out.js ----------
// usr/lib/pkg/pkg1/foo.js
console.log("pkg1");

// lib/pkg/pkg2/bar.js
console.log("pkg2");

// var/lib/pkg/@scope/pkg3/baz-browser.js
console.log("pkg3");

// tmp/pkg/@scope/pkg4/bat.js
console.log("pkg4");

================================================================================
TestPackageJsonTypeShouldBeTypes
---------- /Users/user/project/out.js ----------
80 changes: 43 additions & 37 deletions internal/resolver/resolver.go
View file
Expand Up @@ -2160,39 +2160,50 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb
}
}

// Then check for the package in any enclosing "node_modules" directories
for {
// Skip directories that are themselves called "node_modules", since we
// don't ever want to search for "node_modules/node_modules"
if dirInfo.hasNodeModules {
absPath := r.fs.Join(dirInfo.absPath, "node_modules", importPath)
if r.debugLogs != nil {
r.debugLogs.addNote(fmt.Sprintf("Checking for a package in the directory %q", absPath))
}
// Common package resolution logic shared between "node_modules" and "NODE_PATHS"
tryToResolvePackage := func(absDir string) (PathPair, bool, *fs.DifferentCase, bool) {
absPath := r.fs.Join(absDir, importPath)
if r.debugLogs != nil {
r.debugLogs.addNote(fmt.Sprintf("Checking for a package in the directory %q", absPath))
}

// Check the package's package.json file
if esmOK {
absPkgPath := r.fs.Join(dirInfo.absPath, "node_modules", esmPackageName)
if pkgDirInfo := r.dirInfoCached(absPkgPath); pkgDirInfo != nil {
// Check the "exports" map
if packageJSON := pkgDirInfo.packageJSON; packageJSON != nil && packageJSON.exportsMap != nil {
return r.esmResolveAlgorithm(esmPackageName, esmPackageSubpath, packageJSON, absPkgPath, absPath)
}
// Try node's new package resolution rules
if esmOK {
absPkgPath := r.fs.Join(absDir, esmPackageName)
if pkgDirInfo := r.dirInfoCached(absPkgPath); pkgDirInfo != nil {
// Check the "exports" map
if packageJSON := pkgDirInfo.packageJSON; packageJSON != nil && packageJSON.exportsMap != nil {
absolute, ok, diffCase := r.esmResolveAlgorithm(esmPackageName, esmPackageSubpath, packageJSON, absPkgPath, absPath)
return absolute, ok, diffCase, true
}

// Check the "browser" map
if remapped, ok := r.checkBrowserMap(pkgDirInfo, absPath, absolutePathKind); ok {
if remapped == nil {
return PathPair{Primary: logger.Path{Text: absPath, Namespace: "file", Flags: logger.PathDisabled}}, true, nil
}
if remappedResult, ok, diffCase := r.resolveWithoutRemapping(pkgDirInfo.enclosingBrowserScope, *remapped); ok {
return remappedResult, true, diffCase
}
// Check the "browser" map
if remapped, ok := r.checkBrowserMap(pkgDirInfo, absPath, absolutePathKind); ok {
if remapped == nil {
return PathPair{Primary: logger.Path{Text: absPath, Namespace: "file", Flags: logger.PathDisabled}}, true, nil, true
}
if remappedResult, ok, diffCase := r.resolveWithoutRemapping(pkgDirInfo.enclosingBrowserScope, *remapped); ok {
return remappedResult, true, diffCase, true
}
}
}
}

if absolute, ok, diffCase := r.loadAsFileOrDirectory(absPath); ok {
return absolute, true, diffCase
// Try node's old package resolution rules
if absolute, ok, diffCase := r.loadAsFileOrDirectory(absPath); ok {
return absolute, true, diffCase, true
}

return PathPair{}, false, nil, false
}

// Then check for the package in any enclosing "node_modules" directories
for {
// Skip directories that are themselves called "node_modules", since we
// don't ever want to search for "node_modules/node_modules"
if dirInfo.hasNodeModules {
if absolute, ok, diffCase, shouldStop := tryToResolvePackage(r.fs.Join(dirInfo.absPath, "node_modules")); shouldStop {
return absolute, ok, diffCase
}
}

Expand All @@ -2203,17 +2214,12 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb
}
}

// Then check the global "NODE_PATH" environment variable.
//
// Note: This is a deviation from node's published module resolution
// algorithm. The published algorithm says "NODE_PATH" must take precedence
// over "node_modules" paths, but it appears that the published algorithm is
// incorrect. We follow node's actual behavior instead of following the
// published algorithm. See also: https://github.com/nodejs/node/issues/38128.
// Then check the global "NODE_PATH" environment variable. It has been
// clarified that this step comes last after searching for "node_modules"
// directories: https://github.com/nodejs/node/issues/38128.
for _, absDir := range r.options.AbsNodePaths {
absPath := r.fs.Join(absDir, importPath)
if absolute, ok, diffCase := r.loadAsFileOrDirectory(absPath); ok {
return absolute, true, diffCase
if absolute, ok, diffCase, shouldStop := tryToResolvePackage(absDir); shouldStop {
return absolute, ok, diffCase
}
}

Expand Down

0 comments on commit 14cc802

Please sign in to comment.