Skip to content

Fix import elision with type-only usage #2646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 11 commits into from
Closed

Fix import elision with type-only usage #2646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a92014d
Initial plan
Copilot Feb 2, 2026
af21080
Add test case for import value only used as type bug
Copilot Feb 2, 2026
d45ec2b
Add investigation notes for import elision bug
Copilot Feb 3, 2026
c27b95b
Update investigation with root cause analysis
Copilot Feb 3, 2026
2c64cc5
Add minimal reproduction test case for import elision bug
Copilot Feb 3, 2026
d3ac5f3
Add investigation summary document
Copilot Feb 3, 2026
2581d1b
Add IsTypeOnly check for import specifiers in import elision transformer
Copilot Feb 3, 2026
e624128
Fix import elision when import clause was modified by previous transf…
Copilot Feb 3, 2026
3a0ebb7
Remove redundant test case importValueOnlyUsedAsType
Copilot Feb 3, 2026
400d9cd
Simplify import elision logic by removing redundant isElisionBlocked …
Copilot Feb 3, 2026
df2e210
Apply suggestion from @jakebailey
jakebailey Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions internal/transformers/tstransforms/importelision.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,15 @@ func (tx *ImportElisionTransformer) visit(node *ast.Node) *ast.Node {
}
return tx.Visitor().VisitEachChild(node)
case ast.KindImportDeclaration:
if !tx.isElisionBlocked(node) {
n := node.AsImportDeclaration()
// Do not elide a side-effect only import declaration.
// import "foo";
if n.ImportClause != nil {
importClause := tx.Visitor().VisitNode(n.ImportClause)
if importClause == nil {
return nil
}
return tx.Factory().UpdateImportDeclaration(n, n.Modifiers(), importClause, n.ModuleSpecifier, tx.Visitor().VisitNode(n.Attributes))
n := node.AsImportDeclaration()
// Do not elide a side-effect only import declaration.
// import "foo";
if n.ImportClause != nil {
importClause := tx.Visitor().VisitNode(n.ImportClause)
if importClause == nil {
return nil
}
return tx.Factory().UpdateImportDeclaration(n, n.Modifiers(), importClause, n.ModuleSpecifier, tx.Visitor().VisitNode(n.Attributes))
}
return tx.Visitor().VisitEachChild(node)
Comment on lines 42 to 53
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description says the fix is to elide imports even when isElisionBlocked is true only when the visited ImportClause becomes nil, but the implementation removes the isElisionBlocked gate for all ImportDeclarations. This changes behavior for any import declaration that was modified earlier in the pipeline (including by custom before transforms) and may re-enable elision in cases the isElisionBlocked contract is meant to prevent. Consider restoring the isElisionBlocked check for the general case and only bypassing it for the specific scenario where the visited ImportClause becomes nil (or adjust isElisionBlocked to treat TypeEraser updates as non-blocking).

Copilot uses AI. Check for mistakes.
Copy link
Member

@jakebailey jakebailey Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description was not updated to reflect the later changes, unfortunately

Copy link
Member

@jakebailey jakebailey Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isElisionBlocked seems to only be needed for custom transformers, which we don't have, too.

case ast.KindImportClause:
Expand All @@ -77,7 +75,7 @@ func (tx *ImportElisionTransformer) visit(node *ast.Node) *ast.Node {
}
return tx.Factory().UpdateNamedImports(n, elements)
case ast.KindImportSpecifier:
if !tx.shouldEmitAliasDeclaration(node) {
if node.IsTypeOnly() || !tx.shouldEmitAliasDeclaration(node) {
// elide type-only or unused imports
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion testdata/baselines/reference/submodule/conformance/importSpecifiers1.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ type; // Error (cannot resolve name)
as; // Error (used in emitting position)
export {};
//// [d.js]
import "./mod.js"; // Error
export {};
//// [e.js]
import { type as type } from "./mod.js";
type;
Expand Down
11 changes: 0 additions & 11 deletions testdata/baselines/reference/submodule/conformance/importSpecifiers1.js.diff
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions ...module/conformance/nodeModulesImportAttributesModeDeclarationEmitErrors(module=node16).js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ export interface LocalInterface extends RequireInterface, ImportInterface {}
//// [index.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
// not exclusively type-only
require("pkg");


//// [index.d.ts]
Expand Down
11 changes: 0 additions & 11 deletions ...e/conformance/nodeModulesImportAttributesModeDeclarationEmitErrors(module=node16).js.diff
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions ...module/conformance/nodeModulesImportAttributesModeDeclarationEmitErrors(module=node18).js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ export interface LocalInterface extends RequireInterface, ImportInterface {}
//// [index.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
// not exclusively type-only
require("pkg");


//// [index.d.ts]
Expand Down
11 changes: 0 additions & 11 deletions ...e/conformance/nodeModulesImportAttributesModeDeclarationEmitErrors(module=node18).js.diff
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions ...module/conformance/nodeModulesImportAttributesModeDeclarationEmitErrors(module=node20).js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ export interface LocalInterface extends RequireInterface, ImportInterface {}
//// [index.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
// not exclusively type-only
require("pkg");


//// [index.d.ts]
Expand Down
11 changes: 0 additions & 11 deletions ...e/conformance/nodeModulesImportAttributesModeDeclarationEmitErrors(module=node20).js.diff
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions ...dule/conformance/nodeModulesImportAttributesModeDeclarationEmitErrors(module=nodenext).js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ export interface LocalInterface extends RequireInterface, ImportInterface {}
//// [index.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
// not exclusively type-only
require("pkg");


//// [index.d.ts]
Expand Down
11 changes: 0 additions & 11 deletions ...conformance/nodeModulesImportAttributesModeDeclarationEmitErrors(module=nodenext).js.diff
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions ...rence/submodule/conformance/nodeModulesImportModeDeclarationEmitErrors1(module=node16).js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ export interface LocalInterface extends RequireInterface, ImportInterface {}
//// [index.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
// not exclusively type-only
require("pkg");


//// [index.d.ts]
Expand Down
11 changes: 0 additions & 11 deletions .../submodule/conformance/nodeModulesImportModeDeclarationEmitErrors1(module=node16).js.diff
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions ...rence/submodule/conformance/nodeModulesImportModeDeclarationEmitErrors1(module=node18).js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ export interface LocalInterface extends RequireInterface, ImportInterface {}
//// [index.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
// not exclusively type-only
require("pkg");


//// [index.d.ts]
Expand Down
11 changes: 0 additions & 11 deletions .../submodule/conformance/nodeModulesImportModeDeclarationEmitErrors1(module=node18).js.diff
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions ...rence/submodule/conformance/nodeModulesImportModeDeclarationEmitErrors1(module=node20).js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ export interface LocalInterface extends RequireInterface, ImportInterface {}
//// [index.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
// not exclusively type-only
require("pkg");


//// [index.d.ts]
Expand Down
11 changes: 0 additions & 11 deletions .../submodule/conformance/nodeModulesImportModeDeclarationEmitErrors1(module=node20).js.diff
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions ...nce/submodule/conformance/nodeModulesImportModeDeclarationEmitErrors1(module=nodenext).js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ export interface LocalInterface extends RequireInterface, ImportInterface {}
//// [index.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
// not exclusively type-only
require("pkg");


//// [index.d.ts]
Expand Down
11 changes: 0 additions & 11 deletions ...ubmodule/conformance/nodeModulesImportModeDeclarationEmitErrors1(module=nodenext).js.diff
View file
Open in desktop

This file was deleted.

Loading