diff --git a/CHANGELOG.md b/CHANGELOG.md index ab45b2b79d0..d0047ddc767 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/internal/bundler/bundler_packagejson_test.go b/internal/bundler/bundler_packagejson_test.go index c00446f06da..9f1eba520f4 100644 --- a/internal/bundler/bundler_packagejson_test.go +++ b/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 [] +`, + }) +} diff --git a/internal/resolver/package_json.go b/internal/resolver/package_json.go index c7d9725eb93..414483c8f23 100644 --- a/internal/resolver/package_json.go +++ b/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 } } } diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 3d38892f69d..e121b22c3c7 100644 --- a/internal/resolver/resolver.go +++ b/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 }