Skip to content

Commit

Permalink
improve "exports" error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Mar 14, 2021
1 parent 74c4dba commit 5e6528d
Show file tree
Hide file tree
Showing 5 changed files with 247 additions and 64 deletions.
38 changes: 38 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,43 @@
# Changelog

## Unreleased

* Improved error message in `exports` failure case

Node's new [conditional exports feature](https://nodejs.org/docs/latest/api/packages.html#packages_conditional_exports) can be non-intuitive and hard to use. Now that esbuild supports this feature (as of version 0.9.0), you can get into a situation where it's impossible to import a package if the package's `exports` field in its `package.json` file isn't configured correctly.

Previously the error message for this looked like this:

```
> entry.js:1:7: error: Could not resolve "jotai" (mark it as external to exclude it from the bundle)
1 │ import 'jotai'
╵ ~~~~~~~
node_modules/jotai/package.json:16:13: note: The path "." is not exported by "jotai"
16 │ "exports": {
╵ ^
```
With this release, the error message will now provide additional information about why the package cannot be imported:
```
> entry.js:1:7: error: Could not resolve "jotai" (mark it as external to exclude it from the bundle)
1 │ import 'jotai'
╵ ~~~~~~~
node_modules/jotai/package.json:16:13: note: The path "." is not currently exported by package "jotai"
16 │ "exports": {
╵ ^
node_modules/jotai/package.json:18:9: note: None of the conditions provided ("module", "require", "types") match any of the currently active conditions ("browser", "default", "import")
18 │ ".": {
╵ ^
entry.js:1:7: note: Consider using a "require()" call to import this package
1 │ import 'jotai'
╵ ~~~~~~~
```
In this case, one solution could be import this module using `require()` since this package provides an export for the `require` condition. Another solution could be to pass `--conditions=module` to esbuild since this package provides an export for the `module` condition (the `types` condition is likely not valid JavaScript code).
This problem occurs because this package doesn't provide an import path for ESM code using the `import` condition and also doesn't provide a fallback import path using the `default` condition.
## 0.9.2
* Fix export name annotations in CommonJS output for node ([#960](https://github.com/evanw/esbuild/issues/960))
Expand Down
20 changes: 10 additions & 10 deletions internal/bundler/bundler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ func parseFile(args parseArgs) {
}

// Run the resolver and log an error if the path couldn't be resolved
resolveResult, didLogError, notes := runOnResolvePlugins(
resolveResult, didLogError, debug := runOnResolvePlugins(
args.options.Plugins,
args.res,
args.log,
Expand Down Expand Up @@ -484,7 +484,7 @@ func parseFile(args parseArgs) {
hint = fmt.Sprintf(" (the plugin %q didn't set a resolve directory)", pluginName)
}
args.log.AddRangeErrorWithNotes(&source, record.Range,
fmt.Sprintf("Could not resolve %q%s", record.Path.Text, hint), notes)
fmt.Sprintf("Could not resolve %q%s", record.Path.Text, hint), debug.Notes(&source, record.Range))
}
continue
}
Expand Down Expand Up @@ -652,7 +652,7 @@ func runOnResolvePlugins(
kind ast.ImportKind,
absResolveDir string,
pluginData interface{},
) (*resolver.ResolveResult, bool, []logger.MsgData) {
) (*resolver.ResolveResult, bool, resolver.DebugMeta) {
resolverArgs := config.OnResolveArgs{
Path: path,
ResolveDir: absResolveDir,
Expand Down Expand Up @@ -681,7 +681,7 @@ func runOnResolvePlugins(

// Stop now if there was an error
if didLogError {
return nil, true, nil
return nil, true, resolver.DebugMeta{}
}

// The "file" namespace is the default for non-external paths, but not
Expand Down Expand Up @@ -710,21 +710,21 @@ func runOnResolvePlugins(
log.AddRangeError(importSource, importPathRange,
fmt.Sprintf("Plugin %q returned a non-absolute path: %s (set a namespace if this is not a file path)", pluginName, result.Path.Text))
}
return nil, true, nil
return nil, true, resolver.DebugMeta{}
}

return &resolver.ResolveResult{
PathPair: resolver.PathPair{Primary: result.Path},
IsExternal: result.External,
PluginData: result.PluginData,
}, false, nil
}, false, resolver.DebugMeta{}
}
}

// Resolve relative to the resolve directory by default. All paths in the
// "file" namespace automatically have a resolve directory. Loader plugins
// can also configure a custom resolve directory for files in other namespaces.
result, notes := res.Resolve(absResolveDir, path, kind)
result, debug := res.Resolve(absResolveDir, path, kind)

// Warn when the case used for importing differs from the actual file name
if result != nil && result.DifferentCase != nil && !resolver.IsInsideNodeModules(absResolveDir) {
Expand All @@ -736,7 +736,7 @@ func runOnResolvePlugins(
))
}

return result, false, notes
return result, false, debug
}

type loaderPluginResult struct {
Expand Down Expand Up @@ -1202,7 +1202,7 @@ func (s *scanner) addEntryPoints(entryPoints []string) []uint32 {
for i, path := range entryPoints {
go func(i int, path string) {
// Run the resolver and log an error if the path couldn't be resolved
resolveResult, didLogError, notes := runOnResolvePlugins(
resolveResult, didLogError, debug := runOnResolvePlugins(
s.options.Plugins,
s.res,
s.log,
Expand All @@ -1227,7 +1227,7 @@ func (s *scanner) addEntryPoints(entryPoints []string) []uint32 {
hint = fmt.Sprintf(" (use %q to reference the file %q)", "./"+path, s.res.PrettyPath(query.PathPair.Primary))
}
}
s.log.AddErrorWithNotes(nil, logger.Loc{}, fmt.Sprintf("Could not resolve %q%s", path, hint), notes)
s.log.AddErrorWithNotes(nil, logger.Loc{}, fmt.Sprintf("Could not resolve %q%s", path, hint), debug.Notes(nil, logger.Range{}))
}
entryPointWaitGroup.Done()
}(i, path)
Expand Down
98 changes: 90 additions & 8 deletions internal/bundler/bundler_packagejson_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1773,21 +1773,99 @@ Users/user/project/node_modules/pkg1/package.json: note: The module "./dir/bar"
})
}

func TestPackageJsonExportsNoConditionsMatchDot(t *testing.T) {
func TestPackageJsonExportsNoConditionsMatch(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import 'pkg1'
import 'pkg1/foo'
import 'pkg1/foo.js'
`,
"/Users/user/project/node_modules/pkg1/package.json": `
{
"exports": {
".": {
"what": "./foo.js"
},
"./foo.js": {
"what": "./foo.js"
}
}
}
`,
"/Users/user/project/node_modules/pkg1/foo.js": `
console.log('FAILURE')
`,
},
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 "pkg1" (mark it as external to exclude it from the bundle)
Users/user/project/node_modules/pkg1/package.json: note: The path "." is not currently exported by package "pkg1"
Users/user/project/node_modules/pkg1/package.json: note: None of the conditions provided ("what") match any of the currently active conditions ("browser", "default", "import")
Users/user/project/src/entry.js: error: Could not resolve "pkg1/foo.js" (mark it as external to exclude it from the bundle)
Users/user/project/node_modules/pkg1/package.json: note: The path "./foo.js" is not currently exported by package "pkg1"
Users/user/project/node_modules/pkg1/package.json: note: None of the conditions provided ("what") match any of the currently active conditions ("browser", "default", "import")
`,
})
}

func TestPackageJsonExportsMustUseRequire(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import 'pkg1'
import 'pkg1/foo.js'
`,
"/Users/user/project/node_modules/pkg1/package.json": `
{
"exports": {
".": {
"require": "./foo.js"
},
"./foo.js": {
"require": "./foo.js"
}
}
}
`,
"/Users/user/project/node_modules/pkg1/foo.js": `
console.log('FAILURE')
`,
},
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 "pkg1" (mark it as external to exclude it from the bundle)
Users/user/project/node_modules/pkg1/package.json: note: The path "." is not currently exported by package "pkg1"
Users/user/project/node_modules/pkg1/package.json: note: None of the conditions provided ("require") match any of the currently active conditions ("browser", "default", "import")
Users/user/project/src/entry.js: note: Consider using a "require()" call to import this package
Users/user/project/src/entry.js: error: Could not resolve "pkg1/foo.js" (mark it as external to exclude it from the bundle)
Users/user/project/node_modules/pkg1/package.json: note: The path "./foo.js" is not currently exported by package "pkg1"
Users/user/project/node_modules/pkg1/package.json: note: None of the conditions provided ("require") match any of the currently active conditions ("browser", "default", "import")
Users/user/project/src/entry.js: note: Consider using a "require()" call to import this package
`,
})
}

func TestPackageJsonExportsMustUseImport(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
require('pkg1')
require('pkg1/foo.js')
`,
"/Users/user/project/node_modules/pkg1/package.json": `
{
"exports": {
".": {
"what": "./foo"
"import": "./foo.js"
},
"./foo": {
"what": "./foo"
"./foo.js": {
"import": "./foo.js"
}
}
}
Expand All @@ -1802,9 +1880,13 @@ func TestPackageJsonExportsNoConditionsMatchDot(t *testing.T) {
AbsOutputFile: "/Users/user/project/out.js",
},
expectedScanLog: `Users/user/project/src/entry.js: error: Could not resolve "pkg1" (mark it as external to exclude it from the bundle)
Users/user/project/node_modules/pkg1/package.json: note: No conditions match for path "." in package "pkg1" (active conditions: "browser", "default", "import")
Users/user/project/src/entry.js: error: Could not resolve "pkg1/foo" (mark it as external to exclude it from the bundle)
Users/user/project/node_modules/pkg1/package.json: note: No conditions match for path "./foo" in package "pkg1" (active conditions: "browser", "default", "import")
Users/user/project/node_modules/pkg1/package.json: note: The path "." is not currently exported by package "pkg1"
Users/user/project/node_modules/pkg1/package.json: note: None of the conditions provided ("import") match any of the currently active conditions ("browser", "default", "require")
Users/user/project/src/entry.js: note: Consider using an "import" statement to import this package
Users/user/project/src/entry.js: error: Could not resolve "pkg1/foo.js" (mark it as external to exclude it from the bundle)
Users/user/project/node_modules/pkg1/package.json: note: The path "./foo.js" is not currently exported by package "pkg1"
Users/user/project/node_modules/pkg1/package.json: note: None of the conditions provided ("import") match any of the currently active conditions ("browser", "default", "require")
Users/user/project/src/entry.js: note: Consider using an "import" statement to import this package
`,
})
}
16 changes: 14 additions & 2 deletions internal/resolver/package_json.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,12 @@ func (status peStatus) isUndefined() bool {
}

type peDebug struct {
// This is the range of the token to use for error messages
token logger.Range

// If the status is "peStatusUndefinedNoConditionsMatch", this is the set of
// conditions that didn't match. This information is used for error messages.
unmatchedConditions []string
}

func esmPackageExportsResolveWithPostConditions(
Expand Down Expand Up @@ -650,8 +655,15 @@ func esmPackageTargetResolve(
}

// ALGORITHM DEVIATION: Provide a friendly error message if no conditions matched
if !target.keysStartWithDot() {
return "", peStatusUndefinedNoConditionsMatch, peDebug{token: target.firstToken}
if len(target.mapData) > 0 && !target.keysStartWithDot() {
keys := make([]string, len(target.mapData))
for i, p := range target.mapData {
keys[i] = p.key
}
return "", peStatusUndefinedNoConditionsMatch, peDebug{
token: target.firstToken,
unmatchedConditions: keys,
}
}

return "", peStatusUndefined, peDebug{token: target.firstToken}
Expand Down
Loading

0 comments on commit 5e6528d

Please sign in to comment.