Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 54 additions & 0 deletions crates/oxc_angular_compiler/src/component/cross_file_elision.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,9 @@ impl CrossFileAnalyzer {
exports.get(export_name).cloned()
};

// If not found directly, check star exports (export * from './other')
let export_info = export_info.or_else(|| self.find_in_star_exports(file_path, export_name));

let Some(export_info) = export_info else {
return false;
};
Expand Down Expand Up @@ -920,4 +923,55 @@ export { doSomething } from './utils';
let resolved = analyzer.resolve_import_source_path("./index", "RenamedExport", &main_file);
assert_eq!(resolved, Some("./original".to_string()));
}

/// Regression test for quick-access.component.ts:
/// `import { WIDGET_CONTROL, WidgetControlService } from '../../widget-control'`
/// where `widget-control/index.ts` has `export * from './widget-control.service'`
/// and `widget-control.service.ts` has `export interface WidgetControlService { ... }`
///
/// The interface should be detected as type-only through the barrel star export chain.
#[test]
fn test_interface_through_star_export_barrel_is_type_only() {
let dir = TempDir::new().unwrap();
// widget-control/widget-control.service.ts — interface (type-only)
create_test_file(
dir.path(),
"widget-control/widget-control.service.ts",
"export interface WidgetControlService { hideWidget(widgetId: string): void; }",
);
// widget-control/widget-control.token.ts — const (runtime value)
create_test_file(
dir.path(),
"widget-control/widget-control.token.ts",
"import { InjectionToken } from '@angular/core';\nexport const WIDGET_CONTROL = new InjectionToken('WidgetControlService');",
);
// widget-control/index.ts — barrel re-export via star
create_test_file(
dir.path(),
"widget-control/index.ts",
"export * from './widget-control.service';\nexport * from './widget-control.token';",
);

let main_file = dir.path().join("quick-access/quick-access/main.ts");
// Create parent dir so the path is valid
std::fs::create_dir_all(main_file.parent().unwrap()).unwrap();

let mut analyzer = CrossFileAnalyzer::new(dir.path(), None);

// WidgetControlService is an interface — should be type-only
assert!(
analyzer.is_type_only_import(
"../../widget-control",
"WidgetControlService",
&main_file
),
"WidgetControlService interface should be detected as type-only through star export barrel"
);

// WIDGET_CONTROL is a const — should NOT be type-only
assert!(
!analyzer.is_type_only_import("../../widget-control", "WIDGET_CONTROL", &main_file),
"WIDGET_CONTROL const should NOT be type-only"
);
}
}
150 changes: 141 additions & 9 deletions crates/oxc_angular_compiler/src/component/import_elision.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,13 @@ impl<'a> ImportElisionAnalyzer<'a> {
type_only_specifiers.insert(name.clone());
}
}
ImportDeclarationSpecifier::ImportNamespaceSpecifier(_) => {
// Namespace imports are generally kept (hard to determine type-only usage)
ImportDeclarationSpecifier::ImportNamespaceSpecifier(spec) => {
// Check if the namespace import is type-only using semantic analysis.
// e.g., `import * as moment from 'moment'` where `moment` is only
// used in type annotations like `moment.Moment`.
if Self::is_type_only_import(&spec.local, semantic) {
type_only_specifiers.insert(spec.local.name.clone());
}
}
}
}
Expand Down Expand Up @@ -587,9 +592,25 @@ impl<'a> ImportElisionAnalyzer<'a> {
}
}
}
Expression::ArrowFunctionExpression(_) => {
// For arrow functions, we don't need to deeply analyze the body
// since we're only checking decorator positions
Expression::ArrowFunctionExpression(arrow) => {
// Arrow function bodies may contain value references, e.g.,
// `forwardRef(() => TagPickerComponent)` in Component imports.
for stmt in &arrow.body.statements {
match stmt {
Statement::ExpressionStatement(expr_stmt) => {
Self::collect_value_uses_from_expr(
&expr_stmt.expression,
other_value_uses,
);
}
Statement::ReturnStatement(ret) => {
if let Some(arg) = &ret.argument {
Self::collect_value_uses_from_expr(arg, other_value_uses);
}
}
_ => {}
}
}
}
_ => {}
}
Expand Down Expand Up @@ -701,13 +722,24 @@ impl<'a> ImportElisionAnalyzer<'a> {
/// Filter import declarations to remove type-only specifiers.
///
/// Returns a new source string with type-only import specifiers removed.
/// Entire import declarations are removed if all their specifiers are type-only.
/// Entire import declarations are removed if all their specifiers are type-only,
/// or if the import has no specifiers at all (`import {} from 'module'`).
pub fn filter_imports<'a>(
source: &str,
program: &Program<'a>,
analyzer: &ImportElisionAnalyzer<'a>,
) -> String {
if !analyzer.has_type_only_imports() {
// Check if there are empty imports that need removal (import {} from '...')
let has_empty_imports = program.body.iter().any(|stmt| {
if let Statement::ImportDeclaration(import_decl) = stmt {
if let Some(specifiers) = &import_decl.specifiers {
return specifiers.is_empty();
}
}
false
});

if !analyzer.has_type_only_imports() && !has_empty_imports {
return source.to_string();
}

Expand Down Expand Up @@ -740,8 +772,8 @@ pub fn filter_imports<'a>(
!analyzer.should_elide(name)
});

if removed.is_empty() {
// All specifiers kept, no changes needed
if removed.is_empty() && !kept.is_empty() {
// All specifiers kept and at least one exists, no changes needed
continue;
}

Expand Down Expand Up @@ -1870,4 +1902,104 @@ class MyComponent {
"myKey in parenthesized type literal should be preserved"
);
}

// =========================================================================
// Regression tests for ClickUp import elision mismatches
// =========================================================================

#[test]
fn test_namespace_import_type_only_should_be_elided() {
// Reproduces: bookmark.component.ts
// `import * as moment from 'moment'` where `moment` is only used as
// `moment.Moment` in type annotations should be elided.
let source = r#"
import * as moment from 'moment';
import { Component } from '@angular/core';

@Component({ selector: 'app-bookmark' })
class BookmarkComponent {
dueDate: moment.Moment = null;
}
"#;
let type_only = analyze_source(source);
// `moment` is only referenced in type position (moment.Moment)
// so it should be marked for elision
assert!(
type_only.contains("moment"),
"Namespace import `moment` used only in type annotation `moment.Moment` should be elided"
);
}

#[test]
fn test_namespace_import_with_value_usage_preserved() {
// Namespace import that IS used at runtime should be preserved
let source = r#"
import * as moment from 'moment';
import { Component } from '@angular/core';

@Component({ selector: 'app-test' })
class TestComponent {
now = moment();
}
"#;
let type_only = analyze_source(source);
assert!(
!type_only.contains("moment"),
"Namespace import `moment` used in value expression `moment()` should be preserved"
);
}

#[test]
fn test_forward_ref_arrow_function_preserves_value_use() {
// Reproduces: tags.component.ts
// `forwardRef(() => TagPickerComponent)` in @Component imports array
// uses TagPickerComponent as a value inside an arrow function.
// The arrow function body MUST be traversed to find value uses.
let source = r#"
import { Component, forwardRef, Inject, Optional, SkipSelf } from '@angular/core';
import { TagPickerComponent } from './tag-picker/tag-picker.component';

@Component({
selector: 'app-tags',
imports: [forwardRef(() => TagPickerComponent)]
})
class TagsComponent {
constructor(
@Optional()
@SkipSelf()
@Inject(TagPickerComponent)
readonly tagPickerComponent: TagPickerComponent,
) {}
}
"#;
let type_only = analyze_source(source);
// TagPickerComponent is used as a value in forwardRef(() => TagPickerComponent)
// and as @Inject(TagPickerComponent) argument. It must NOT be elided.
assert!(
!type_only.contains("TagPickerComponent"),
"TagPickerComponent used in forwardRef arrow function and @Inject should be preserved"
);
}

#[test]
fn test_empty_import_should_be_elided() {
// Reproduces: users-table.component.ts
// `import {} from '@cu/teams-pulse/types'` is an empty import that
// should be completely removed (no specifiers to keep).
let source = r#"
import { Component } from '@angular/core';
import {} from '@cu/teams-pulse/types';

@Component({ selector: 'app-users-table' })
class UsersTableComponent {}
"#;
let filtered = filter_source(source);

// The empty import should be removed entirely
assert!(
!filtered.contains("@cu/teams-pulse/types"),
"Empty import `import {{}} from '@cu/teams-pulse/types'` should be removed.\nFiltered:\n{}",
filtered
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -174,20 +174,6 @@ fn needs_temporary_in_safe_access(expr: &IrExpression<'_>) -> bool {
needs_temporary_in_safe_access(&keyed.receiver)
|| needs_temporary_in_safe_access(&keyed.key)
}
// Binary operators need to check both operands
IrExpression::Binary(bin) => {
needs_temporary_in_safe_access(&bin.lhs) || needs_temporary_in_safe_access(&bin.rhs)
}
// Ternary needs to check all branches
IrExpression::Ternary(ternary) => {
needs_temporary_in_safe_access(&ternary.condition)
|| needs_temporary_in_safe_access(&ternary.true_expr)
|| needs_temporary_in_safe_access(&ternary.false_expr)
}
// Not expression needs to check operand
IrExpression::Not(not) => needs_temporary_in_safe_access(&not.expr),
// Unary operator needs to check operand
IrExpression::Unary(unary) => needs_temporary_in_safe_access(&unary.expr),
// Check AST expressions for function calls
IrExpression::Ast(ast_expr) => needs_temporary_in_ast_expression(ast_expr),
// Parenthesized expressions need to check their inner expression
Expand Down
Loading