diff --git a/CHANGELOG.md b/CHANGELOG.md index 7557e7b77c8..80fe5cec588 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,33 @@ ## Unreleased +* Correct esbuild's implementation of `"preserveValueImports": true` ([#2268](https://github.com/evanw/esbuild/issues/2268)) + + TypeScript's [`preserveValueImports` setting](https://www.typescriptlang.org/tsconfig#preserveValueImports) tells the compiler to preserve unused imports, which can sometimes be necessary because otherwise TypeScript will remove unused imports as it assumes they are type annotations. This setting is useful for programming environments that strip TypeScript types as part of a larger code transformation where additional code is appended later that will then make use of those unused imports, such as with [Svelte](https://svelte.dev/) or [Vue](https://vuejs.org/). + + This release fixes an issue where esbuild's implementation of `preserveValueImports` diverged from the official TypeScript compiler. If the import clause is present but empty of values (even if it contains types), then the import clause should be considered a type-only import clause. This was an oversight, and has now been fixed: + + ```ts + // Original code + import "keep" + import { k1 } from "keep" + import k2, { type t1 } from "keep" + import {} from "remove" + import { type t2 } from "remove" + + // Old output under "preserveValueImports": true + import "keep"; + import { k1 } from "keep"; + import k2, {} from "keep"; + import {} from "remove"; + import {} from "remove"; + + // New output under "preserveValueImports": true (matches the TypeScript compiler) + import "keep"; + import { k1 } from "keep"; + import k2 from "keep"; + ``` + * Add Opera to more internal feature compatibility tables ([#2247](https://github.com/evanw/esbuild/issues/2247), [#2252](https://github.com/evanw/esbuild/pull/2252)) The internal compatibility tables that esbuild uses to determine which environments support which features are derived from multiple sources. Most of it is automatically derived from [these ECMAScript compatibility tables](https://kangax.github.io/compat-table/), but missing information is manually copied from [MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/), GitHub PR comments, and various other websites. Version 0.14.35 of esbuild introduced Opera as a possible target environment which was automatically picked up by the compatibility table script, but the manually-copied information wasn't updated to include Opera. This release fixes this omission so Opera feature compatibility should now be accurate. diff --git a/internal/bundler/bundler.go b/internal/bundler/bundler.go index b7d002458e6..cdb77582c66 100644 --- a/internal/bundler/bundler.go +++ b/internal/bundler/bundler.go @@ -1185,8 +1185,8 @@ func (s *scanner) maybeParseFile( if resolveResult.UseDefineForClassFieldsTS != config.Unspecified { optionsClone.UseDefineForClassFields = resolveResult.UseDefineForClassFieldsTS } - if resolveResult.UnusedImportsTS != config.UnusedImportsRemoveStmt { - optionsClone.UnusedImportsTS = resolveResult.UnusedImportsTS + if resolveResult.UnusedImportFlagsTS != 0 { + optionsClone.UnusedImportFlagsTS = resolveResult.UnusedImportFlagsTS } optionsClone.TSTarget = resolveResult.TSTarget diff --git a/internal/bundler/bundler_tsconfig_test.go b/internal/bundler/bundler_tsconfig_test.go index 8f556a2b766..10711a974a9 100644 --- a/internal/bundler/bundler_tsconfig_test.go +++ b/internal/bundler/bundler_tsconfig_test.go @@ -1162,13 +1162,55 @@ 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) + import {} from "a" + import {b1} from "b" + import {c1, type c2} from "c" + import {d1, d2, type d3} from "d" + import {type e1, type e2} from "e" + import f1, {} from "f" + import g1, {g2} from "g" + import h1, {type h2} from "h" + import * as i1 from "i" + import "j" + `, + "/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", + ExternalSettings: config.ExternalSettings{ + PostResolve: config.ExternalMatchers{Exact: map[string]bool{ + "/Users/user/project/src/foo": true, + }}, + }, + }, + }) +} + +func TestTsconfigPreserveValueImportsAndImportsNotUsedAsValuesPreserve(t *testing.T) { + tsconfig_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.ts": ` + import {} from "a" + import {b1} from "b" + import {c1, type c2} from "c" + import {d1, d2, type d3} from "d" + import {type e1, type e2} from "e" + import f1, {} from "f" + import g1, {g2} from "g" + import h1, {type h2} from "h" + import * as i1 from "i" + import "j" `, "/Users/user/project/src/tsconfig.json": `{ "compilerOptions": { + "importsNotUsedAsValues": "preserve", "preserveValueImports": true } }`, diff --git a/internal/bundler/snapshots/snapshots_tsconfig.txt b/internal/bundler/snapshots/snapshots_tsconfig.txt index ab1eb83f75e..d433817ef1f 100644 --- a/internal/bundler/snapshots/snapshots_tsconfig.txt +++ b/internal/bundler/snapshots/snapshots_tsconfig.txt @@ -393,10 +393,28 @@ 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); +import { b1 } from "b"; +import { c1 } from "c"; +import { d1, d2 } from "d"; +import f1 from "f"; +import g1, { g2 } from "g"; +import h1 from "h"; +import * as i1 from "i"; +import "j"; + +================================================================================ +TestTsconfigPreserveValueImportsAndImportsNotUsedAsValuesPreserve +---------- /Users/user/project/out.js ---------- +import {} from "a"; +import { b1 } from "b"; +import { c1 } from "c"; +import { d1, d2 } from "d"; +import {} from "e"; +import f1, {} from "f"; +import g1, { g2 } from "g"; +import h1, {} from "h"; +import * as i1 from "i"; +import "j"; ================================================================================ TestTsconfigRemoveUnusedImports diff --git a/internal/config/config.go b/internal/config/config.go index dfb2b53c912..9b9cfbf847b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -274,7 +274,7 @@ type Options struct { WriteToStdout bool OmitRuntimeForTests bool - UnusedImportsTS UnusedImportsTS + UnusedImportFlagsTS UnusedImportFlagsTS UseDefineForClassFields MaybeBool ASCIIOnly bool KeepNames bool @@ -303,27 +303,49 @@ const ( TargetWasConfiguredAndAtLeastES2022 ) -type UnusedImportsTS uint8 - +type UnusedImportFlagsTS uint8 + +// With !UnusedImportKeepStmt && !UnusedImportKeepValues: +// +// "import 'foo'" => "import 'foo'" +// "import * as unused from 'foo'" => "" +// "import { unused } from 'foo'" => "" +// "import { type unused } from 'foo'" => "" +// +// With UnusedImportKeepStmt && !UnusedImportKeepValues: +// +// "import 'foo'" => "import 'foo'" +// "import * as unused from 'foo'" => "import 'foo'" +// "import { unused } from 'foo'" => "import 'foo'" +// "import { type unused } from 'foo'" => "import 'foo'" +// +// With !UnusedImportKeepStmt && UnusedImportKeepValues: +// +// "import 'foo'" => "import 'foo'" +// "import * as unused from 'foo'" => "import * as unused from 'foo'" +// "import { unused } from 'foo'" => "import { unused } from 'foo'" +// "import { type unused } from 'foo'" => "" +// +// With UnusedImportKeepStmt && UnusedImportKeepValues: +// +// "import 'foo'" => "import 'foo'" +// "import * as unused from 'foo'" => "import * as unused from 'foo'" +// "import { unused } from 'foo'" => "import { unused } from 'foo'" +// "import { type unused } from 'foo'" => "import {} from 'foo'" +// 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 + UnusedImportKeepStmt UnusedImportFlagsTS = 1 << iota // "importsNotUsedAsValues" != "remove" + UnusedImportKeepValues // "preserveValueImports" == true ) -func UnusedImportsFromTsconfigValues(preserveImportsNotUsedAsValues bool, preserveValueImports bool) UnusedImportsTS { +func UnusedImportFlagsFromTsconfigValues(preserveImportsNotUsedAsValues bool, preserveValueImports bool) (flags UnusedImportFlagsTS) { if preserveValueImports { - return UnusedImportsKeepValues + flags |= UnusedImportKeepValues } if preserveImportsNotUsedAsValues { - return UnusedImportsKeepStmtRemoveValues + flags |= UnusedImportKeepStmt } - return UnusedImportsRemoveStmt + return } type TSTarget struct { diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 41b5ec06988..07c742c7f51 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -395,7 +395,7 @@ type optionsThatSupportStructuralEquality struct { treeShaking bool dropDebugger bool mangleQuoted bool - unusedImportsTS config.UnusedImportsTS + unusedImportFlagsTS config.UnusedImportFlagsTS useDefineForClassFields config.MaybeBool } @@ -426,7 +426,7 @@ func OptionsFromConfig(options *config.Options) Options { treeShaking: options.TreeShaking, dropDebugger: options.DropDebugger, mangleQuoted: options.MangleQuoted, - unusedImportsTS: options.UnusedImportsTS, + unusedImportFlagsTS: options.UnusedImportFlagsTS, useDefineForClassFields: options.UseDefineForClassFields, }, } @@ -6628,11 +6628,38 @@ func (p *parser) parseStmt(opts parseStmtOpts) js_ast.Stmt { } pathLoc, pathText, assertions := p.parsePath() + p.lexer.ExpectOrInsertSemicolon() + + // If TypeScript's "preserveValueImports": true setting is active, TypeScript's + // "importsNotUsedAsValues": "preserve" setting is NOT active, and the import + // clause is present and empty (or is non-empty but filled with type-only + // items), then the import statement should still be removed entirely to match + // the behavior of the TypeScript compiler: + // + // // Keep these + // import 'x' + // import { y } from 'x' + // import { y, type z } from 'x' + // + // // Remove these + // import {} from 'x' + // import { type y } from 'x' + // + // // Remove the items from these + // import d, {} from 'x' + // import d, { type y } from 'x' + // + if p.options.ts.Parse && p.options.unusedImportFlagsTS == config.UnusedImportKeepValues && stmt.Items != nil && len(*stmt.Items) == 0 { + if stmt.DefaultName == nil { + return js_ast.Stmt{Loc: loc, Data: &js_ast.STypeScript{}} + } + stmt.Items = nil + } + stmt.ImportRecordIndex = p.addImportRecord(ast.ImportStmt, pathLoc, pathText, assertions) if wasOriginallyBareImport { p.importRecords[stmt.ImportRecordIndex].Flags |= ast.WasOriginallyBareImport } - p.lexer.ExpectOrInsertSemicolon() if stmt.StarNameLoc != nil { name := p.loadNameFromRef(stmt.NamespaceRef) @@ -14164,7 +14191,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.unusedImportsTS == config.UnusedImportsKeepValues && + keepUnusedImports := p.options.ts.Parse && (p.options.unusedImportFlagsTS&config.UnusedImportKeepValues) != 0 && p.options.mode != config.ModeBundle && !p.options.minifyIdentifiers // TypeScript always trims unused imports. This is important for @@ -14180,7 +14207,7 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) (result importsEx symbol := p.symbols[s.DefaultName.Ref.InnerIndex] // TypeScript has a separate definition of unused - if p.options.ts.Parse && p.tsUseCounts[s.DefaultName.Ref.InnerIndex] != 0 { + if p.options.ts.Parse && (p.tsUseCounts[s.DefaultName.Ref.InnerIndex] != 0 || (p.options.unusedImportFlagsTS&config.UnusedImportKeepValues) != 0) { isUnusedInTypeScript = false } @@ -14196,7 +14223,7 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) (result importsEx symbol := p.symbols[s.NamespaceRef.InnerIndex] // TypeScript has a separate definition of unused - if p.options.ts.Parse && p.tsUseCounts[s.NamespaceRef.InnerIndex] != 0 { + if p.options.ts.Parse && (p.tsUseCounts[s.NamespaceRef.InnerIndex] != 0 || (p.options.unusedImportFlagsTS&config.UnusedImportKeepValues) != 0) { isUnusedInTypeScript = false } @@ -14219,7 +14246,7 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) (result importsEx symbol := p.symbols[item.Name.Ref.InnerIndex] // TypeScript has a separate definition of unused - if p.options.ts.Parse && p.tsUseCounts[item.Name.Ref.InnerIndex] != 0 { + if p.options.ts.Parse && (p.tsUseCounts[item.Name.Ref.InnerIndex] != 0 || (p.options.unusedImportFlagsTS&config.UnusedImportKeepValues) != 0) { isUnusedInTypeScript = false } @@ -14251,7 +14278,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.unusedImportsTS == config.UnusedImportsRemoveStmt { + if p.options.ts.Parse && foundImports && isUnusedInTypeScript && (p.options.unusedImportFlagsTS&config.UnusedImportKeepStmt) == 0 { // 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() { diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index eb59a824ace..a896efa3061 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -122,7 +122,7 @@ type ResolveResult struct { UseDefineForClassFieldsTS config.MaybeBool // This is the "importsNotUsedAsValues" and "preserveValueImports" fields from "package.json" - UnusedImportsTS config.UnusedImportsTS + UnusedImportFlagsTS config.UnusedImportFlagsTS } type DebugMeta struct { @@ -626,7 +626,7 @@ func (r resolverQuery) finalizeResolve(result *ResolveResult) { result.JSXFactory = dirInfo.enclosingTSConfigJSON.JSXFactory result.JSXFragment = dirInfo.enclosingTSConfigJSON.JSXFragmentFactory result.UseDefineForClassFieldsTS = dirInfo.enclosingTSConfigJSON.UseDefineForClassFields - result.UnusedImportsTS = config.UnusedImportsFromTsconfigValues( + result.UnusedImportFlagsTS = config.UnusedImportFlagsFromTsconfigValues( dirInfo.enclosingTSConfigJSON.PreserveImportsNotUsedAsValues, dirInfo.enclosingTSConfigJSON.PreserveValueImports, ) diff --git a/pkg/api/api_impl.go b/pkg/api/api_impl.go index 56ddbd2e1d2..8268674c022 100644 --- a/pkg/api/api_impl.go +++ b/pkg/api/api_impl.go @@ -1361,7 +1361,7 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult }) // Settings from the user come first - unusedImportsTS := config.UnusedImportsRemoveStmt + var unusedImportFlagsTS config.UnusedImportFlagsTS useDefineForClassFieldsTS := config.Unspecified jsx := config.JSXOptions{ Preserve: transformOpts.JSXMode == JSXModePreserve, @@ -1388,7 +1388,7 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult if result.UseDefineForClassFields != config.Unspecified { useDefineForClassFieldsTS = result.UseDefineForClassFields } - unusedImportsTS = config.UnusedImportsFromTsconfigValues( + unusedImportFlagsTS = config.UnusedImportFlagsFromTsconfigValues( result.PreserveImportsNotUsedAsValues, result.PreserveValueImports, ) @@ -1436,7 +1436,7 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult AbsOutputFile: transformOpts.Sourcefile + "-out", KeepNames: transformOpts.KeepNames, UseDefineForClassFields: useDefineForClassFieldsTS, - UnusedImportsTS: unusedImportsTS, + UnusedImportFlagsTS: unusedImportFlagsTS, Stdin: &config.StdinInfo{ Loader: validateLoader(transformOpts.Loader), Contents: input,