fix #1525: implement TS 4.5 "preserveValueImports"

CHANGELOG.md

@@ -1,5 +1,38 @@
 # Changelog
 
+## Breaking changes
+
+* Add support for TypeScript's `preserveValueImports` setting ([#1525](https://github.com/evanw/esbuild/issues/1525))
+
+    TypeScript 4.5, which was just released, added [a new setting called `preserveValueImports`](https://devblogs.microsoft.com/typescript/announcing-typescript-4-5/#preserve-value-imports). This release of esbuild implements support for this new setting. However, this release also changes esbuild's behavior regarding the `importsNotUsedAsValues` setting, so this release is being considered a breaking change. Now esbuild's behavior should more accurately match the behavior of the TypeScript compiler. This is described in more detail below.
+
+    The difference in behavior is around unused imports. By default, unused import names are considered to be types and are completely removed if they are unused. If all import names are removed for a given import statement, then the whole import statement is removed too. The two `tsconfig.json` settings [`importsNotUsedAsValues`](https://www.typescriptlang.org/tsconfig#importsNotUsedAsValues) and [`preserveValueImports`](https://www.typescriptlang.org/tsconfig#preserveValueImports) let you customize this. Here's what the TypeScript compiler's output looks like with these different settings enabled:
+
+    ```ts
+    // Original code
+    import { unused } from "foo";
+
+    // Default output
+    /* (the import is completely removed) */
+
+    // Output with "importsNotUsedAsValues": "preserve"
+    import "foo";
+
+    // Output with "preserveValueImports": true
+    import { unused } from "foo";
+    ```
+
+    Previously, since the `preserveValueImports` setting didn't exist yet, esbuild had treated the `importsNotUsedAsValues` setting as if it were what is now the `preserveValueImports` setting instead. This was a deliberate deviation from how the TypeScript compiler behaves, but was necessary to allow esbuild to be used as a TypeScript-to-JavaScript compiler inside of certain composite languages such as Svelte and Vue. These languages append additional code after converting the TypeScript to JavaScript so unused imports may actually turn out to be used later on:
+
+    ```svelte
+    <script>
+    import { someFunc } from "./some-module.js";
+    </script>
+    <button on:click={someFunc}>Click me!</button>
+    ```
+
+    Previously the implementers of these languages had to use the `importsNotUsedAsValues` setting as a hack for esbuild to preserve the import statements. With this release, esbuild now follows the behavior of the TypeScript compiler so implementers will need to use the new `preserveValueImports` setting to do this instead. This is the breaking change.
+
 ## 0.13.15
 
 * Fix `super` in lowered `async` arrow functions ([#1777](https://github.com/evanw/esbuild/issues/1777))

internal/bundler/bundler.go

@@ -1114,8 +1114,8 @@ func (s *scanner) maybeParseFile(
 	if resolveResult.UseDefineForClassFieldsTS != config.Unspecified {
 		optionsClone.UseDefineForClassFields = resolveResult.UseDefineForClassFieldsTS
 	}
-	if resolveResult.PreserveUnusedImportsTS {
-		optionsClone.PreserveUnusedImportsTS = true
+	if resolveResult.UnusedImportsTS != config.UnusedImportsRemoveStmt {
+		optionsClone.UnusedImportsTS = resolveResult.UnusedImportsTS
 	}
 	optionsClone.TSTarget = resolveResult.TSTarget
 

internal/bundler/bundler_tsconfig_test.go

@@ -1090,11 +1090,7 @@ func TestTsconfigPreserveUnusedImports(t *testing.T) {
 	})
 }
 
-// This must preserve the import clause even though all imports are not used as
-// values. THIS BEHAVIOR IS A DEVIATION FROM THE TYPESCRIPT COMPILER! It exists
-// to support the use case of compiling partial modules for compile-to-JavaScript
-// languages such as Svelte.
-func TestTsconfigPreserveUnusedImportClause(t *testing.T) {
+func TestTsconfigImportsNotUsedAsValuesPreserve(t *testing.T) {
 	tsconfig_suite.expectBundled(t, bundled{
 		files: map[string]string{
 			"/Users/user/project/src/entry.ts": `
@@ -1123,6 +1119,35 @@ func TestTsconfigPreserveUnusedImportClause(t *testing.T) {
 	})
 }
 
+func TestTsconfigPreserveValueImports(t *testing.T) {
+	tsconfig_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/Users/user/project/src/entry.ts": `
+				import {x, y} from "./foo"
+				import z from "./foo"
+				import * as ns from "./foo"
+				console.log(1 as x, 2 as z, 3 as ns.y)
+			`,
+			"/Users/user/project/src/tsconfig.json": `{
+				"compilerOptions": {
+					"preserveValueImports": true
+				}
+			}`,
+		},
+		entryPaths: []string{"/Users/user/project/src/entry.ts"},
+		options: config.Options{
+			Mode:          config.ModeConvertFormat,
+			OutputFormat:  config.FormatESModule,
+			AbsOutputFile: "/Users/user/project/out.js",
+			ExternalModules: config.ExternalModules{
+				AbsPaths: map[string]bool{
+					"/Users/user/project/src/foo": true,
+				},
+			},
+		},
+	})
+}
+
 func TestTsconfigTarget(t *testing.T) {
 	tsconfig_suite.expectBundled(t, bundled{
 		files: map[string]string{

internal/bundler/snapshots/snapshots_tsconfig.txt

@@ -198,6 +198,14 @@ var test_default = 123;
 // Users/user/project/src/entry.ts
 console.log(test_default);
 
+================================================================================
+TestTsconfigImportsNotUsedAsValuesPreserve
+---------- /Users/user/project/out.js ----------
+import "./foo";
+import "./foo";
+import "./foo";
+console.log(1, 2, 3);
+
 ================================================================================
 TestTsconfigJsonAbsoluteBaseUrl
 ---------- /Users/user/project/out.js ----------
@@ -319,21 +327,21 @@ var require_util = __commonJS({
 var import_util = __toModule(require_util());
 console.log((0, import_util.default)());
 
-================================================================================
-TestTsconfigPreserveUnusedImportClause
----------- /Users/user/project/out.js ----------
-import { x, y } from "./foo";
-import z from "./foo";
-import * as ns from "./foo";
-console.log(1, 2, 3);
-
 ================================================================================
 TestTsconfigPreserveUnusedImports
 ---------- /Users/user/project/out.js ----------
 // Users/user/project/src/entry.ts
 import "./src/foo";
 console.log(1);
 
+================================================================================
+TestTsconfigPreserveValueImports
+---------- /Users/user/project/out.js ----------
+import { x, y } from "./foo";
+import z from "./foo";
+import * as ns from "./foo";
+console.log(1, 2, 3);
+
 ================================================================================
 TestTsconfigRemoveUnusedImports
 ---------- /Users/user/project/out.js ----------

internal/config/config.go

@@ -223,7 +223,7 @@ type Options struct {
 	WriteToStdout bool
 
 	OmitRuntimeForTests     bool
-	PreserveUnusedImportsTS bool
+	UnusedImportsTS         UnusedImportsTS
 	UseDefineForClassFields MaybeBool
 	ASCIIOnly               bool
 	KeepNames               bool
@@ -284,6 +284,29 @@ type Options struct {
 	Stdin *StdinInfo
 }
 
+type UnusedImportsTS uint8
+
+const (
+	// "import { unused } from 'foo'" => "" (TypeScript's default behavior)
+	UnusedImportsRemoveStmt UnusedImportsTS = iota
+
+	// "import { unused } from 'foo'" => "import 'foo'" ("importsNotUsedAsValues" != "remove")
+	UnusedImportsKeepStmtRemoveValues
+
+	// "import { unused } from 'foo'" => "import { unused } from 'foo'" ("preserveValueImports" == true)
+	UnusedImportsKeepValues
+)
+
+func UnusedImportsFromTsconfigValues(preserveImportsNotUsedAsValues bool, preserveValueImports bool) UnusedImportsTS {
+	if preserveValueImports {
+		return UnusedImportsKeepValues
+	}
+	if preserveImportsNotUsedAsValues {
+		return UnusedImportsKeepStmtRemoveValues
+	}
+	return UnusedImportsRemoveStmt
+}
+
 type TSTarget struct {
 	Source                logger.Source
 	Range                 logger.Range

internal/js_parser/js_parser.go

@@ -337,7 +337,7 @@ type optionsThatSupportStructuralEquality struct {
 	omitRuntimeForTests     bool
 	ignoreDCEAnnotations    bool
 	treeShaking             bool
-	preserveUnusedImportsTS bool
+	unusedImportsTS         config.UnusedImportsTS
 	useDefineForClassFields config.MaybeBool
 }
 
@@ -363,7 +363,7 @@ func OptionsFromConfig(options *config.Options) Options {
 			omitRuntimeForTests:     options.OmitRuntimeForTests,
 			ignoreDCEAnnotations:    options.IgnoreDCEAnnotations,
 			treeShaking:             options.TreeShaking,
-			preserveUnusedImportsTS: options.PreserveUnusedImportsTS,
+			unusedImportsTS:         options.UnusedImportsTS,
 			useDefineForClassFields: options.UseDefineForClassFields,
 		},
 	}
@@ -13054,19 +13054,12 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) (result importsEx
 		case *js_ast.SImport:
 			record := &p.importRecords[s.ImportRecordIndex]
 
-			// The official TypeScript compiler always removes unused imported
-			// symbols. However, we deliberately deviate from the official
-			// TypeScript compiler's behavior doing this in a specific scenario:
-			// we are not bundling, symbol renaming is off, and the tsconfig.json
-			// "importsNotUsedAsValues" setting is present and is not set to
-			// "remove".
-			//
-			// This exists to support the use case of compiling partial modules for
-			// compile-to-JavaScript languages such as Svelte. These languages try
-			// to reference imports in ways that are impossible for esbuild to know
-			// about when esbuild is only given a partial module to compile. Here
-			// is an example of some Svelte code that might use esbuild to convert
-			// TypeScript to JavaScript:
+			// We implement TypeScript's "preserveValueImports" tsconfig.json setting
+			// to support the use case of compiling partial modules for compile-to-
+			// JavaScript languages such as Svelte. These languages try to reference
+			// imports in ways that are impossible for TypeScript and esbuild to know
+			// about when they are only given a partial module to compile. Here is an
+			// example of some Svelte code that contains a TypeScript snippet:
 			//
 			//   <script lang="ts">
 			//     import Counter from './Counter.svelte';
@@ -13079,24 +13072,11 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) (result importsEx
 			//
 			// Tools that use esbuild to compile TypeScript code inside a Svelte
 			// file like this only give esbuild the contents of the <script> tag.
-			// These tools work around this missing import problem when using the
-			// official TypeScript compiler by hacking the TypeScript AST to
-			// remove the "unused import" flags. This isn't possible in esbuild
-			// because esbuild deliberately does not expose an AST manipulation
-			// API for performance reasons.
-			//
-			// We deviate from the TypeScript compiler's behavior in this specific
-			// case because doing so is useful for these compile-to-JavaScript
-			// languages and is benign in other cases. The rationale is as follows:
+			// The "preserveValueImports" setting avoids removing unused import
+			// names, which means additional code appended after the TypeScript-
+			// to-JavaScript conversion can still access those unused imports.
 			//
-			//   * If "importsNotUsedAsValues" is absent or set to "remove", then
-			//     we don't know if these imports are values or types. It's not
-			//     safe to keep them because if they are types, the missing imports
-			//     will cause run-time failures because there will be no matching
-			//     exports. It's only safe keep imports if "importsNotUsedAsValues"
-			//     is set to "preserve" or "error" because then we can assume that
-			//     none of the imports are types (since the TypeScript compiler
-			//     would generate an error in that case).
+			// There are two scenarios where we don't do this:
 			//
 			//   * If we're bundling, then we know we aren't being used to compile
 			//     a partial module. The parser is seeing the entire code for the
@@ -13110,7 +13090,7 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) (result importsEx
 			//     user is expecting the output to be as small as possible. So we
 			//     should omit unused imports.
 			//
-			keepUnusedImports := p.options.ts.Parse && p.options.preserveUnusedImportsTS &&
+			keepUnusedImports := p.options.ts.Parse && p.options.unusedImportsTS == config.UnusedImportsKeepValues &&
 				p.options.mode != config.ModeBundle && !p.options.minifyIdentifiers
 
 			// TypeScript always trims unused imports. This is important for
@@ -13197,7 +13177,7 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) (result importsEx
 				//
 				// We do not want to do this culling in JavaScript though because the
 				// module may have side effects even if all imports are unused.
-				if p.options.ts.Parse && foundImports && isUnusedInTypeScript && !p.options.preserveUnusedImportsTS {
+				if p.options.ts.Parse && foundImports && isUnusedInTypeScript && p.options.unusedImportsTS == config.UnusedImportsRemoveStmt {
 					// Ignore import records with a pre-filled source index. These are
 					// for injected files and we definitely do not want to trim these.
 					if !record.SourceIndex.IsValid() {

internal/resolver/resolver.go

@@ -118,10 +118,8 @@ type ResolveResult struct {
 	// If true, the class field transform should use Object.defineProperty().
 	UseDefineForClassFieldsTS config.MaybeBool
 
-	// If true, unused imports are retained in TypeScript code. This matches the
-	// behavior of the "importsNotUsedAsValues" field in "tsconfig.json" when the
-	// value is not "remove".
-	PreserveUnusedImportsTS bool
+	// This is the "importsNotUsedAsValues" and "preserveValueImports" fields from "package.json"
+	UnusedImportsTS config.UnusedImportsTS
 
 	// This is the "type" field from "package.json"
 	ModuleType config.ModuleType
@@ -562,7 +560,10 @@ func (r resolverQuery) finalizeResolve(result *ResolveResult) {
 						result.JSXFactory = dirInfo.enclosingTSConfigJSON.JSXFactory
 						result.JSXFragment = dirInfo.enclosingTSConfigJSON.JSXFragmentFactory
 						result.UseDefineForClassFieldsTS = dirInfo.enclosingTSConfigJSON.UseDefineForClassFields
-						result.PreserveUnusedImportsTS = dirInfo.enclosingTSConfigJSON.PreserveImportsNotUsedAsValues
+						result.UnusedImportsTS = config.UnusedImportsFromTsconfigValues(
+							dirInfo.enclosingTSConfigJSON.PreserveImportsNotUsedAsValues,
+							dirInfo.enclosingTSConfigJSON.PreserveValueImports,
+						)
 						result.TSTarget = dirInfo.enclosingTSConfigJSON.TSTarget
 
 						if r.debugLogs != nil {

internal/resolver/tsconfig_json.go

@@ -40,6 +40,7 @@ type TSConfigJSON struct {
 	TSTarget                       *config.TSTarget
 	UseDefineForClassFields        config.MaybeBool
 	PreserveImportsNotUsedAsValues bool
+	PreserveValueImports           bool
 }
 
 func ParseTSConfigJSON(
@@ -174,6 +175,13 @@ func ParseTSConfigJSON(
 			}
 		}
 
+		// Parse "preserveValueImports"
+		if valueJSON, _, ok := getProperty(compilerOptionsJSON, "preserveValueImports"); ok {
+			if value, ok := getBool(valueJSON); ok {
+				result.PreserveValueImports = value
+			}
+		}
+
 		// Parse "paths"
 		if valueJSON, _, ok := getProperty(compilerOptionsJSON, "paths"); ok {
 			if paths, ok := valueJSON.Data.(*js_ast.EObject); ok {

pkg/api/api_impl.go

@@ -1256,7 +1256,7 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult
 	})
 
 	// Settings from the user come first
-	preserveUnusedImportsTS := false
+	unusedImportsTS := config.UnusedImportsRemoveStmt
 	useDefineForClassFieldsTS := config.Unspecified
 	jsx := config.JSXOptions{
 		Preserve: transformOpts.JSXMode == JSXModePreserve,
@@ -1283,9 +1283,10 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult
 			if result.UseDefineForClassFields != config.Unspecified {
 				useDefineForClassFieldsTS = result.UseDefineForClassFields
 			}
-			if result.PreserveImportsNotUsedAsValues {
-				preserveUnusedImportsTS = true
-			}
+			unusedImportsTS = config.UnusedImportsFromTsconfigValues(
+				result.PreserveImportsNotUsedAsValues,
+				result.PreserveValueImports,
+			)
 			tsTarget = result.TSTarget
 		}
 	}
@@ -1325,7 +1326,7 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult
 		AbsOutputFile:           transformOpts.Sourcefile + "-out",
 		KeepNames:               transformOpts.KeepNames,
 		UseDefineForClassFields: useDefineForClassFieldsTS,
-		PreserveUnusedImportsTS: preserveUnusedImportsTS,
+		UnusedImportsTS:         unusedImportsTS,
 		Stdin: &config.StdinInfo{
 			Loader:     validateLoader(transformOpts.Loader),
 			Contents:   input,