fix #3067: crash due to bad subpath import error

CHANGELOG.md

@@ -6,6 +6,10 @@
 
     This release fixes a bug where esbuild incorrectly identified statements of the form `export { default as x } from "y" assert { type: "json" }` as a non-default import. The bug did not affect code of the form `import { default as x } from ...` (only code that used the `export` keyword).
 
+* Fix a crash with an invalid subpath import ([#3067](https://github.com/evanw/esbuild/issues/3067))
+
+    Previously esbuild could crash when attempting to generate a friendly error message for an invalid [subpath import](https://nodejs.org/api/packages.html#subpath-imports) (i.e. an import starting with `#`). This happened because esbuild originally only supported the `exports` field and the code for that error message was not updated when esbuild later added support for the `imports` field. This crash has been fixed.
+
 ## 0.17.17
 
 * Fix CSS nesting transform for top-level `&` ([#3052](https://github.com/evanw/esbuild/issues/3052))

internal/bundler_tests/bundler_packagejson_test.go

@@ -1463,6 +1463,46 @@ NOTE: You can mark the path "pkg2" as external to exclude it from the bundle, wh
 	})
 }
 
+func TestPackageJsonImportsErrorUnsupportedDirectoryImport(t *testing.T) {
+	packagejson_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/Users/user/project/src/entry.js": `
+				import '#foo1/bar'
+				import '#foo2/bar'
+			`,
+			"/Users/user/project/package.json": `
+				{
+					"imports": {
+						"#foo1/*": "./foo1/*",
+						"#foo2/bar": "./foo2/bar"
+					}
+				}
+			`,
+			"/Users/user/project/foo1/bar/index.js": `
+				console.log(bar)
+			`,
+			"/Users/user/project/foo2/bar/index.js": `
+				console.log(bar)
+			`,
+		},
+		entryPaths: []string{"/Users/user/project/src/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/Users/user/project/out.js",
+		},
+		expectedScanLog: `Users/user/project/src/entry.js: ERROR: Could not resolve "#foo1/bar"
+Users/user/project/package.json: NOTE: Importing the directory "./foo1/bar" is forbidden by this package:
+Users/user/project/package.json: NOTE: The presence of "imports" here makes importing a directory forbidden:
+Users/user/project/src/entry.js: NOTE: Import from "/index.js" to get the file "Users/user/project/foo1/bar/index.js":
+NOTE: You can mark the path "#foo1/bar" as external to exclude it from the bundle, which will remove this error.
+Users/user/project/src/entry.js: ERROR: Could not resolve "#foo2/bar"
+Users/user/project/package.json: NOTE: Importing the directory "./foo2/bar" is forbidden by this package:
+Users/user/project/package.json: NOTE: The presence of "imports" here makes importing a directory forbidden:
+NOTE: You can mark the path "#foo2/bar" as external to exclude it from the bundle, which will remove this error.
+`,
+	})
+}
+
 func TestPackageJsonExportsRequireOverImport(t *testing.T) {
 	packagejson_suite.expectBundled(t, bundled{
 		files: map[string]string{

internal/resolver/package_json.go

@@ -443,7 +443,7 @@ func (r resolverQuery) parsePackageJSON(inputPath string) *packageJSON {
 
 	// Read the "imports" map
 	if importsJSON, importsLoc, ok := getProperty(json, "imports"); ok {
-		if importsMap := parseImportsExportsMap(jsonSource, r.log, importsJSON, importsLoc); importsMap != nil {
+		if importsMap := parseImportsExportsMap(jsonSource, r.log, importsJSON, "imports", importsLoc); importsMap != nil {
 			if importsMap.root.kind != pjObject {
 				r.log.AddID(logger.MsgID_PackageJSON_InvalidImportsOrExports, logger.Warning, &tracker, importsMap.root.firstToken,
 					"The value for \"imports\" must be an object")
@@ -454,7 +454,7 @@ func (r resolverQuery) parsePackageJSON(inputPath string) *packageJSON {
 
 	// Read the "exports" map
 	if exportsJSON, exportsLoc, ok := getProperty(json, "exports"); ok {
-		if exportsMap := parseImportsExportsMap(jsonSource, r.log, exportsJSON, exportsLoc); exportsMap != nil {
+		if exportsMap := parseImportsExportsMap(jsonSource, r.log, exportsJSON, "exports", exportsLoc); exportsMap != nil {
 			packageJSON.exportsMap = exportsMap
 		}
 	}
@@ -525,6 +525,7 @@ func globstarToEscapedRegexp(glob string) (string, bool) {
 // Reference: https://nodejs.org/api/esm.html#esm_resolver_algorithm_specification
 type pjMap struct {
 	root           pjEntry
+	propertyKey    string
 	propertyKeyLoc logger.Loc
 }
 
@@ -621,7 +622,7 @@ func (entry pjEntry) valueForKey(key string) (pjEntry, bool) {
 	return pjEntry{}, false
 }
 
-func parseImportsExportsMap(source logger.Source, log logger.Log, json js_ast.Expr, propertyKeyLoc logger.Loc) *pjMap {
+func parseImportsExportsMap(source logger.Source, log logger.Log, json js_ast.Expr, propertyKey string, propertyKeyLoc logger.Loc) *pjMap {
 	var visit func(expr js_ast.Expr) pjEntry
 	tracker := logger.MakeLineColumnTracker(&source)
 
@@ -730,7 +731,11 @@ func parseImportsExportsMap(source logger.Source, log logger.Log, json js_ast.Ex
 		return nil
 	}
 
-	return &pjMap{root: root, propertyKeyLoc: propertyKeyLoc}
+	return &pjMap{
+		root:           root,
+		propertyKey:    propertyKey,
+		propertyKeyLoc: propertyKeyLoc,
+	}
 }
 
 func (entry pjEntry) keysStartWithDot() bool {

internal/resolver/resolver.go

@@ -2417,8 +2417,8 @@ func (r resolverQuery) finalizeImportsExportsResult(
 	case pjStatusUnsupportedDirectoryImport, pjStatusUnsupportedDirectoryImportMissingIndex:
 		r.debugMeta.notes = []logger.MsgData{
 			tracker.MsgData(debug.token, fmt.Sprintf("Importing the directory %q is forbidden by this package:", resolvedPath)),
-			tracker.MsgData(packageJSON.source.RangeOfString(packageJSON.exportsMap.propertyKeyLoc),
-				"The presence of \"exports\" here makes importing a directory forbidden:"),
+			tracker.MsgData(packageJSON.source.RangeOfString(importExportMap.propertyKeyLoc),
+				fmt.Sprintf("The presence of %q here makes importing a directory forbidden:", importExportMap.propertyKey)),
 		}
 
 		// Provide an inline suggestion message with the correct import path