Skip to content

Commit

Permalink
fix #3067: crash due to bad subpath import error
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Apr 22, 2023
1 parent 1365a07 commit dbefad5
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 6 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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))
Expand Down
40 changes: 40 additions & 0 deletions internal/bundler_tests/bundler_packagejson_test.go
Expand Up @@ -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{
Expand Down
13 changes: 9 additions & 4 deletions internal/resolver/package_json.go
Expand Up @@ -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")
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions internal/resolver/resolver.go
Expand Up @@ -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
Expand Down

0 comments on commit dbefad5

Please sign in to comment.