Skip to content

Commit

Permalink
fix #474: remove unused import aliases
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Nov 11, 2020
1 parent a370d53 commit 7fe623e
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 29 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@

However, minification renames symbols to reduce code size. That changes value of the `name` property for many of these cases. This is usually fine because the `name` property is normally only used for debugging. However, some frameworks rely on the `name` property for registration and binding purposes. If this is the case, you can now enable `--keep-names` to preserve the original `name` values even in minified code.

* Omit unused TypeScript import assignment aliases ([#474](https://github.com/evanw/esbuild/issues/474))

In TypeScript, `import x = y` is an alias statement that works for both values and types and can reach across files. Because esbuild doesn't replicate TypeScript's type system and because esbuild converts each file from TypeScript to JavaScript independently, it's not clear to esbuild if the alias refers to a value and should be kept as JavaScript or if the alias refers to a type and should be removed.
Previously all import aliases were kept in the generated JavaScript. This could lead to problems if the alias actually referred to a type. Now import aliases are only kept if they are used as values. This way import aliases that are only used as types will be automatically removed. This doesn't exactly match what the TypeScript compiler does in complex scenarios but it should work for many real-world cases.

* Validate that on-resolve plugins return absolute paths in the `file` namespace

The default path namespace for on-resolve plugins is the `file` namespace. Paths in this namespace are expected to be absolute paths. This is now enforced. If the returned path is not supposed to be a file system path, you should set a namespace other than `file` so esbuild doesn't treat it as a file system path.
Expand Down
4 changes: 2 additions & 2 deletions internal/js_ast/js_ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -936,8 +936,8 @@ type SLocal struct {
IsExport bool

// The TypeScript compiler doesn't generate code for "import foo = bar"
// statements inside namespaces where the import is never used.
WasTSImportEqualsInNamespace bool
// statements where the import is never used.
WasTSImportEquals bool
}

type SBreak struct {
Expand Down
102 changes: 81 additions & 21 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -9616,7 +9616,13 @@ func (p *parser) recordExportedBinding(binding js_ast.Binding) {
}
}

func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) []js_ast.Stmt {
type scanForImportsAndExportsResult struct {
stmts []js_ast.Stmt
keptImportEquals bool
removedImportEquals bool
}

func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) (result scanForImportsAndExportsResult) {
stmtsEnd := 0

for _, stmt := range stmts {
Expand Down Expand Up @@ -9860,6 +9866,40 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) []js_ast.Stmt {
}
}

// Remove unused import-equals statements, since those likely
// correspond to types instead of values
if s.WasTSImportEquals && !s.IsExport {
decl := s.Decls[0]

// Skip to the underlying reference
value := *s.Decls[0].Value
for {
if dot, ok := value.Data.(*js_ast.EDot); ok {
value = dot.Target
} else {
break
}
}

// Is this an identifier reference and not a require() call?
if id, ok := value.Data.(*js_ast.EIdentifier); ok {
// Is this import statement unused?
if ref := decl.Binding.Data.(*js_ast.BIdentifier).Ref; p.symbols[ref.InnerIndex].UseCountEstimate == 0 {
// Also don't count the referenced identifier
p.ignoreUsage(id.Ref)

// Import-equals statements can come in any order. Removing one
// could potentially cause another one to be removable too.
// Continue iterating until a fixed point has been reached to make
// sure we get them all.
result.removedImportEquals = true
continue
} else {
result.keptImportEquals = true
}
}
}

case *js_ast.SExportDefault:
p.recordExport(s.DefaultName.Loc, "default", s.DefaultName.Ref)

Expand Down Expand Up @@ -9909,7 +9949,8 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) []js_ast.Stmt {
stmtsEnd++
}

return stmts[:stmtsEnd]
result.stmts = stmts[:stmtsEnd]
return
}

func (p *parser) appendPart(parts []js_ast.Part, stmts []js_ast.Stmt) []js_ast.Part {
Expand Down Expand Up @@ -10715,25 +10756,44 @@ func (p *parser) toAST(source logger.Source, parts []js_ast.Part, hashbang strin
// Handle import paths after the whole file has been visited because we need
// symbol usage counts to be able to remove unused type-only imports in
// TypeScript code.
partsEnd := 0
for _, part := range parts {
p.importRecordsForCurrentPart = nil
p.declaredSymbols = nil
part.Stmts = p.scanForImportsAndExports(part.Stmts)
part.ImportRecordIndices = append(part.ImportRecordIndices, p.importRecordsForCurrentPart...)
part.DeclaredSymbols = append(part.DeclaredSymbols, p.declaredSymbols...)
if len(part.Stmts) > 0 {
if p.moduleScope.ContainsDirectEval && len(part.DeclaredSymbols) > 0 {
// If this file contains a direct call to "eval()", all parts that
// declare top-level symbols must be kept since the eval'd code may
// reference those symbols.
part.CanBeRemovedIfUnused = false
}
parts[partsEnd] = part
partsEnd++
}
}
parts = parts[:partsEnd]
for {
keptImportEquals := false
removedImportEquals := false

// Potentially remove some statements, then filter out parts to remove any
// with no statements
partsEnd := 0
for _, part := range parts {
p.importRecordsForCurrentPart = nil
p.declaredSymbols = nil

result := p.scanForImportsAndExports(part.Stmts)
part.Stmts = result.stmts
keptImportEquals = keptImportEquals || result.keptImportEquals
removedImportEquals = removedImportEquals || result.removedImportEquals

part.ImportRecordIndices = append(part.ImportRecordIndices, p.importRecordsForCurrentPart...)
part.DeclaredSymbols = append(part.DeclaredSymbols, p.declaredSymbols...)

if len(part.Stmts) > 0 {
if p.moduleScope.ContainsDirectEval && len(part.DeclaredSymbols) > 0 {
// If this file contains a direct call to "eval()", all parts that
// declare top-level symbols must be kept since the eval'd code may
// reference those symbols.
part.CanBeRemovedIfUnused = false
}
parts[partsEnd] = part
partsEnd++
}
}
parts = parts[:partsEnd]

// We need to iterate multiple times if an import-equals statement was
// removed and there are more import-equals statements that may be removed
if !keptImportEquals || !removedImportEquals {
break
}
}

// Do a second pass for exported items now that imported items are filled out
for _, part := range parts {
Expand Down
10 changes: 5 additions & 5 deletions internal/js_parser/ts_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -864,10 +864,10 @@ func (p *parser) parseTypeScriptImportEqualsStmt(loc logger.Loc, opts parseStmtO
}}

return js_ast.Stmt{Loc: loc, Data: &js_ast.SLocal{
Kind: kind,
Decls: decls,
IsExport: opts.isExport,
WasTSImportEqualsInNamespace: opts.isNamespaceScope,
Kind: kind,
Decls: decls,
IsExport: opts.isExport,
WasTSImportEquals: true,
}}
}

Expand Down Expand Up @@ -906,7 +906,7 @@ func (p *parser) parseTypeScriptNamespaceStmt(loc logger.Loc, opts parseStmtOpts
// to be considered empty and thus be removed.
importEqualsCount := 0
for _, stmt := range stmts {
if local, ok := stmt.Data.(*js_ast.SLocal); ok && local.WasTSImportEqualsInNamespace && !local.IsExport {
if local, ok := stmt.Data.(*js_ast.SLocal); ok && local.WasTSImportEquals && !local.IsExport {
importEqualsCount++
}
}
Expand Down
9 changes: 8 additions & 1 deletion internal/js_parser/ts_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1275,9 +1275,16 @@ func TestTSTypeOnlyImport(t *testing.T) {
expectPrintedTS(t, "import type {foo, bar as baz} from 'bar'; x", "x;\n")
expectPrintedTS(t, "import type {foo, bar as baz} from 'bar'\nx", "x;\n")

expectPrintedTS(t, "import type = bar", "var type = bar;\n")
expectPrintedTS(t, "import type = bar; type", "var type = bar;\ntype;\n")
expectPrintedTS(t, "import type = foo.bar; type", "var type = foo.bar;\ntype;\n")
expectPrintedTS(t, "import type = require('type'); type", "const type = require(\"type\");\ntype;\n")
expectPrintedTS(t, "import type from 'bar'; type", "import type from \"bar\";\ntype;\n")

expectPrintedTS(t, "import a = b; import c = a.c", "")
expectPrintedTS(t, "import c = a.c; import a = b", "")
expectPrintedTS(t, "import a = b; import c = a.c; c()", "var a = b;\nvar c = a.c;\nc();\n")
expectPrintedTS(t, "import c = a.c; import a = b; c()", "var c = a.c;\nvar a = b;\nc();\n")

expectParseErrorTS(t, "import type", "<stdin>: error: Expected \"from\" but found end of file\n")
expectParseErrorTS(t, "import type * foo", "<stdin>: error: Expected \"as\" but found \"foo\"\n")
expectParseErrorTS(t, "import type * as 'bar'", "<stdin>: error: Expected identifier but found \"'bar'\"\n")
Expand Down

0 comments on commit 7fe623e

Please sign in to comment.