say if "main" is missing from main fields (#1754)

CHANGELOG.md

@@ -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:

internal/bundler/bundler_packagejson_test.go

@@ -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)
 `,
 	})
 }
@@ -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 []
+`,
+	})
+}

internal/resolver/package_json.go

@@ -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
@@ -68,6 +68,11 @@ type packageJSON struct {
 	exportsMap *pjMap
 }
 
+type mainField struct {
+	keyLoc  logger.Loc
+	relPath string
+}
+
 type browserPathKind uint8
 
 const (
@@ -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 {
@@ -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
 			}
 		}
 	}

internal/resolver/resolver.go

@@ -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
@@ -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
 	}
 
@@ -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
 		}
@@ -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
@@ -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
 }