Skip to content

Commit

Permalink
fix(transformer/typescript): remove type-only import = when `only_r…
Browse files Browse the repository at this point in the history
…emove_type_imports` is true (#8275)

close: #8230
close: rolldown/rolldown#3287

Related PR in Babel: #8230

I have compared our output with TypeScript, and it is the same as `TypeScript`, Babel's implementation currently hasn't removed imports referenced by type-only `TSImportEqualsDeclaration`
  • Loading branch information
Dunqing committed Jan 11, 2025
1 parent d56020b commit 9a03bd2
Show file tree
Hide file tree
Showing 13 changed files with 367 additions and 340 deletions.
9 changes: 6 additions & 3 deletions crates/oxc_transformer/src/typescript/annotations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,12 @@ impl<'a> Traverse<'a> for TypeScriptAnnotations<'a, '_> {
true
}
}
Statement::TSExportAssignment(_) | Statement::TSNamespaceExportDeclaration(_) => {
false
}
// `import Binding = X.Y.Z`
// `Binding` can be referenced as a value or a type, but here we already know it only as a type
// See `TypeScriptModule::transform_ts_import_equals`
Statement::TSTypeAliasDeclaration(_)
| Statement::TSExportAssignment(_)
| Statement::TSNamespaceExportDeclaration(_) => false,
_ => return true,
};

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_transformer/src/typescript/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl<'a, 'ctx> TypeScript<'a, 'ctx> {
annotations: TypeScriptAnnotations::new(options, ctx),
r#enum: TypeScriptEnum::new(),
namespace: TypeScriptNamespace::new(options, ctx),
module: TypeScriptModule::new(ctx),
module: TypeScriptModule::new(options.only_remove_type_imports, ctx),
rewrite_extensions: TypeScriptRewriteExtensions::new(options),
}
}
Expand Down
37 changes: 32 additions & 5 deletions crates/oxc_transformer/src/typescript/module.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use oxc_ast::{ast::*, NONE};
use oxc_semantic::Reference;
use oxc_span::SPAN;
use oxc_syntax::reference::ReferenceFlags;
use oxc_traverse::{Traverse, TraverseCtx};
Expand All @@ -7,12 +8,14 @@ use super::diagnostics;
use crate::TransformCtx;

pub struct TypeScriptModule<'a, 'ctx> {
/// <https://babeljs.io/docs/babel-plugin-transform-typescript#onlyremovetypeimports>
only_remove_type_imports: bool,
ctx: &'ctx TransformCtx<'a>,
}

impl<'a, 'ctx> TypeScriptModule<'a, 'ctx> {
pub fn new(ctx: &'ctx TransformCtx<'a>) -> Self {
Self { ctx }
pub fn new(only_remove_type_imports: bool, ctx: &'ctx TransformCtx<'a>) -> Self {
Self { only_remove_type_imports, ctx }
}
}

Expand All @@ -37,7 +40,9 @@ impl<'a> Traverse<'a> for TypeScriptModule<'a, '_> {
fn enter_declaration(&mut self, decl: &mut Declaration<'a>, ctx: &mut TraverseCtx<'a>) {
if let Declaration::TSImportEqualsDeclaration(import_equals) = decl {
if import_equals.import_kind.is_value() {
*decl = self.transform_ts_import_equals(import_equals, ctx);
if let Some(new_decl) = self.transform_ts_import_equals(import_equals, ctx) {
*decl = new_decl;
}
}
}
}
Expand Down Expand Up @@ -88,7 +93,29 @@ impl<'a> TypeScriptModule<'a, '_> {
&self,
decl: &mut TSImportEqualsDeclaration<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Declaration<'a> {
) -> Option<Declaration<'a>> {
if !self.only_remove_type_imports
&& !ctx.parent().is_export_named_declaration()
&& ctx.symbols().get_resolved_references(decl.id.symbol_id()).all(Reference::is_type)
{
// No value reference, we will remove this declaration in `TypeScriptAnnotations`
match &mut decl.module_reference {
module_reference @ match_ts_type_name!(TSModuleReference) => {
let ident = module_reference.to_ts_type_name().get_identifier_reference();
let reference = ctx.symbols_mut().get_reference_mut(ident.reference_id());
// The binding of TSImportEqualsDeclaration has treated as a type reference,
// so an identifier reference that it referenced also should be treated as a type reference.
// `import TypeBinding = X.Y.Z`
// ^ `X` should be treated as a type reference.
let flags = reference.flags_mut();
debug_assert_eq!(*flags, ReferenceFlags::Read);
*flags = ReferenceFlags::Type;
}
TSModuleReference::ExternalModuleReference(_) => {}
}
return None;
}

let binding_pattern_kind =
ctx.ast.binding_pattern_kind_binding_identifier(SPAN, &decl.id.name);
let binding = ctx.ast.binding_pattern(binding_pattern_kind, NONE, false);
Expand Down Expand Up @@ -123,7 +150,7 @@ impl<'a> TypeScriptModule<'a, '_> {
let decls =
ctx.ast.vec1(ctx.ast.variable_declarator(SPAN, kind, binding, Some(init), false));

ctx.ast.declaration_variable(SPAN, kind, decls, false)
Some(ctx.ast.declaration_variable(SPAN, kind, decls, false))
}

#[allow(clippy::only_used_in_recursion)]
Expand Down
8 changes: 7 additions & 1 deletion napi/transform/test/transform.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,19 @@ describe('modules', () => {
const code = `
export = function foo (): void {}
import bar = require('bar')
console.log(bar)
`;
const ret = transform('test.ts', code, {
typescript: {
declaration: {},
},
});
expect(ret.code).toEqual('module.exports = function foo() {};\nconst bar = require("bar");\n');
expect(ret.code).toMatchInlineSnapshot(`
"module.exports = function foo() {};
const bar = require("bar");
console.log(bar);
"
`);
expect(ret.declaration).toEqual('declare const _default: () => void;\nexport = _default;\n');
});
});
Expand Down
59 changes: 32 additions & 27 deletions tasks/coverage/snapshots/semantic_babel.snap
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,9 @@ after transform: ["Y", "foo"]
rebuilt : []

tasks/coverage/babel/packages/babel-parser/test/fixtures/estree/typescript/import-require/input.js
semantic error: Missing SymbolId: "x"
Binding symbols mismatch:
after transform: ScopeId(0): [SymbolId(0)]
rebuilt : ScopeId(0): [SymbolId(0)]
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["x"]
rebuilt : ScopeId(0): []

tasks/coverage/babel/packages/babel-parser/test/fixtures/estree/typescript/literals/input.js
semantic error: Scope children mismatch:
Expand Down Expand Up @@ -723,28 +722,30 @@ after transform: ScopeId(0): [ScopeId(1), ScopeId(2)]
rebuilt : ScopeId(0): []

tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/import/equals/input.ts
semantic error: Missing SymbolId: "A"
Binding symbols mismatch:
after transform: ScopeId(0): [SymbolId(0)]
rebuilt : ScopeId(0): [SymbolId(0)]
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["A"]
rebuilt : ScopeId(0): []
Unresolved references mismatch:
after transform: ["B"]
rebuilt : []

tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/import/equals-in-unambiguous/input.ts
semantic error: Missing SymbolId: "A"
Binding symbols mismatch:
after transform: ScopeId(0): [SymbolId(0)]
rebuilt : ScopeId(0): [SymbolId(0)]
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["A"]
rebuilt : ScopeId(0): []
Unresolved references mismatch:
after transform: ["B"]
rebuilt : []

tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/import/equals-require/input.ts
semantic error: Missing SymbolId: "a"
Binding symbols mismatch:
after transform: ScopeId(0): [SymbolId(0)]
rebuilt : ScopeId(0): [SymbolId(0)]
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["a"]
rebuilt : ScopeId(0): []

tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/import/equals-require-in-unambiguous/input.ts
semantic error: Missing SymbolId: "a"
Binding symbols mismatch:
after transform: ScopeId(0): [SymbolId(0)]
rebuilt : ScopeId(0): [SymbolId(0)]
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["a"]
rebuilt : ScopeId(0): []

tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/import/export-import/input.ts
semantic error: Missing SymbolId: "A"
Expand Down Expand Up @@ -799,10 +800,9 @@ after transform: ScopeId(0): ["a"]
rebuilt : ScopeId(0): []

tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/import/import-type-as-identifier/input.ts
semantic error: Missing SymbolId: "type"
Binding symbols mismatch:
after transform: ScopeId(0): [SymbolId(0)]
rebuilt : ScopeId(0): [SymbolId(0)]
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["type"]
rebuilt : ScopeId(0): []

tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/import/internal-comments/input.ts
semantic error: Bindings mismatch:
Expand Down Expand Up @@ -1431,8 +1431,7 @@ after transform: ScopeId(0): [ScopeId(1)]
rebuilt : ScopeId(0): []

tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/scope/redeclaration-import-equals-var/input.ts
semantic error: Missing SymbolId: "a"
Bindings mismatch:
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["M", "a"]
rebuilt : ScopeId(0): ["a"]
Scope children mismatch:
Expand All @@ -1443,7 +1442,13 @@ after transform: SymbolId(1): SymbolFlags(FunctionScopedVariable | Import)
rebuilt : SymbolId(0): SymbolFlags(FunctionScopedVariable)
Symbol span mismatch for "a":
after transform: SymbolId(1): Span { start: 20, end: 21 }
rebuilt : SymbolId(0): Span { start: 0, end: 0 }
rebuilt : SymbolId(0): Span { start: 31, end: 32 }
Symbol redeclarations mismatch for "a":
after transform: SymbolId(1): [Span { start: 31, end: 32 }]
rebuilt : SymbolId(0): []
Unresolved references mismatch:
after transform: ["M"]
rebuilt : []

tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/scope/redeclaration-import-let/input.ts
semantic error: Symbol flags mismatch for "Context":
Expand Down
Loading

0 comments on commit 9a03bd2

Please sign in to comment.