Skip to content

Commit

Permalink
suggest enabling missing exports condition (#2163)
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Apr 12, 2022
1 parent 45c0ab2 commit 0286f66
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 14 deletions.
44 changes: 44 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,50 @@
so providing inconsistent metadata for the same path can cause non-determinism.
```

* Suggest enabling a missing condition when `exports` map fails ([#2163](https://github.com/evanw/esbuild/issues/2163))

This release adds another suggestion to the error message that happens when an `exports` map lookup fails if the failure could potentially be fixed by adding a missing condition. Here's what the new error message looks like (which now suggests `--conditions=module` as a possible workaround):

```
✘ [ERROR] Could not resolve "@sentry/electron/main"

index.js:1:24:
1 │ import * as Sentry from '@sentry/electron/main'
╵ ~~~~~~~~~~~~~~~~~~~~~~~

The path "./main" is not currently exported by package "@sentry/electron":

node_modules/@sentry/electron/package.json:8:13:
8 │ "exports": {
╵ ^

None of the conditions provided ("require", "module") match any of the currently active conditions
("browser", "default", "import"):

node_modules/@sentry/electron/package.json:16:14:
16 │ "./main": {
╵ ^

Consider enabling the "module" condition if this package expects it to be enabled. You can use
"--conditions=module" to do that.

node_modules/@sentry/electron/package.json:18:6:
18 │ "module": "./esm/main/index.js"
╵ ~~~~~~~~

Consider using a "require()" call to import this file, which will work because the "require"
condition is supported by this package:

index.js:1:24:
1 │ import * as Sentry from '@sentry/electron/main'
╵ ~~~~~~~~~~~~~~~~~~~~~~~

You can mark the path "@sentry/electron/main" as external to exclude it from the bundle, which
will remove this error.
```

This particular package had an issue where it was using the Webpack-specific `module` condition without providing a `default` condition. It looks like the intent in this case was to use the standard `import` condition instead. This specific change wasn't suggested here because this error message is for package consumers, not package authors.

## 0.14.34

Something went wrong with the publishing script for the previous release. Publishing again.
Expand Down
10 changes: 6 additions & 4 deletions internal/bundler/bundler_packagejson_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1979,10 +1979,12 @@ func TestPackageJsonExportsNoConditionsMatch(t *testing.T) {
expectedScanLog: `Users/user/project/src/entry.js: ERROR: Could not resolve "pkg1"
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/node_modules/pkg1/package.json: NOTE: Consider enabling the "what" condition if this package expects it to be enabled. You can use 'Conditions: []string{"what"}' to do that.
NOTE: You can mark the path "pkg1" as external to exclude it from the bundle, which will remove this error.
Users/user/project/src/entry.js: ERROR: Could not resolve "pkg1/foo.js"
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"):
Users/user/project/node_modules/pkg1/package.json: NOTE: Consider enabling the "what" condition if this package expects it to be enabled. You can use 'Conditions: []string{"what"}' to do that.
NOTE: You can mark the path "pkg1/foo.js" as external to exclude it from the bundle, which will remove this error.
`,
})
Expand Down Expand Up @@ -2019,12 +2021,12 @@ func TestPackageJsonExportsMustUseRequire(t *testing.T) {
expectedScanLog: `Users/user/project/src/entry.js: ERROR: Could not resolve "pkg1"
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 file:
Users/user/project/src/entry.js: NOTE: Consider using a "require()" call to import this file, which will work because the "require" condition is supported by this package:
NOTE: You can mark the path "pkg1" as external to exclude it from the bundle, which will remove this error.
Users/user/project/src/entry.js: ERROR: Could not resolve "pkg1/foo.js"
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 file:
Users/user/project/src/entry.js: NOTE: Consider using a "require()" call to import this file, which will work because the "require" condition is supported by this package:
NOTE: You can mark the path "pkg1/foo.js" as external to exclude it from the bundle, which will remove this error.
`,
})
Expand Down Expand Up @@ -2061,12 +2063,12 @@ func TestPackageJsonExportsMustUseImport(t *testing.T) {
expectedScanLog: `Users/user/project/src/entry.js: ERROR: Could not resolve "pkg1"
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 file:
Users/user/project/src/entry.js: NOTE: Consider using an "import" statement to import this file, which will work because the "import" condition is supported by this package:
NOTE: You can mark the path "pkg1" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
Users/user/project/src/entry.js: ERROR: Could not resolve "pkg1/foo.js"
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 file:
Users/user/project/src/entry.js: NOTE: Consider using an "import" statement to import this file, which will work because the "import" condition is supported by this package:
NOTE: You can mark the path "pkg1/foo.js" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
`,
})
Expand Down
9 changes: 5 additions & 4 deletions internal/resolver/package_json.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -701,8 +701,9 @@ func (status pjStatus) isUndefined() bool {

type pjDebug struct {
// If the status is "pjStatusUndefinedNoConditionsMatch", this is the set of
// conditions that didn't match. This information is used for error messages.
unmatchedConditions []string
// conditions that didn't match, in the order that they were found in the file.
// This information is used for error messages.
unmatchedConditions []logger.Span

// This is the range of the token to use for error messages
token logger.Range
Expand Down Expand Up @@ -1056,9 +1057,9 @@ func (r resolverQuery) esmPackageTargetResolve(
// More information: https://github.com/evanw/esbuild/issues/1484
target = lastMapEntry.value
}
keys := make([]string, len(target.mapData))
keys := make([]logger.Span, len(target.mapData))
for i, p := range target.mapData {
keys[i] = p.key
keys[i] = logger.Span{Text: p.key, Range: p.keyRange}
}
return "", pjStatusUndefinedNoConditionsMatch, pjDebug{
token: target.firstToken,
Expand Down
48 changes: 42 additions & 6 deletions internal/resolver/resolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2052,25 +2052,61 @@ func (r resolverQuery) finalizeImportsExportsResult(
}
return strings.Join(quoted, ", ")
}

keys := make([]string, 0, len(conditions))
for key := range conditions {
keys = append(keys, key)
}
sort.Strings(keys)

unmatchedConditions := make([]string, len(debug.unmatchedConditions))
for i, key := range debug.unmatchedConditions {
unmatchedConditions[i] = key.Text
}

r.debugMeta.notes = []logger.MsgData{
tracker.MsgData(importExportMap.root.firstToken,
fmt.Sprintf("The path %q is not currently exported by package %q:",
esmPackageSubpath, esmPackageName)),

tracker.MsgData(debug.token,
fmt.Sprintf("None of the conditions provided (%s) match any of the currently active conditions (%s):",
prettyPrintConditions(debug.unmatchedConditions),
prettyPrintConditions(unmatchedConditions),
prettyPrintConditions(keys),
))}
)),
}

didSuggestEnablingCondition := false
for _, key := range debug.unmatchedConditions {
if key == "import" && (r.kind == ast.ImportRequire || r.kind == ast.ImportRequireResolve) {
r.debugMeta.suggestionMessage = "Consider using an \"import\" statement to import this file:"
} else if key == "require" && (r.kind == ast.ImportStmt || r.kind == ast.ImportDynamic) {
r.debugMeta.suggestionMessage = "Consider using a \"require()\" call to import this file:"
switch key.Text {
case "import":
if r.kind == ast.ImportRequire || r.kind == ast.ImportRequireResolve {
r.debugMeta.suggestionMessage = "Consider using an \"import\" statement to import this file, " +
"which will work because the \"import\" condition is supported by this package:"
}

case "require":
if r.kind == ast.ImportStmt || r.kind == ast.ImportDynamic {
r.debugMeta.suggestionMessage = "Consider using a \"require()\" call to import this file, " +
"which will work because the \"require\" condition is supported by this package:"
}

default:
if !didSuggestEnablingCondition {
var how string
switch logger.API {
case logger.CLIAPI:
how = fmt.Sprintf("%q", "--conditions="+key.Text)
case logger.JSAPI:
how = fmt.Sprintf("\"conditions: ['%s']\"", key.Text)
case logger.GoAPI:
how = fmt.Sprintf("'Conditions: []string{%q}'", key.Text)
}
r.debugMeta.notes = append(r.debugMeta.notes, tracker.MsgData(key.Range,
fmt.Sprintf("Consider enabling the %q condition if this package expects it to be enabled. "+
"You can use %s to do that.", key.Text, how)))
didSuggestEnablingCondition = true
}
}
}
}
Expand Down

0 comments on commit 0286f66

Please sign in to comment.