From bdd11fa62bd70d6d6dd3932c7ef7bfefed1901c6 Mon Sep 17 00:00:00 2001 From: underfin Date: Fri, 27 Dec 2024 16:52:10 +0800 Subject: [PATCH 1/4] fix(transformer-typescript): should strip import specifiers type with only_remove_type_imports --- .../src/typescript/annotations.rs | 22 ++++++++++++++++--- .../snapshots/babel.snap.md | 8 +++++-- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/crates/oxc_transformer/src/typescript/annotations.rs b/crates/oxc_transformer/src/typescript/annotations.rs index d321921602d68..47067a71b6d59 100644 --- a/crates/oxc_transformer/src/typescript/annotations.rs +++ b/crates/oxc_transformer/src/typescript/annotations.rs @@ -1,6 +1,6 @@ use rustc_hash::FxHashSet; -use oxc_allocator::Vec as ArenaVec; +use oxc_allocator::{CloneIn, Vec as ArenaVec}; use oxc_ast::ast::*; use oxc_diagnostics::OxcDiagnostic; use oxc_semantic::SymbolFlags; @@ -95,31 +95,47 @@ impl<'a, 'ctx> Traverse<'a> for TypeScriptAnnotations<'a, 'ctx> { Statement::ImportDeclaration(decl) => { if decl.import_kind.is_type() { false - } else if self.only_remove_type_imports { - true } else if let Some(specifiers) = &mut decl.specifiers { if specifiers.is_empty() { // import {} from 'mod' -> import 'mod' decl.specifiers = None; true } else { + let mut all_specifiers_is_type = true; specifiers.retain(|specifier| { let id = match specifier { ImportDeclarationSpecifier::ImportSpecifier(s) => { if s.import_kind.is_type() { return false; } + all_specifiers_is_type = false; &s.local } ImportDeclarationSpecifier::ImportDefaultSpecifier(s) => { + all_specifiers_is_type = false; &s.local } ImportDeclarationSpecifier::ImportNamespaceSpecifier(s) => { + all_specifiers_is_type = false; &s.local } }; + if self.only_remove_type_imports { + return true; + } self.has_value_reference(&id.name, ctx) }); + if all_specifiers_is_type && self.only_remove_type_imports { + *decl = ctx.ast.alloc_import_declaration( + decl.span, + None, + decl.source.clone(), + None, + decl.with_clause.clone_in(ctx.ast.allocator), + decl.import_kind, + ); + return true; + } !specifiers.is_empty() } } else { diff --git a/tasks/transform_conformance/snapshots/babel.snap.md b/tasks/transform_conformance/snapshots/babel.snap.md index 35e84c82d74b6..ada27f3245ccb 100644 --- a/tasks/transform_conformance/snapshots/babel.snap.md +++ b/tasks/transform_conformance/snapshots/babel.snap.md @@ -1691,7 +1691,9 @@ after transform: [] rebuilt : ["require"] * imports/only-remove-type-imports/input.ts -x Output mismatch +Bindings mismatch: +after transform: ScopeId(0): ["H", "I", "I2", "J", "K1", "K2", "L1", "L2", "L3", "a", "b", "c2", "d", "d2", "e", "e4"] +rebuilt : ScopeId(0): ["L2", "a", "b", "c2", "d", "d2", "e", "e4"] * imports/property-signature/input.ts Bindings mismatch: @@ -1722,7 +1724,9 @@ after transform: ScopeId(0): ["Foo1", "Foo2"] rebuilt : ScopeId(0): [] * imports/type-only-import-specifier-4/input.ts -x Output mismatch +Bindings mismatch: +after transform: ScopeId(0): ["A"] +rebuilt : ScopeId(0): [] * lvalues/TSTypeParameterInstantiation/input.ts Symbol reference IDs mismatch for "AbstractClass": From 92508b47182b352b408764f6fd703ad0475c8bbb Mon Sep 17 00:00:00 2001 From: underfin Date: Fri, 27 Dec 2024 16:57:06 +0800 Subject: [PATCH 2/4] chore: add comment --- crates/oxc_transformer/src/typescript/annotations.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/oxc_transformer/src/typescript/annotations.rs b/crates/oxc_transformer/src/typescript/annotations.rs index 47067a71b6d59..7b35111bdd7f3 100644 --- a/crates/oxc_transformer/src/typescript/annotations.rs +++ b/crates/oxc_transformer/src/typescript/annotations.rs @@ -120,11 +120,13 @@ impl<'a, 'ctx> Traverse<'a> for TypeScriptAnnotations<'a, 'ctx> { &s.local } }; + // Should preserve `import x from 'x'`(x is unused) if only_remove_type_imports enable if self.only_remove_type_imports { return true; } self.has_value_reference(&id.name, ctx) }); + // Should transform `import { type x } from 'x'`` to `import 'xx'` if only_remove_type_imports enable if all_specifiers_is_type && self.only_remove_type_imports { *decl = ctx.ast.alloc_import_declaration( decl.span, From da88ca352c6f5304c0802d324db54156d3a7e7d2 Mon Sep 17 00:00:00 2001 From: Dunqing Date: Mon, 6 Jan 2025 11:34:11 +0800 Subject: [PATCH 3/4] support only_remove_type_imports --- .../src/typescript/annotations.rs | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/oxc_transformer/src/typescript/annotations.rs b/crates/oxc_transformer/src/typescript/annotations.rs index 7b35111bdd7f3..f5c303741f22a 100644 --- a/crates/oxc_transformer/src/typescript/annotations.rs +++ b/crates/oxc_transformer/src/typescript/annotations.rs @@ -1,6 +1,6 @@ use rustc_hash::FxHashSet; -use oxc_allocator::{CloneIn, Vec as ArenaVec}; +use oxc_allocator::Vec as ArenaVec; use oxc_ast::ast::*; use oxc_diagnostics::OxcDiagnostic; use oxc_semantic::SymbolFlags; @@ -101,44 +101,43 @@ impl<'a, 'ctx> Traverse<'a> for TypeScriptAnnotations<'a, 'ctx> { decl.specifiers = None; true } else { - let mut all_specifiers_is_type = true; specifiers.retain(|specifier| { let id = match specifier { ImportDeclarationSpecifier::ImportSpecifier(s) => { if s.import_kind.is_type() { return false; } - all_specifiers_is_type = false; &s.local } ImportDeclarationSpecifier::ImportDefaultSpecifier(s) => { - all_specifiers_is_type = false; &s.local } ImportDeclarationSpecifier::ImportNamespaceSpecifier(s) => { - all_specifiers_is_type = false; &s.local } }; - // Should preserve `import x from 'x'`(x is unused) if only_remove_type_imports enable + // If `only_remove_type_imports` is true, then we can return `true` to keep it because + // it is not a type import, otherwise we need to check if the identifier is referenced if self.only_remove_type_imports { - return true; + true + } else { + self.has_value_reference(&id.name, ctx) } - self.has_value_reference(&id.name, ctx) }); - // Should transform `import { type x } from 'x'`` to `import 'xx'` if only_remove_type_imports enable - if all_specifiers_is_type && self.only_remove_type_imports { - *decl = ctx.ast.alloc_import_declaration( - decl.span, - None, - decl.source.clone(), - None, - decl.with_clause.clone_in(ctx.ast.allocator), - decl.import_kind, - ); + + if !specifiers.is_empty() { return true; } - !specifiers.is_empty() + + // `import { type A } from 'mod'` + if self.only_remove_type_imports { + // -> `import 'mod'` + decl.specifiers = None; + true + } else { + // Remove the import declaration if all specifiers are removed + false + } } } else { true From 447ca8e6d2cf18dff56acdb3a6f6476f12f7baa9 Mon Sep 17 00:00:00 2001 From: Dunqing Date: Mon, 6 Jan 2025 12:27:21 +0800 Subject: [PATCH 4/4] Don't use return --- .../src/typescript/annotations.rs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/oxc_transformer/src/typescript/annotations.rs b/crates/oxc_transformer/src/typescript/annotations.rs index f5c303741f22a..77e01b975e773 100644 --- a/crates/oxc_transformer/src/typescript/annotations.rs +++ b/crates/oxc_transformer/src/typescript/annotations.rs @@ -125,18 +125,18 @@ impl<'a, 'ctx> Traverse<'a> for TypeScriptAnnotations<'a, 'ctx> { } }); - if !specifiers.is_empty() { - return true; - } - - // `import { type A } from 'mod'` - if self.only_remove_type_imports { - // -> `import 'mod'` - decl.specifiers = None; - true + if specifiers.is_empty() { + // `import { type A } from 'mod'` + if self.only_remove_type_imports { + // -> `import 'mod'` + decl.specifiers = None; + true + } else { + // Remove the import declaration if all specifiers are removed + false + } } else { - // Remove the import declaration if all specifiers are removed - false + true } } } else {