From 83e8c7f0b86e7b627990339870c8cc68c154c77b Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sat, 18 Nov 2023 20:21:24 -0500 Subject: [PATCH] fix #3485: map subpath imports to node built-ins --- CHANGELOG.md | 25 +++ .../bundler_tests/bundler_packagejson_test.go | 35 ++++ .../snapshots/snapshots_packagejson.txt | 9 + internal/resolver/resolver.go | 194 ++++++++++-------- 4 files changed, 174 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92beba10950..4cfefee1ee7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,31 @@ } ``` +* Allow package subpath imports to map to node built-ins ([#3485](https://github.com/evanw/esbuild/issues/3485)) + + You are now able to use a [subpath import](https://nodejs.org/api/packages.html#subpath-imports) in your package to resolve to a node built-in module. For example, with a `package.json` file like this: + + ```json + { + "type": "module", + "imports": { + "#stream": { + "node": "stream", + "default": "./stub.js" + } + } + } + ``` + + You can now import from node's `stream` module like this: + + ```js + import * as stream from '#stream'; + console.log(Object.keys(stream)); + ``` + + This will import from node's `stream` module when the platform is `node` and from `./stub.js` otherwise. + * No longer throw an error when a `Symbol` is missing ([#3453](https://github.com/evanw/esbuild/issues/3453)) Certain JavaScript syntax features use special properties on the global `Symbol` object. For example, the asynchronous iteration syntax uses `Symbol.asyncIterator`. Previously esbuild's generated code for older browsers required this symbol to be polyfilled. However, starting with this release esbuild will use [`Symbol.for()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/for) to construct these symbols if they are missing instead of throwing an error about a missing polyfill. This means your code no longer needs to include a polyfill for missing symbols as long as your code also uses `Symbol.for()` for missing symbols. diff --git a/internal/bundler_tests/bundler_packagejson_test.go b/internal/bundler_tests/bundler_packagejson_test.go index e31b10bf656..8843f82d58c 100644 --- a/internal/bundler_tests/bundler_packagejson_test.go +++ b/internal/bundler_tests/bundler_packagejson_test.go @@ -2873,3 +2873,38 @@ func TestPackageJsonDisabledTypeModuleIssue3367(t *testing.T) { }, }) } + +// See: https://github.com/evanw/esbuild/issues/3485 +func TestPackageJsonSubpathImportNodeBuiltinIssue3485(t *testing.T) { + packagejson_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + import fs from '#fs' + import http from '#http' + fs.readFileSync() + http.createServer() + `, + "/package.json": ` + { + "imports": { + "#fs": { + "node": "fs", + "default": "./empty.js" + }, + "#http": { + "node": "node:http", + "default": "./empty.js" + } + } + } + `, + "/empty.js": ``, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + Platform: config.PlatformNode, + AbsOutputFile: "/out.js", + }, + }) +} diff --git a/internal/bundler_tests/snapshots/snapshots_packagejson.txt b/internal/bundler_tests/snapshots/snapshots_packagejson.txt index 7579f8aef9d..eaf8239cb51 100644 --- a/internal/bundler_tests/snapshots/snapshots_packagejson.txt +++ b/internal/bundler_tests/snapshots/snapshots_packagejson.txt @@ -843,6 +843,15 @@ console.log("pkg3"); // tmp/pkg/@scope/pkg4/bat.js console.log("pkg4"); +================================================================================ +TestPackageJsonSubpathImportNodeBuiltinIssue3485 +---------- /out.js ---------- +// entry.js +import fs from "fs"; +import http from "node:http"; +fs.readFileSync(); +http.createServer(); + ================================================================================ TestPackageJsonTypeShouldBeTypes ---------- /Users/user/project/out.js ---------- diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 13567c3307a..b7d8193e440 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -435,61 +435,10 @@ func (res *Resolver) Resolve(sourceDir string, importPath string, kind ast.Impor }, debugMeta } - // "import fs from 'fs'" - if r.options.Platform == config.PlatformNode && BuiltInNodeModules[importPath] { - if r.debugLogs != nil { - r.debugLogs.addNote("Marking this path as implicitly external due to it being a node built-in") - } - + if pathPair, ok, sideEffects := r.checkForBuiltInNodeModules(importPath); ok { r.flushDebugLogs(flushDueToSuccess) return &ResolveResult{ - PathPair: PathPair{Primary: logger.Path{Text: importPath}, IsExternal: true}, - PrimarySideEffectsData: &SideEffectsData{}, // Mark this with "sideEffects: false" - }, debugMeta - } - - // "import fs from 'node:fs'" - // "require('node:fs')" - if r.options.Platform == config.PlatformNode && strings.HasPrefix(importPath, "node:") { - if r.debugLogs != nil { - r.debugLogs.addNote("Marking this path as implicitly external due to the \"node:\" prefix") - } - - // If this is a known node built-in module, mark it with "sideEffects: false" - var sideEffects *SideEffectsData - if BuiltInNodeModules[strings.TrimPrefix(importPath, "node:")] { - sideEffects = &SideEffectsData{} - } - - // Check whether the path will end up as "import" or "require" - convertImportToRequire := !r.options.OutputFormat.KeepESMImportExportSyntax() - isImport := !convertImportToRequire && (kind == ast.ImportStmt || kind == ast.ImportDynamic) - isRequire := kind == ast.ImportRequire || kind == ast.ImportRequireResolve || - (convertImportToRequire && (kind == ast.ImportStmt || kind == ast.ImportDynamic)) - - // Check for support with "import" - if isImport && r.options.UnsupportedJSFeatures.Has(compat.NodeColonPrefixImport) { - if r.debugLogs != nil { - r.debugLogs.addNote("Removing the \"node:\" prefix because the target environment doesn't support it with \"import\" statements") - } - - // Automatically strip the prefix if it's not supported - importPath = importPath[5:] - } - - // Check for support with "require" - if isRequire && r.options.UnsupportedJSFeatures.Has(compat.NodeColonPrefixRequire) { - if r.debugLogs != nil { - r.debugLogs.addNote("Removing the \"node:\" prefix because the target environment doesn't support it with \"require\" calls") - } - - // Automatically strip the prefix if it's not supported - importPath = importPath[5:] - } - - r.flushDebugLogs(flushDueToSuccess) - return &ResolveResult{ - PathPair: PathPair{Primary: logger.Path{Text: importPath}, IsExternal: true}, + PathPair: pathPair, PrimarySideEffectsData: sideEffects, }, debugMeta } @@ -1021,8 +970,8 @@ func (r resolverQuery) resolveWithoutSymlinks(sourceDir string, sourceDirInfo *d if remapped == nil { return &ResolveResult{PathPair: PathPair{Primary: logger.Path{Text: absPath, Namespace: "file", Flags: logger.PathDisabled}}} } - if remappedResult, ok, diffCase := r.resolveWithoutRemapping(importDirInfo.enclosingBrowserScope, *remapped); ok { - result = ResolveResult{PathPair: remappedResult, DifferentCase: diffCase} + if remappedResult, ok, diffCase, sideEffects := r.resolveWithoutRemapping(importDirInfo.enclosingBrowserScope, *remapped); ok { + result = ResolveResult{PathPair: remappedResult, DifferentCase: diffCase, PrimarySideEffectsData: sideEffects} checkRelative = false checkPackage = false } @@ -1049,12 +998,12 @@ func (r resolverQuery) resolveWithoutSymlinks(sourceDir string, sourceDirInfo *d if remapped, ok := r.checkBrowserMap(sourceDirInfo, importPath, packagePathKind); ok { if remapped == nil { // "browser": {"module": false} - if absolute, ok, diffCase := r.loadNodeModules(importPath, sourceDirInfo, false /* forbidImports */); ok { + if absolute, ok, diffCase, sideEffects := r.loadNodeModules(importPath, sourceDirInfo, false /* forbidImports */); ok { absolute.Primary = logger.Path{Text: absolute.Primary.Text, Namespace: "file", Flags: logger.PathDisabled} if absolute.HasSecondary() { absolute.Secondary = logger.Path{Text: absolute.Secondary.Text, Namespace: "file", Flags: logger.PathDisabled} } - return &ResolveResult{PathPair: absolute, DifferentCase: diffCase} + return &ResolveResult{PathPair: absolute, DifferentCase: diffCase, PrimarySideEffectsData: sideEffects} } else { return &ResolveResult{PathPair: PathPair{Primary: logger.Path{Text: importPath, Flags: logger.PathDisabled}}, DifferentCase: diffCase} } @@ -1066,8 +1015,8 @@ func (r resolverQuery) resolveWithoutSymlinks(sourceDir string, sourceDirInfo *d sourceDirInfo = sourceDirInfo.enclosingBrowserScope } - if absolute, ok, diffCase := r.resolveWithoutRemapping(sourceDirInfo, importPath); ok { - result = ResolveResult{PathPair: absolute, DifferentCase: diffCase} + if absolute, ok, diffCase, sideEffects := r.resolveWithoutRemapping(sourceDirInfo, importPath); ok { + result = ResolveResult{PathPair: absolute, DifferentCase: diffCase, PrimarySideEffectsData: sideEffects} } else { // Note: node's "self references" are not currently supported return nil @@ -1077,11 +1026,12 @@ func (r resolverQuery) resolveWithoutSymlinks(sourceDir string, sourceDirInfo *d return &result } -func (r resolverQuery) resolveWithoutRemapping(sourceDirInfo *dirInfo, importPath string) (PathPair, bool, *fs.DifferentCase) { +func (r resolverQuery) resolveWithoutRemapping(sourceDirInfo *dirInfo, importPath string) (PathPair, bool, *fs.DifferentCase, *SideEffectsData) { if IsPackagePath(importPath) { return r.loadNodeModules(importPath, sourceDirInfo, false /* forbidImports */) } else { - return r.loadAsFileOrDirectory(r.fs.Join(sourceDirInfo.absPath, importPath)) + absolute, ok, diffCase := r.loadAsFileOrDirectory(r.fs.Join(sourceDirInfo.absPath, importPath)) + return absolute, ok, diffCase, nil } } @@ -2179,7 +2129,7 @@ func (r resolverQuery) matchTSConfigPaths(tsConfigJSON *TSConfigJSON, path strin return PathPair{}, false, nil } -func (r resolverQuery) loadPackageImports(importPath string, dirInfoPackageJSON *dirInfo) (PathPair, bool, *fs.DifferentCase) { +func (r resolverQuery) loadPackageImports(importPath string, dirInfoPackageJSON *dirInfo) (PathPair, bool, *fs.DifferentCase, *SideEffectsData) { packageJSON := dirInfoPackageJSON.packageJSON if r.debugLogs != nil { @@ -2197,7 +2147,7 @@ func (r resolverQuery) loadPackageImports(importPath string, dirInfoPackageJSON tracker := logger.MakeLineColumnTracker(&packageJSON.source) r.debugMeta.notes = append(r.debugMeta.notes, tracker.MsgData(packageJSON.importsMap.root.firstToken, fmt.Sprintf("This \"imports\" map was ignored because the module specifier %q is invalid:", importPath))) - return PathPair{}, false, nil + return PathPair{}, false, nil, nil } // The condition set is determined by the kind of import @@ -2213,10 +2163,14 @@ func (r resolverQuery) loadPackageImports(importPath string, dirInfoPackageJSON resolvedPath, status, debug = r.esmHandlePostConditions(resolvedPath, status, debug) if status == pjStatusPackageResolve { + if pathPair, ok, sideEffects := r.checkForBuiltInNodeModules(resolvedPath); ok { + return pathPair, true, nil, sideEffects + } + // The import path was remapped via "imports" to another import path // that now needs to be resolved too. Set "forbidImports" to true // so we don't try to resolve "imports" again and end up in a loop. - absolute, ok, diffCase := r.loadNodeModules(resolvedPath, dirInfoPackageJSON, true /* forbidImports */) + absolute, ok, diffCase, sideEffects := r.loadNodeModules(resolvedPath, dirInfoPackageJSON, true /* forbidImports */) if !ok { tracker := logger.MakeLineColumnTracker(&packageJSON.source) r.debugMeta.notes = append( @@ -2224,14 +2178,15 @@ func (r resolverQuery) loadPackageImports(importPath string, dirInfoPackageJSON fmt.Sprintf("The remapped path %q could not be resolved:", resolvedPath))}, r.debugMeta.notes...) } - return absolute, ok, diffCase + return absolute, ok, diffCase, sideEffects } - return r.finalizeImportsExportsResult( + absolute, ok, diffCase := r.finalizeImportsExportsResult( dirInfoPackageJSON.absPath, conditions, *packageJSON.importsMap, packageJSON, resolvedPath, status, debug, "", "", "", ) + return absolute, ok, diffCase, nil } func (r resolverQuery) esmResolveAlgorithm(esmPackageName string, esmPackageSubpath string, packageJSON *packageJSON, absPkgPath string, absPath string) (PathPair, bool, *fs.DifferentCase) { @@ -2275,7 +2230,7 @@ func (r resolverQuery) esmResolveAlgorithm(esmPackageName string, esmPackageSubp ) } -func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forbidImports bool) (PathPair, bool, *fs.DifferentCase) { +func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forbidImports bool) (PathPair, bool, *fs.DifferentCase, *SideEffectsData) { if r.debugLogs != nil { r.debugLogs.addNote(fmt.Sprintf("Searching for %q in \"node_modules\" directories starting from %q", importPath, dirInfo.absPath)) r.debugLogs.increaseIndent() @@ -2287,7 +2242,7 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb // Try path substitutions first if tsConfigJSON.Paths != nil { if absolute, ok, diffCase := r.matchTSConfigPaths(tsConfigJSON, importPath); ok { - return absolute, true, diffCase + return absolute, true, diffCase, nil } } @@ -2295,7 +2250,7 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb if tsConfigJSON.BaseURL != nil { basePath := r.fs.Join(*tsConfigJSON.BaseURL, importPath) if absolute, ok, diffCase := r.loadAsFileOrDirectory(basePath); ok { - return absolute, true, diffCase + return absolute, true, diffCase, nil } } } @@ -2316,7 +2271,7 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb if r.debugLogs != nil { r.debugLogs.addNote("Marking this path as external because it's a package path") } - return PathPair{Primary: logger.Path{Text: importPath}, IsExternal: true}, true, nil + return PathPair{Primary: logger.Path{Text: importPath}, IsExternal: true}, true, nil, nil } // If Yarn PnP is active, use it to find the package @@ -2337,7 +2292,7 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb fmt.Sprintf("The Yarn Plug'n'Play manifest says this package has a peer dependency on %q, but the package %q has not been installed:", result.errorIdent, result.errorIdent))} } - return PathPair{}, false, nil + return PathPair{}, false, nil, nil } else if result.status == pnpSuccess { absPath := r.fs.Join(result.pkgDirPath, result.pkgSubpath) @@ -2349,28 +2304,29 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb if pkgDirInfo := r.dirInfoCached(result.pkgDirPath); pkgDirInfo != nil { // Check the "exports" map if packageJSON := pkgDirInfo.packageJSON; packageJSON != nil && packageJSON.exportsMap != nil { - return r.esmResolveAlgorithm(result.pkgIdent, "."+result.pkgSubpath, packageJSON, pkgDirInfo.absPath, absPath) + absolute, ok, diffCase := r.esmResolveAlgorithm(result.pkgIdent, "."+result.pkgSubpath, packageJSON, pkgDirInfo.absPath, absPath) + return absolute, ok, diffCase, nil } // 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 + return PathPair{Primary: logger.Path{Text: absPath, Namespace: "file", Flags: logger.PathDisabled}}, true, nil, nil } - if remappedResult, ok, diffCase := r.resolveWithoutRemapping(pkgDirInfo.enclosingBrowserScope, *remapped); ok { - return remappedResult, true, diffCase + if remappedResult, ok, diffCase, sideEffects := r.resolveWithoutRemapping(pkgDirInfo.enclosingBrowserScope, *remapped); ok { + return remappedResult, true, diffCase, sideEffects } } if absolute, ok, diffCase := r.loadAsFileOrDirectory(absPath); ok { - return absolute, true, diffCase + return absolute, true, diffCase, nil } } if r.debugLogs != nil { r.debugLogs.addNote(fmt.Sprintf("Failed to resolve %q to a file", absPath)) } - return PathPair{}, false, nil + return PathPair{}, false, nil, nil } } @@ -2383,13 +2339,14 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb // Check for self-references if dirInfoPackageJSON != nil { if packageJSON := dirInfoPackageJSON.packageJSON; packageJSON.name == esmPackageName && packageJSON.exportsMap != nil { - return r.esmResolveAlgorithm(esmPackageName, esmPackageSubpath, packageJSON, + absolute, ok, diffCase := r.esmResolveAlgorithm(esmPackageName, esmPackageSubpath, packageJSON, dirInfoPackageJSON.absPath, r.fs.Join(dirInfoPackageJSON.absPath, esmPackageSubpath)) + return absolute, ok, diffCase, nil } } // Common package resolution logic shared between "node_modules" and "NODE_PATHS" - tryToResolvePackage := func(absDir string) (PathPair, bool, *fs.DifferentCase, bool) { + tryToResolvePackage := func(absDir string) (PathPair, bool, *fs.DifferentCase, *SideEffectsData, 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)) @@ -2402,16 +2359,16 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb // 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 + return absolute, ok, diffCase, nil, 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, true + return PathPair{Primary: logger.Path{Text: absPath, Namespace: "file", Flags: logger.PathDisabled}}, true, nil, nil, true } - if remappedResult, ok, diffCase := r.resolveWithoutRemapping(pkgDirInfo.enclosingBrowserScope, *remapped); ok { - return remappedResult, true, diffCase, true + if remappedResult, ok, diffCase, sideEffects := r.resolveWithoutRemapping(pkgDirInfo.enclosingBrowserScope, *remapped); ok { + return remappedResult, true, diffCase, sideEffects, true } } } @@ -2419,10 +2376,10 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb // Try node's old package resolution rules if absolute, ok, diffCase := r.loadAsFileOrDirectory(absPath); ok { - return absolute, true, diffCase, true + return absolute, true, diffCase, nil, true } - return PathPair{}, false, nil, false + return PathPair{}, false, nil, nil, false } // Then check for the package in any enclosing "node_modules" directories @@ -2430,8 +2387,8 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb // 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 + if absolute, ok, diffCase, sideEffects, shouldStop := tryToResolvePackage(r.fs.Join(dirInfo.absPath, "node_modules")); shouldStop { + return absolute, ok, diffCase, sideEffects } } @@ -2446,9 +2403,68 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb // 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 { - if absolute, ok, diffCase, shouldStop := tryToResolvePackage(absDir); shouldStop { - return absolute, ok, diffCase + if absolute, ok, diffCase, sideEffects, shouldStop := tryToResolvePackage(absDir); shouldStop { + return absolute, ok, diffCase, sideEffects + } + } + + return PathPair{}, false, nil, nil +} + +func (r resolverQuery) checkForBuiltInNodeModules(importPath string) (PathPair, bool, *SideEffectsData) { + // "import fs from 'fs'" + if r.options.Platform == config.PlatformNode && BuiltInNodeModules[importPath] { + if r.debugLogs != nil { + r.debugLogs.addNote("Marking this path as implicitly external due to it being a node built-in") + } + + r.flushDebugLogs(flushDueToSuccess) + return PathPair{Primary: logger.Path{Text: importPath}, IsExternal: true}, + true, + &SideEffectsData{} // Mark this with "sideEffects: false" + } + + // "import fs from 'node:fs'" + // "require('node:fs')" + if r.options.Platform == config.PlatformNode && strings.HasPrefix(importPath, "node:") { + if r.debugLogs != nil { + r.debugLogs.addNote("Marking this path as implicitly external due to the \"node:\" prefix") + } + + // If this is a known node built-in module, mark it with "sideEffects: false" + var sideEffects *SideEffectsData + if BuiltInNodeModules[strings.TrimPrefix(importPath, "node:")] { + sideEffects = &SideEffectsData{} } + + // Check whether the path will end up as "import" or "require" + convertImportToRequire := !r.options.OutputFormat.KeepESMImportExportSyntax() + isImport := !convertImportToRequire && (r.kind == ast.ImportStmt || r.kind == ast.ImportDynamic) + isRequire := r.kind == ast.ImportRequire || r.kind == ast.ImportRequireResolve || + (convertImportToRequire && (r.kind == ast.ImportStmt || r.kind == ast.ImportDynamic)) + + // Check for support with "import" + if isImport && r.options.UnsupportedJSFeatures.Has(compat.NodeColonPrefixImport) { + if r.debugLogs != nil { + r.debugLogs.addNote("Removing the \"node:\" prefix because the target environment doesn't support it with \"import\" statements") + } + + // Automatically strip the prefix if it's not supported + importPath = importPath[5:] + } + + // Check for support with "require" + if isRequire && r.options.UnsupportedJSFeatures.Has(compat.NodeColonPrefixRequire) { + if r.debugLogs != nil { + r.debugLogs.addNote("Removing the \"node:\" prefix because the target environment doesn't support it with \"require\" calls") + } + + // Automatically strip the prefix if it's not supported + importPath = importPath[5:] + } + + r.flushDebugLogs(flushDueToSuccess) + return PathPair{Primary: logger.Path{Text: importPath}, IsExternal: true}, true, sideEffects } return PathPair{}, false, nil