Skip to content

Commit b6bdcd7

Browse files
Brooooooklynclaude
andcommitted
fix: import elision for inline type specifiers, declare property decorators, and computed keys
Three import specifier mismatches fixed: 1. Inline `type` specifiers (`import { type FormGroup }`) now tracked and elided 2. `declare` property decorator args (`@ViewChild(X) declare y`) now elided 3. Computed property keys in type annotations (`[fromEmail]: T`) now preserved Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 9e2d565 commit b6bdcd7

File tree

1 file changed

+273
-16
lines changed

1 file changed

+273
-16
lines changed

crates/oxc_angular_compiler/src/component/import_elision.rs

Lines changed: 273 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ use std::path::Path;
3838

3939
use oxc_ast::ast::{
4040
BindingIdentifier, ClassElement, Expression, ImportDeclarationSpecifier, MethodDefinitionKind,
41-
Program, Statement,
41+
Program, Statement, TSType,
4242
};
4343
use oxc_semantic::{Semantic, SemanticBuilder, SymbolFlags};
4444
use oxc_span::Atom;
@@ -90,8 +90,9 @@ impl<'a> ImportElisionAnalyzer<'a> {
9090
for specifier in specifiers {
9191
match specifier {
9292
ImportDeclarationSpecifier::ImportSpecifier(spec) => {
93-
// Skip explicit type-only specifiers (import { type X })
93+
// Explicit type-only specifiers (import { type X }) are always elided
9494
if spec.import_kind.is_type() {
95+
type_only_specifiers.insert(spec.local.name.clone());
9596
continue;
9697
}
9798

@@ -120,19 +121,146 @@ impl<'a> ImportElisionAnalyzer<'a> {
120121
}
121122
}
122123

124+
// Post-pass: remove identifiers used as computed property keys in type annotations.
125+
// These are value references (they compute runtime property names) even though they
126+
// appear inside type contexts. TypeScript preserves these imports.
127+
// Example: `[fromEmail]: Emailer[]` in a type literal uses `fromEmail` as a value.
128+
let computed_key_idents = Self::collect_computed_property_key_idents(program);
129+
for name in &computed_key_idents {
130+
type_only_specifiers.remove(name);
131+
}
132+
123133
Self { type_only_specifiers }
124134
}
125135

126-
/// Collect import names that are used ONLY in constructor parameter decorators.
136+
/// Collect identifiers used as computed property keys in type annotations.
127137
///
128-
/// Constructor parameter decorators like `@Inject`, `@Optional`, `@Self`, `@SkipSelf`,
129-
/// `@Host`, and `@Attribute` are removed by Angular's compiler and converted to
130-
/// factory metadata. The imports for these decorators and their arguments (like
131-
/// `@Inject(TOKEN)`) should be elided.
138+
/// Computed property keys like `[fromEmail]` in type literals reference runtime values,
139+
/// even when they appear in type contexts. TypeScript considers these as value references
140+
/// and preserves their imports.
141+
fn collect_computed_property_key_idents(program: &'a Program<'a>) -> FxHashSet<Atom<'a>> {
142+
let mut result = FxHashSet::default();
143+
144+
for stmt in &program.body {
145+
Self::collect_computed_keys_from_statement(stmt, &mut result);
146+
}
147+
148+
result
149+
}
150+
151+
/// Walk a statement collecting computed property key identifiers from type annotations.
152+
fn collect_computed_keys_from_statement(
153+
stmt: &'a Statement<'a>,
154+
result: &mut FxHashSet<Atom<'a>>,
155+
) {
156+
match stmt {
157+
Statement::ClassDeclaration(class) => {
158+
Self::collect_computed_keys_from_class(class, result);
159+
}
160+
Statement::ExportDefaultDeclaration(export) => {
161+
if let oxc_ast::ast::ExportDefaultDeclarationKind::ClassDeclaration(class) =
162+
&export.declaration
163+
{
164+
Self::collect_computed_keys_from_class(class, result);
165+
}
166+
}
167+
Statement::ExportNamedDeclaration(export) => {
168+
if let Some(oxc_ast::ast::Declaration::ClassDeclaration(class)) =
169+
&export.declaration
170+
{
171+
Self::collect_computed_keys_from_class(class, result);
172+
}
173+
}
174+
_ => {}
175+
}
176+
}
177+
178+
/// Walk class members collecting computed property key identifiers from type annotations.
179+
fn collect_computed_keys_from_class(
180+
class: &'a oxc_ast::ast::Class<'a>,
181+
result: &mut FxHashSet<Atom<'a>>,
182+
) {
183+
for element in &class.body.body {
184+
if let ClassElement::PropertyDefinition(prop) = element {
185+
// Check the type annotation for computed property keys in type literals
186+
if let Some(ts_type) = &prop.type_annotation {
187+
Self::collect_computed_keys_from_ts_type(&ts_type.type_annotation, result);
188+
}
189+
}
190+
}
191+
}
192+
193+
/// Recursively walk a TypeScript type collecting computed property key identifiers.
194+
fn collect_computed_keys_from_ts_type(
195+
ts_type: &'a TSType<'a>,
196+
result: &mut FxHashSet<Atom<'a>>,
197+
) {
198+
match ts_type {
199+
TSType::TSTypeLiteral(type_lit) => {
200+
for member in &type_lit.members {
201+
if let oxc_ast::ast::TSSignature::TSPropertySignature(prop_sig) = member {
202+
// Check if the property key is computed
203+
if prop_sig.computed {
204+
Self::collect_idents_from_expr(&prop_sig.key, result);
205+
}
206+
}
207+
}
208+
}
209+
TSType::TSUnionType(union_type) => {
210+
for ty in &union_type.types {
211+
Self::collect_computed_keys_from_ts_type(ty, result);
212+
}
213+
}
214+
TSType::TSIntersectionType(intersection_type) => {
215+
for ty in &intersection_type.types {
216+
Self::collect_computed_keys_from_ts_type(ty, result);
217+
}
218+
}
219+
_ => {}
220+
}
221+
}
222+
223+
/// Collect identifier names from an expression (for computed property keys).
224+
fn collect_idents_from_expr(
225+
expr: &'a oxc_ast::ast::PropertyKey<'a>,
226+
result: &mut FxHashSet<Atom<'a>>,
227+
) {
228+
match expr {
229+
oxc_ast::ast::PropertyKey::StaticIdentifier(_) => {
230+
// Static identifiers are NOT computed property keys
231+
}
232+
oxc_ast::ast::PropertyKey::PrivateIdentifier(_) => {}
233+
_ => {
234+
if let Some(expr) = expr.as_expression() {
235+
Self::collect_idents_from_expression(expr, result);
236+
}
237+
}
238+
}
239+
}
240+
241+
/// Collect identifier names from an expression.
242+
fn collect_idents_from_expression(expr: &'a Expression<'a>, result: &mut FxHashSet<Atom<'a>>) {
243+
match expr {
244+
Expression::Identifier(id) => {
245+
result.insert(id.name.clone());
246+
}
247+
Expression::StaticMemberExpression(member) => {
248+
// For `RecipientType.To`, collect `RecipientType`
249+
if let Expression::Identifier(id) = &member.object {
250+
result.insert(id.name.clone());
251+
}
252+
}
253+
_ => {}
254+
}
255+
}
256+
257+
/// Collect import names that are used ONLY in compiler-handled positions.
132258
///
133-
/// This function returns a set of local import names that should be elided because:
134-
/// 1. The import is a known parameter decorator (Inject, Optional, etc.)
135-
/// 2. The import is ONLY used as an argument to @Inject()
259+
/// This includes:
260+
/// 1. Constructor parameter decorators (`@Inject`, `@Optional`, etc.) - removed by Angular
261+
/// 2. `@Inject(TOKEN)` arguments - only used in ctor param decorators
262+
/// 3. `declare` property decorator arguments - `declare` properties are not emitted by
263+
/// TypeScript, so their decorator arguments have no runtime value references
136264
///
137265
/// Reference: packages/compiler-cli/src/ngtsc/transform/jit/src/downlevel_decorators_transform.ts
138266
fn collect_ctor_param_decorator_only_imports(program: &'a Program<'a>) -> FxHashSet<&'a str> {
@@ -141,8 +269,10 @@ impl<'a> ImportElisionAnalyzer<'a> {
141269
// Track:
142270
// 1. Symbols used ONLY in ctor param decorator position (the decorator itself)
143271
// 2. Symbols used ONLY as @Inject() arguments
272+
// 3. Symbols used ONLY in `declare` property decorator arguments
144273
let mut ctor_param_decorator_uses: FxHashSet<&'a str> = FxHashSet::default();
145274
let mut inject_arg_uses: FxHashSet<&'a str> = FxHashSet::default();
275+
let mut declare_prop_decorator_uses: FxHashSet<&'a str> = FxHashSet::default();
146276
let mut other_value_uses: FxHashSet<&'a str> = FxHashSet::default();
147277

148278
// Walk the AST to find constructor parameters and their decorators
@@ -151,13 +281,13 @@ impl<'a> ImportElisionAnalyzer<'a> {
151281
stmt,
152282
&mut ctor_param_decorator_uses,
153283
&mut inject_arg_uses,
284+
&mut declare_prop_decorator_uses,
154285
&mut other_value_uses,
155286
);
156287
}
157288

158-
// A symbol can be elided if:
159-
// 1. It's used in ctor param decorators AND NOT used elsewhere, OR
160-
// 2. It's used in @Inject() args AND NOT used elsewhere
289+
// A symbol can be elided if it appears ONLY in compiler-handled positions
290+
// and NOT in other value positions
161291
for name in ctor_param_decorator_uses {
162292
if !other_value_uses.contains(name) {
163293
result.insert(name);
@@ -168,6 +298,11 @@ impl<'a> ImportElisionAnalyzer<'a> {
168298
result.insert(name);
169299
}
170300
}
301+
for name in declare_prop_decorator_uses {
302+
if !other_value_uses.contains(name) {
303+
result.insert(name);
304+
}
305+
}
171306

172307
result
173308
}
@@ -177,6 +312,7 @@ impl<'a> ImportElisionAnalyzer<'a> {
177312
stmt: &'a Statement<'a>,
178313
ctor_param_decorator_uses: &mut FxHashSet<&'a str>,
179314
inject_arg_uses: &mut FxHashSet<&'a str>,
315+
declare_prop_decorator_uses: &mut FxHashSet<&'a str>,
180316
other_value_uses: &mut FxHashSet<&'a str>,
181317
) {
182318
match stmt {
@@ -185,6 +321,7 @@ impl<'a> ImportElisionAnalyzer<'a> {
185321
class,
186322
ctor_param_decorator_uses,
187323
inject_arg_uses,
324+
declare_prop_decorator_uses,
188325
other_value_uses,
189326
);
190327
}
@@ -196,6 +333,7 @@ impl<'a> ImportElisionAnalyzer<'a> {
196333
class,
197334
ctor_param_decorator_uses,
198335
inject_arg_uses,
336+
declare_prop_decorator_uses,
199337
other_value_uses,
200338
);
201339
}
@@ -208,6 +346,7 @@ impl<'a> ImportElisionAnalyzer<'a> {
208346
class,
209347
ctor_param_decorator_uses,
210348
inject_arg_uses,
349+
declare_prop_decorator_uses,
211350
other_value_uses,
212351
);
213352
}
@@ -233,6 +372,7 @@ impl<'a> ImportElisionAnalyzer<'a> {
233372
class: &'a oxc_ast::ast::Class<'a>,
234373
ctor_param_decorator_uses: &mut FxHashSet<&'a str>,
235374
inject_arg_uses: &mut FxHashSet<&'a str>,
375+
declare_prop_decorator_uses: &mut FxHashSet<&'a str>,
236376
other_value_uses: &mut FxHashSet<&'a str>,
237377
) {
238378
// Process class decorators - these are NOT elided (they run at runtime)
@@ -259,9 +399,24 @@ impl<'a> ImportElisionAnalyzer<'a> {
259399
}
260400
}
261401
ClassElement::PropertyDefinition(prop) => {
262-
// Process property decorators (e.g., @Input, @ViewChild) - NOT elided
263-
for decorator in &prop.decorators {
264-
Self::collect_value_uses_from_expr(&decorator.expression, other_value_uses);
402+
if prop.declare {
403+
// `declare` properties are not emitted by TypeScript, so their
404+
// decorators and arguments have no runtime value references.
405+
// Track them separately for elision.
406+
for decorator in &prop.decorators {
407+
Self::collect_value_uses_from_expr(
408+
&decorator.expression,
409+
declare_prop_decorator_uses,
410+
);
411+
}
412+
} else {
413+
// Process property decorators (e.g., @Input, @ViewChild) - NOT elided
414+
for decorator in &prop.decorators {
415+
Self::collect_value_uses_from_expr(
416+
&decorator.expression,
417+
other_value_uses,
418+
);
419+
}
265420
}
266421
// Process property initializers (e.g., doc = DOCUMENT)
267422
if let Some(init) = &prop.value {
@@ -1432,4 +1587,106 @@ export class BitLabelComponent {
14321587
// Should NOT contain ElementRef (type annotation)
14331588
assert!(!import_line.contains("ElementRef"), "ElementRef should be removed from imports");
14341589
}
1590+
1591+
// =========================================================================
1592+
// Regression tests for specific import specifier mismatches (1d category)
1593+
// =========================================================================
1594+
1595+
#[test]
1596+
fn test_inline_type_specifier_elided() {
1597+
// Case 1: `import { type FormGroup, FormControl } from '@angular/forms'`
1598+
// The `type` keyword on a specifier means it should be elided.
1599+
// Angular strips type-only specifiers regardless of other specifiers in the same import.
1600+
let source = r#"
1601+
import { type FormGroup, FormControl, FormsModule, ReactiveFormsModule, UntypedFormGroup, Validators } from '@angular/forms';
1602+
import { Component } from '@angular/core';
1603+
1604+
@Component({ selector: 'test' })
1605+
class MyComponent {
1606+
form = new FormControl('');
1607+
addEditFieldForm = new UntypedFormGroup({});
1608+
}
1609+
1610+
type AddEditFieldForm = FormGroup<{ name: FormControl<string> }>;
1611+
"#;
1612+
let type_only = analyze_source(source);
1613+
1614+
// FormGroup has `type` keyword — must be elided
1615+
assert!(
1616+
type_only.contains("FormGroup"),
1617+
"type FormGroup (inline type specifier) should be elided"
1618+
);
1619+
1620+
// FormControl, UntypedFormGroup are used as values — must NOT be elided
1621+
assert!(
1622+
!type_only.contains("FormControl"),
1623+
"FormControl should be preserved (value usage)"
1624+
);
1625+
assert!(
1626+
!type_only.contains("UntypedFormGroup"),
1627+
"UntypedFormGroup should be preserved (value usage)"
1628+
);
1629+
}
1630+
1631+
#[test]
1632+
fn test_declare_property_viewchild_arg_elided() {
1633+
// Case 2: `@ViewChild(GridstackComponent) declare gridstack: GridstackComponent;`
1634+
// When a property is `declare`, TypeScript does not emit it, so the decorator
1635+
// and its arguments have no runtime value references. Angular strips it.
1636+
let source = r#"
1637+
import { Component, ViewChild } from '@angular/core';
1638+
import { GridstackComponent, GridstackModule } from 'gridstack/dist/angular';
1639+
1640+
@Component({ selector: 'test', imports: [GridstackModule] })
1641+
class MyComponent {
1642+
@ViewChild(GridstackComponent, { static: true })
1643+
declare gridstack: GridstackComponent;
1644+
}
1645+
"#;
1646+
let type_only = analyze_source(source);
1647+
1648+
// GridstackComponent is only used on a `declare` property — must be elided
1649+
assert!(
1650+
type_only.contains("GridstackComponent"),
1651+
"GridstackComponent on declare property should be elided"
1652+
);
1653+
1654+
// GridstackModule is used in Component imports array — must NOT be elided
1655+
assert!(
1656+
!type_only.contains("GridstackModule"),
1657+
"GridstackModule should be preserved (value usage in imports array)"
1658+
);
1659+
}
1660+
1661+
#[test]
1662+
fn test_computed_property_key_in_type_annotation_preserved() {
1663+
// Case 3: `[fromEmail]: Emailer[]` as computed property key in a type annotation.
1664+
// Even though `[fromEmail]` appears in a type context, the computed property key
1665+
// references the runtime value of `fromEmail`. Angular preserves it.
1666+
let source = r#"
1667+
import { Component, Input } from '@angular/core';
1668+
import { fromEmail, RecipientType } from './email.interface';
1669+
1670+
@Component({ selector: 'test' })
1671+
class MyComponent {
1672+
@Input() collapseEmailInfo: {
1673+
[fromEmail]: string[];
1674+
[RecipientType.To]: string[];
1675+
};
1676+
}
1677+
"#;
1678+
let type_only = analyze_source(source);
1679+
1680+
// fromEmail should NOT be elided — it's a computed property key that references a value
1681+
assert!(
1682+
!type_only.contains("fromEmail"),
1683+
"fromEmail used as computed property key in type annotation should be preserved"
1684+
);
1685+
1686+
// RecipientType is used via member access — should NOT be elided
1687+
assert!(
1688+
!type_only.contains("RecipientType"),
1689+
"RecipientType used as computed property key in type annotation should be preserved"
1690+
);
1691+
}
14351692
}

0 commit comments

Comments
 (0)