diff --git a/CHANGELOG.md b/CHANGELOG.md index 52ca4023539..ac65bf238f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,12 @@ This release changes the install script for the `esbuild` npm package to use the "rename a temporary file" approach instead of the "write the file directly" approach to replace the `esbuild` command stub file with the real binary executable. This should hopefully work around a problem with the [pnpm](https://pnpm.js.org/) package manager and its use of hard links. +* Avoid warning about potential issues with `sideEffects` in packages ([#999](https://github.com/evanw/esbuild/issues/999)) + + Bare imports such as `import "foo"` mean the package is only imported for its side effects. Doing this when the package contains `"sideEffects": false` in `package.json` causes a warning because it means esbuild will not import the file since it has been marked as having no side effects, even though the import statement clearly expects it to have side effects. This is usually caused by an incorrect `sideEffects` annotation in the package. + + However, this warning is not immediately actionable if the file containing the import statement is itself in a package. So with this release, esbuild will no longer issue this warning if the file containing the import is inside a `node_modules` folder. Note that even though the warning is no longer there, this situation can still result in a broken bundle if the `sideEffects` annotation is incorrect. + ## 0.9.3 * Fix path resolution with the `exports` field for scoped packages diff --git a/internal/bundler/bundler.go b/internal/bundler/bundler.go index a889108699a..9feaa7464fd 100644 --- a/internal/bundler/bundler.go +++ b/internal/bundler/bundler.go @@ -1434,9 +1434,16 @@ func (s *scanner) processScannedFiles() []file { } } - // Don't include this module for its side effects if it can be - // considered to have no side effects - if record.WasOriginallyBareImport && !s.options.IgnoreDCEAnnotations { + // Warn about this import if it's a bare import statement without any + // imported names (i.e. a side-effect-only import) and the module has + // been marked as having no side effects. + // + // Except don't do this if this file is inside "node_modules" since + // it's a bug in the package and the user won't be able to do anything + // about it. Note that this can result in esbuild silently generating + // broken code. If this actually happens for people, it's probably worth + // re-enabling the warning about code inside "node_modules". + if record.WasOriginallyBareImport && !s.options.IgnoreDCEAnnotations && !resolver.IsInsideNodeModules(result.file.source.KeyPath.Text) { if otherFile := &s.results[record.SourceIndex.GetIndex()].file; otherFile.warnIfUnused { var notes []logger.MsgData if otherFile.warnIfUnusedData != nil { diff --git a/internal/bundler/bundler_dce_test.go b/internal/bundler/bundler_dce_test.go index 6d1e2a951f9..71c98a100ce 100644 --- a/internal/bundler/bundler_dce_test.go +++ b/internal/bundler/bundler_dce_test.go @@ -779,6 +779,35 @@ func TestPackageJsonSideEffectsKeepExportDefaultExpr(t *testing.T) { }) } +func TestPackageJsonSideEffectsFalseNoWarningInNodeModulesIssue999(t *testing.T) { + dce_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import "demo-pkg" + console.log('used import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + import "demo-pkg2" + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg2/index.js": ` + export const foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg2/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + }, + }) +} + func TestJSONLoaderRemoveUnused(t *testing.T) { dce_suite.expectBundled(t, bundled{ files: map[string]string{ diff --git a/internal/bundler/snapshots/snapshots_dce.txt b/internal/bundler/snapshots/snapshots_dce.txt index 8b6d3bd4d10..92cba19481b 100644 --- a/internal/bundler/snapshots/snapshots_dce.txt +++ b/internal/bundler/snapshots/snapshots_dce.txt @@ -310,6 +310,15 @@ console.log("hello"); // Users/user/project/src/entry.js console.log(demo_pkg_exports); +================================================================================ +TestPackageJsonSideEffectsFalseNoWarningInNodeModulesIssue999 +---------- /out.js ---------- +// Users/user/project/node_modules/demo-pkg/index.js +console.log("unused import"); + +// Users/user/project/src/entry.js +console.log("used import"); + ================================================================================ TestPackageJsonSideEffectsFalseRemoveBareImportCommonJS ---------- /out.js ----------