fix #1785: don't warn about imports of empty files

CHANGELOG.md

@@ -60,6 +60,12 @@
 
     Notice how esbuild can still collapse rules together when they all share the same unit, even if the unit is one that doesn't have universal browser support such as the unit `Q`. One subtlety is that esbuild now distinguishes between "safe" and "unsafe" units where safe units are old enough that they are guaranteed to work in any browser a user might reasonably use, such as `px`. Safe units are allowed to be collapsed together even if there are multiple different units while multiple different unsafe units are not allowed to be collapsed together. Another detail is that esbuild no longer minifies zero lengths by removing the unit if the unit is unsafe (e.g. `0rem` into `0`) since that could cause a rendering difference if a previously-ignored rule is now no longer ignored due to the unit change. If you are curious, you can learn more about browser support levels for different CSS units in [Mozilla's documentation about CSS length units](https://developer.mozilla.org/en-US/docs/Web/CSS/length).
 
+* Avoid warning about ignored side-effect free imports for empty files ([#1785](https://github.com/evanw/esbuild/issues/1785))
+
+    When bundling, esbuild warns about bare imports such as `import "lodash-es"` when the package has been marked as `"sideEffects": false` in its `package.json` file. This is because the only reason to use a bare import is because you are relying on the side effects of the import, but imports for packages marked as side-effect free are supposed to be removed. If the package indicates that it has no side effects, then this bare import is likely a bug.
+
+    However, some people have packages just for TypeScript type definitions. These package can actually have a side effect as they can augment the type of the global object in TypeScript, even if they are marked with `"sideEffects": false`. To avoid warning in this case, esbuild will now only issue this warning if the imported file is non-empty. If the file is empty, then it's irrelevant whether you import it or not so any import of that file does not indicate a bug. This fixes this case because `.d.ts` files typically end up being empty after esbuild parses them since they typically only contain type declarations.
+
 ## 0.13.14
 
 * Fix dynamic `import()` on node 12.20+ ([#1772](https://github.com/evanw/esbuild/issues/1772))

internal/bundler/bundler.go

@@ -190,26 +190,38 @@ func parseFile(args parseArgs) {
 	switch loader {
 	case config.LoaderJS:
 		ast, ok := args.caches.JSCache.Parse(args.log, source, js_parser.OptionsFromConfig(&args.options))
+		if len(ast.Parts) <= 1 { // Ignore the implicitly-generated namespace export part
+			result.file.inputFile.SideEffects.Kind = graph.NoSideEffects_EmptyAST
+		}
 		result.file.inputFile.Repr = &graph.JSRepr{AST: ast}
 		result.ok = ok
 
 	case config.LoaderJSX:
 		args.options.JSX.Parse = true
 		ast, ok := args.caches.JSCache.Parse(args.log, source, js_parser.OptionsFromConfig(&args.options))
+		if len(ast.Parts) <= 1 { // Ignore the implicitly-generated namespace export part
+			result.file.inputFile.SideEffects.Kind = graph.NoSideEffects_EmptyAST
+		}
 		result.file.inputFile.Repr = &graph.JSRepr{AST: ast}
 		result.ok = ok
 
 	case config.LoaderTS, config.LoaderTSNoAmbiguousLessThan:
 		args.options.TS.Parse = true
 		args.options.TS.NoAmbiguousLessThan = loader == config.LoaderTSNoAmbiguousLessThan
 		ast, ok := args.caches.JSCache.Parse(args.log, source, js_parser.OptionsFromConfig(&args.options))
+		if len(ast.Parts) <= 1 { // Ignore the implicitly-generated namespace export part
+			result.file.inputFile.SideEffects.Kind = graph.NoSideEffects_EmptyAST
+		}
 		result.file.inputFile.Repr = &graph.JSRepr{AST: ast}
 		result.ok = ok
 
 	case config.LoaderTSX:
 		args.options.TS.Parse = true
 		args.options.JSX.Parse = true
 		ast, ok := args.caches.JSCache.Parse(args.log, source, js_parser.OptionsFromConfig(&args.options))
+		if len(ast.Parts) <= 1 { // Ignore the implicitly-generated namespace export part
+			result.file.inputFile.SideEffects.Kind = graph.NoSideEffects_EmptyAST
+		}
 		result.file.inputFile.Repr = &graph.JSRepr{AST: ast}
 		result.ok = ok
 
@@ -1730,7 +1742,12 @@ func (s *scanner) processScannedFiles() []scannerFile {
 						// Do not warn if this is from a plugin, since removing the import
 						// would cause the plugin to not run, and running a plugin is a side
 						// effect.
-						otherModule.SideEffects.Kind != graph.NoSideEffects_PureData_FromPlugin {
+						otherModule.SideEffects.Kind != graph.NoSideEffects_PureData_FromPlugin &&
+
+						// Do not warn if this has no side effects because the parsed AST
+						// is empty. This is the case for ".d.ts" files, for example.
+						otherModule.SideEffects.Kind != graph.NoSideEffects_EmptyAST {
+
 						var notes []logger.MsgData
 						var by string
 						if data := otherModule.SideEffects.Data; data != nil {

internal/bundler/bundler_ts_test.go

@@ -1297,3 +1297,42 @@ func TestTSImportCTS(t *testing.T) {
 		},
 	})
 }
+
+func TestTSSideEffectsFalseWarningTypeDeclarations(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.ts": `
+				import "some-js"
+				import "some-ts"
+				import "empty-js"
+				import "empty-ts"
+				import "empty-dts"
+			`,
+
+			"/node_modules/some-js/package.json": `{ "main": "./foo.js", "sideEffects": false }`,
+			"/node_modules/some-js/foo.js":       `console.log('foo')`,
+
+			"/node_modules/some-ts/package.json": `{ "main": "./foo.ts", "sideEffects": false }`,
+			"/node_modules/some-ts/foo.ts":       `console.log('foo' as string)`,
+
+			"/node_modules/empty-js/package.json": `{ "main": "./foo.js", "sideEffects": false }`,
+			"/node_modules/empty-js/foo.js":       ``,
+
+			"/node_modules/empty-ts/package.json": `{ "main": "./foo.ts", "sideEffects": false }`,
+			"/node_modules/empty-ts/foo.ts":       `export type Foo = number`,
+
+			"/node_modules/empty-dts/package.json": `{ "main": "./foo.d.ts", "sideEffects": false }`,
+			"/node_modules/empty-dts/foo.d.ts":     `export type Foo = number`,
+		},
+		entryPaths: []string{"/entry.ts"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+		},
+		expectedScanLog: `entry.ts: warning: Ignoring this import because "node_modules/some-js/foo.js" was marked as having no side effects
+node_modules/some-js/package.json: note: "sideEffects" is false in the enclosing "package.json" file
+entry.ts: warning: Ignoring this import because "node_modules/some-ts/foo.ts" was marked as having no side effects
+node_modules/some-ts/package.json: note: "sideEffects" is false in the enclosing "package.json" file
+`,
+	})
+}

internal/bundler/snapshots/snapshots_default.txt

@@ -2917,6 +2917,10 @@ TestTSImportMTS
 // imported.mts
 console.log("works");
 
+================================================================================
+TestTSSideEffectsFalseWarningTypeDeclarations
+---------- /out.js ----------
+
 ================================================================================
 TestThisInsideFunction
 ---------- /out.js ----------

internal/graph/input.go

@@ -61,6 +61,10 @@ const (
 	// file in one of our containing directories with a "sideEffects" field.
 	NoSideEffects_PackageJSON
 
+	// This file is considered to have no side effects because the AST was empty
+	// after parsing finished. This should be the case for ".d.ts" files.
+	NoSideEffects_EmptyAST
+
 	// This file was loaded using a data-oriented loader (e.g. "text") that is
 	// known to not have side effects.
 	NoSideEffects_PureData