Skip to content

Commit

Permalink
say if "main" is missing from main fields (#1754)
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Nov 9, 2021
1 parent 0441a6b commit 01b65ec
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 11 deletions.
13 changes: 13 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,19 @@

## Unreleased

* Add more information about skipping `"main"` in `package.json` ([#1754](https://github.com/evanw/esbuild/issues/1754))

Configuring `mainFields: []` breaks most npm packages since it tells esbuild to ignore the `"main"` field in `package.json`, which most npm packages use to specify their entry point. This is not a bug with esbuild because esbuild is just doing what it was told to do. However, people may do this without understanding how npm packages work, and then be confused about why it doesn't work. This release now includes additional information in the error message:

```
> foo.js:1:27: error: Could not resolve "events" (use "--platform=node" when building for node)
1 │ var EventEmitter = require('events')
╵ ~~~~~~~~
node_modules/events/package.json:20:2: note: The "main" field was ignored because the list of main fields to use is currently set to []
20 │ "main": "./events.js",
╵ ~~~~~~
```

* Fix a tree-shaking bug with `var exports` ([#1739](https://github.com/evanw/esbuild/issues/1739))

This release fixes a bug where a variable named `var exports = {}` was incorrectly removed by tree-shaking (i.e. dead code elimination). The `exports` variable is a special variable in CommonJS modules that is automatically provided by the CommonJS runtime. CommonJS modules are transformed into something like this before being run:
Expand Down
71 changes: 71 additions & 0 deletions internal/bundler/bundler_packagejson_test.go
Expand Up @@ -1162,6 +1162,7 @@ func TestPackageJsonNeutralNoDefaultMainFields(t *testing.T) {
AbsOutputFile: "/Users/user/project/out.js",
},
expectedScanLog: `Users/user/project/src/entry.js: error: Could not resolve "demo-pkg" (mark it as external to exclude it from the bundle)
Users/user/project/node_modules/demo-pkg/package.json: note: The "main" field was ignored (main fields must be configured manually when using the "neutral" platform)
`,
})
}
Expand Down Expand Up @@ -2088,3 +2089,73 @@ Users/user/project/src/package.json: note: This "imports" map was ignored becaus
`,
})
}

func TestPackageJsonMainFieldsErrorMessageDefault(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import 'foo'
`,
"/Users/user/project/node_modules/foo/package.json": `
{
"main": "./foo"
}
`,
},
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 "foo" (mark it as external to exclude it from the bundle)
`,
})
}

func TestPackageJsonMainFieldsErrorMessageNotIncluded(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import 'foo'
`,
"/Users/user/project/node_modules/foo/package.json": `
{
"main": "./foo"
}
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/Users/user/project/out.js",
MainFields: []string{"some", "fields"},
},
expectedScanLog: `Users/user/project/src/entry.js: error: Could not resolve "foo" (mark it as external to exclude it from the bundle)
Users/user/project/node_modules/foo/package.json: note: The "main" field was ignored because the list of main fields to use is currently set to ["some", "fields"]
`,
})
}

func TestPackageJsonMainFieldsErrorMessageEmpty(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import 'foo'
`,
"/Users/user/project/node_modules/foo/package.json": `
{
"main": "./foo"
}
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/Users/user/project/out.js",
MainFields: []string{},
},
expectedScanLog: `Users/user/project/src/entry.js: error: Could not resolve "foo" (mark it as external to exclude it from the bundle)
Users/user/project/node_modules/foo/package.json: note: The "main" field was ignored because the list of main fields to use is currently set to []
`,
})
}
26 changes: 20 additions & 6 deletions internal/resolver/package_json.go
Expand Up @@ -17,7 +17,7 @@ import (

type packageJSON struct {
source logger.Source
mainFields map[string]string
mainFields map[string]mainField
moduleType config.ModuleType

// Present if the "browser" field is present. This field is intended to be
Expand Down Expand Up @@ -68,6 +68,11 @@ type packageJSON struct {
exportsMap *pjMap
}

type mainField struct {
keyLoc logger.Loc
relPath string
}

type browserPathKind uint8

const (
Expand Down Expand Up @@ -238,7 +243,10 @@ func (r resolverQuery) parsePackageJSON(inputPath string) *packageJSON {
return nil
}

packageJSON := &packageJSON{source: jsonSource}
packageJSON := &packageJSON{
source: jsonSource,
mainFields: make(map[string]mainField),
}

// Read the "type" field
if typeJSON, _, ok := getProperty(json, "type"); ok {
Expand All @@ -264,12 +272,18 @@ func (r resolverQuery) parsePackageJSON(inputPath string) *packageJSON {
mainFields = defaultMainFields[r.options.Platform]
}
for _, field := range mainFields {
if mainJSON, _, ok := getProperty(json, field); ok {
if mainJSON, mainRange, ok := getProperty(json, field); ok {
if main, ok := getString(mainJSON); ok && main != "" {
if packageJSON.mainFields == nil {
packageJSON.mainFields = make(map[string]string)
packageJSON.mainFields[field] = mainField{mainRange, main}
}
}
}
for _, field := range mainFieldsForFailure {
if _, ok := packageJSON.mainFields[field]; !ok {
if mainJSON, mainRange, ok := getProperty(json, field); ok {
if main, ok := getString(mainJSON); ok && main != "" {
packageJSON.mainFields[field] = mainField{mainRange, main}
}
packageJSON.mainFields[field] = main
}
}
}
Expand Down
43 changes: 38 additions & 5 deletions internal/resolver/resolver.go
Expand Up @@ -54,6 +54,10 @@ var defaultMainFields = map[config.Platform][]string{
config.PlatformNeutral: {},
}

// These are the main fields to use when the "main fields" setting is configured
// to something unusual, such as something without the "main" field.
var mainFieldsForFailure = []string{"main", "module"}

// Path resolution is a mess. One tricky issue is the "module" override for the
// "main" field in "package.json" files. Bundlers generally prefer "module" over
// "main" but that breaks packages that export a function in "main" for use with
Expand Down Expand Up @@ -1250,7 +1254,7 @@ func (r resolverQuery) loadAsFileOrDirectory(path string) (PathPair, bool, *fs.D
}

func (r resolverQuery) loadAsMainField(dirInfo *dirInfo, path string, extensionOrder []string) (PathPair, bool, *fs.DifferentCase) {
if dirInfo.packageJSON == nil || dirInfo.packageJSON.mainFields == nil {
if dirInfo.packageJSON == nil {
return PathPair{}, false, nil
}

Expand Down Expand Up @@ -1299,18 +1303,23 @@ func (r resolverQuery) loadAsMainField(dirInfo *dirInfo, path string, extensionO

if r.debugLogs != nil {
r.debugLogs.addNote(fmt.Sprintf("Searching for main fields in %q", dirInfo.packageJSON.source.KeyPath.Text))
r.debugLogs.increaseIndent()
defer r.debugLogs.decreaseIndent()
}

foundSomething := false

for _, key := range mainFieldKeys {
fieldRelPath, ok := mainFieldValues[key]
value, ok := mainFieldValues[key]
if !ok {
if r.debugLogs != nil {
r.debugLogs.addNote(fmt.Sprintf("Did not find main field %q", key))
}
continue
}
foundSomething = true

absolute, ok, diffCase := loadMainField(fieldRelPath, key)
absolute, ok, diffCase := loadMainField(value.relPath, key)
if !ok {
continue
}
Expand All @@ -1324,8 +1333,8 @@ func (r resolverQuery) loadAsMainField(dirInfo *dirInfo, path string, extensionO
var okMain bool
var diffCaseMain *fs.DifferentCase

if mainRelPath, ok := mainFieldValues["main"]; ok {
if absolute, ok, diffCase := loadMainField(mainRelPath, "main"); ok {
if main, ok := mainFieldValues["main"]; ok {
if absolute, ok, diffCase := loadMainField(main.relPath, "main"); ok {
absoluteMain = absolute
okMain = true
diffCaseMain = diffCase
Expand Down Expand Up @@ -1377,6 +1386,30 @@ func (r resolverQuery) loadAsMainField(dirInfo *dirInfo, path string, extensionO
return absolute, true, diffCase
}

// Let the user know if "main" exists but was skipped due to mis-configuration
if !foundSomething {
for _, field := range mainFieldsForFailure {
if main, ok := mainFieldValues[field]; ok {
tracker := logger.MakeLineColumnTracker(&dirInfo.packageJSON.source)
keyRange := dirInfo.packageJSON.source.RangeOfString(main.keyLoc)
if len(mainFieldKeys) == 0 && r.options.Platform == config.PlatformNeutral {
r.debugMeta.notes = append(r.debugMeta.notes, logger.RangeData(&tracker, keyRange,
fmt.Sprintf("The %q field was ignored (main fields must be configured manually when using the \"neutral\" platform)",
field)))
} else {
quoted := make([]string, len(mainFieldKeys))
for i, key := range mainFieldKeys {
quoted[i] = fmt.Sprintf("%q", key)
}
r.debugMeta.notes = append(r.debugMeta.notes, logger.RangeData(&tracker, keyRange,
fmt.Sprintf("The %q field was ignored because the list of main fields to use is currently set to [%s]",
field, strings.Join(quoted, ", "))))
}
break
}
}
}

return PathPair{}, false, nil
}

Expand Down

0 comments on commit 01b65ec

Please sign in to comment.