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
9 changes: 7 additions & 2 deletions crates/oxc_angular_compiler/src/class_metadata/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,9 @@ fn build_param_type_expression<'a>(
/// Used to get the type name for namespace-prefixed references in metadata.
fn extract_param_type_name<'a>(param: &FormalParameter<'a>) -> Option<Ident<'a>> {
let type_annotation = param.type_annotation.as_ref()?;
match &type_annotation.type_annotation {
// Narrow `T | null` unions to `T` so optional-DI patterns expose the type.
let ts_type = crate::util::resolve_di_token_type(&type_annotation.type_annotation)?;
match ts_type {
TSType::TSTypeReference(type_ref) => match &type_ref.type_name {
TSTypeName::IdentifierReference(id) => Some(id.name.into()),
TSTypeName::QualifiedName(qualified) => Some(qualified.right.name.into()),
Expand All @@ -381,8 +383,11 @@ fn extract_param_type_expression<'a>(
// Get the type annotation from the formal parameter
let type_annotation = param.type_annotation.as_ref()?;

// Narrow `T | null` unions to `T` so optional-DI patterns expose the type.
let ts_type = crate::util::resolve_di_token_type(&type_annotation.type_annotation)?;

// Extract the type name from the annotation
match &type_annotation.type_annotation {
match ts_type {
TSType::TSTypeReference(type_ref) => {
// Handle simple type references like SomeService
match &type_ref.type_name {
Expand Down
133 changes: 132 additions & 1 deletion crates/oxc_angular_compiler/src/component/decorator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1029,7 +1029,8 @@ fn extract_inject_token<'a>(arg: &'a Argument<'a>) -> Option<Ident<'a>> {
fn extract_param_token<'a>(param: &'a oxc_ast::ast::FormalParameter<'a>) -> Option<Ident<'a>> {
// Get the type annotation (directly on FormalParameter)
let type_annotation = param.type_annotation.as_ref()?;
let ts_type = &type_annotation.type_annotation;
// Narrow `T | null` unions to `T` to match the reference compiler.
let ts_type = crate::util::resolve_di_token_type(&type_annotation.type_annotation)?;

// Handle TSTypeReference: SomeClass, SomeModule, etc.
if let oxc_ast::ast::TSType::TSTypeReference(type_ref) = ts_type {
Expand Down Expand Up @@ -2635,6 +2636,136 @@ mod tests {
});
}

#[test]
fn test_component_optional_with_nullable_type() {
// Regression test for issue #285:
// `@Optional() svc: MyService | null` is the canonical optional-DI pattern,
// but the union with `null` previously caused token extraction to fail and
// emit `ɵɵinvalidFactoryDep`, which throws at runtime.
//
// Angular's reference compiler filters out `null` literal type nodes from
// the union; when exactly one type remains it becomes the token.
let code = r#"
@Component({
selector: 'app-test',
template: ''
})
class TestComponent {
constructor(
@Optional() private svc: MyService | null,
) {}
}
"#;
assert_metadata(code, |meta| {
let deps = meta.constructor_deps.as_ref().unwrap();
assert_eq!(deps.len(), 1);
let dep = &deps[0];
assert!(dep.optional, "Should have optional flag");
assert_eq!(
dep.token.as_ref().expect("token should resolve to MyService").as_str(),
"MyService",
"null should be filtered from the union, leaving MyService as the token",
);
});
}

#[test]
fn test_component_nullable_type_without_decorator() {
// Bare `svc: MyService | null` (no `@Optional()`) should also have its
// token resolved to `MyService`. Token resolution is independent of
// optionality — `@Optional()` only controls the inject flag.
let code = r#"
@Component({
selector: 'app-test',
template: ''
})
class TestComponent {
constructor(
private svc: MyService | null,
) {}
}
"#;
assert_metadata(code, |meta| {
let deps = meta.constructor_deps.as_ref().unwrap();
assert_eq!(deps.len(), 1);
let dep = &deps[0];
assert!(!dep.optional, "Should not have optional flag");
assert_eq!(
dep.token.as_ref().expect("token should resolve to MyService").as_str(),
"MyService",
);
});
}

#[test]
fn test_component_nullable_type_null_first() {
// `null` can appear in either position of the union — both must be filtered.
let code = r#"
@Component({
selector: 'app-test',
template: ''
})
class TestComponent {
constructor(
@Optional() private svc: null | MyService,
) {}
}
"#;
assert_metadata(code, |meta| {
let deps = meta.constructor_deps.as_ref().unwrap();
let dep = &deps[0];
assert_eq!(dep.token.as_ref().unwrap().as_str(), "MyService");
});
}

#[test]
fn test_component_parenthesized_nullable_type() {
// Parenthesized nullable union: `(MyService | null)`.
// oxc preserves `TSParenthesizedType` in the AST, so the resolver must
// unwrap it before checking for unions, or the dependency falls through
// to `ɵɵinvalidFactoryDep` (issue #285 follow-up).
let code = r#"
@Component({
selector: 'app-test',
template: ''
})
class TestComponent {
constructor(
@Optional() private svc: (MyService | null),
) {}
}
"#;
assert_metadata(code, |meta| {
let deps = meta.constructor_deps.as_ref().unwrap();
let dep = &deps[0];
assert_eq!(
dep.token.as_ref().expect("token should resolve to MyService").as_str(),
"MyService",
);
});
}

#[test]
fn test_component_paren_around_inner_type_in_union() {
// Parens around just the non-null side: `(MyService) | null`.
let code = r#"
@Component({
selector: 'app-test',
template: ''
})
class TestComponent {
constructor(
@Optional() private svc: (MyService) | null,
) {}
}
"#;
assert_metadata(code, |meta| {
let deps = meta.constructor_deps.as_ref().unwrap();
let dep = &deps[0];
assert_eq!(dep.token.as_ref().unwrap().as_str(), "MyService");
});
}

#[test]
fn test_component_with_skip_self_decorator() {
let code = r#"
Expand Down
27 changes: 26 additions & 1 deletion crates/oxc_angular_compiler/src/directive/decorator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,8 @@ fn extract_param_token<'a>(
) -> Option<OutputExpression<'a>> {
// Get the type annotation (directly on FormalParameter)
let type_annotation = param.type_annotation.as_ref()?;
let ts_type = &type_annotation.type_annotation;
// Narrow `T | null` unions to `T` to match the reference compiler.
let ts_type = crate::util::resolve_di_token_type(&type_annotation.type_annotation)?;

// Handle TSTypeReference: SomeClass, SomeModule, etc.
if let oxc_ast::ast::TSType::TSTypeReference(type_ref) = ts_type {
Expand Down Expand Up @@ -1536,6 +1537,30 @@ mod tests {
});
}

#[test]
fn test_directive_optional_with_nullable_type() {
// Regression test for issue #285:
// `@Optional() svc: MyService | null` must resolve the token to `MyService`.
let code = r#"
@Directive({ selector: '[myDir]' })
class MyDirective {
constructor(@Optional() private svc: MyService | null) {}
}
"#;
assert_directive_metadata(code, |meta| {
let deps = meta.deps.as_ref().expect("Directive should have deps");
assert_eq!(deps.len(), 1);
let dep = &deps[0];
assert!(dep.optional, "Should have optional flag");
let token = dep.token.as_ref().expect("token should resolve to MyService");
if let crate::output::ast::OutputExpression::ReadVar(var) = token {
assert_eq!(var.name.as_str(), "MyService");
} else {
panic!("expected ReadVar token, got {:?}", token);
}
});
}

#[test]
fn test_extract_param_token_returns_read_var_not_read_prop() {
// Regression test for bug where extract_param_token() returned
Expand Down
29 changes: 28 additions & 1 deletion crates/oxc_angular_compiler/src/injectable/decorator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,8 @@ fn extract_param_token<'a>(
) -> Option<OutputExpression<'a>> {
// First try to get the type annotation (directly on FormalParameter, not on pattern)
let type_annotation = param.type_annotation.as_ref()?;
let ts_type = &type_annotation.type_annotation;
// Narrow `T | null` unions to `T` to match the reference compiler.
let ts_type = crate::util::resolve_di_token_type(&type_annotation.type_annotation)?;

// Handle TSTypeReference: SomeClass, SomeModule, etc.
if let oxc_ast::ast::TSType::TSTypeReference(type_ref) = ts_type {
Expand Down Expand Up @@ -917,6 +918,32 @@ mod tests {
assert!(deps[2].token.is_some(), "Third dep should have token");
}

#[test]
fn test_injectable_optional_with_nullable_type() {
// Regression test for issue #285:
// `@Optional() svc: MyService | null` must resolve the token to `MyService`.
let allocator = Allocator::default();
let source = r#"
@Injectable()
class MyService {
constructor(@Optional() private svc: OtherService | null) {}
}
"#;

let metadata = parse_and_extract(&allocator, source).expect("Should have metadata");
let deps = metadata.deps.as_ref().expect("Should have constructor deps");
assert_eq!(deps.len(), 1);
let dep = &deps[0];
assert!(dep.optional, "Should have optional flag");
let token = dep.token.as_ref().expect("token should resolve to OtherService");
match token {
crate::output::ast::OutputExpression::ReadVar(var) => {
assert_eq!(var.name.as_str(), "OtherService");
}
other => panic!("expected ReadVar token, got {:?}", other),
}
}

#[test]
fn test_injectable_with_optional_and_skip_self() {
let allocator = Allocator::default();
Expand Down
28 changes: 27 additions & 1 deletion crates/oxc_angular_compiler/src/ng_module/decorator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,8 @@ fn extract_param_token<'a>(
) -> Option<OutputExpression<'a>> {
// First try to get the type annotation (directly on FormalParameter, not on pattern)
let type_annotation = param.type_annotation.as_ref()?;
let ts_type = &type_annotation.type_annotation;
// Narrow `T | null` unions to `T` to match the reference compiler.
let ts_type = crate::util::resolve_di_token_type(&type_annotation.type_annotation)?;

// Handle TSTypeReference: SomeClass, SomeModule, etc.
if let oxc_ast::ast::TSType::TSTypeReference(type_ref) = ts_type {
Expand Down Expand Up @@ -902,4 +903,29 @@ mod tests {
assert!(dep.token.is_some(), "Token should be extracted from type annotation");
});
}

#[test]
fn test_ng_module_optional_with_nullable_type() {
// Regression test for issue #285:
// `@Optional() svc: MyService | null` must resolve the token to `MyService`.
let code = r#"
@NgModule({})
class AppModule {
constructor(@Optional() private parent: AppModule | null) {}
}
"#;
assert_metadata(code, |meta| {
let deps = meta.deps.as_ref().expect("Should have constructor deps");
assert_eq!(deps.len(), 1);
let dep = &deps[0];
assert!(dep.optional);
let token = dep.token.as_ref().expect("token should resolve to AppModule");
match token {
crate::output::ast::OutputExpression::ReadVar(var) => {
assert_eq!(var.name.as_str(), "AppModule");
}
other => panic!("expected ReadVar token, got {:?}", other),
}
});
}
}
29 changes: 28 additions & 1 deletion crates/oxc_angular_compiler/src/pipe/decorator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,8 @@ fn extract_param_token<'a>(
) -> Option<OutputExpression<'a>> {
// First try to get the type annotation (directly on FormalParameter, not on pattern)
let type_annotation = param.type_annotation.as_ref()?;
let ts_type = &type_annotation.type_annotation;
// Narrow `T | null` unions to `T` to match the reference compiler.
let ts_type = crate::util::resolve_di_token_type(&type_annotation.type_annotation)?;

// Handle TSTypeReference: SomeClass, SomeModule, etc.
if let oxc_ast::ast::TSType::TSTypeReference(type_ref) = ts_type {
Expand Down Expand Up @@ -569,6 +570,32 @@ mod tests {
});
}

#[test]
fn test_pipe_optional_with_nullable_type() {
// Regression test for issue #285:
// `@Optional() svc: MyService | null` must resolve the token to `MyService`.
let code = r#"
@Pipe({ name: 'myPipe' })
class MyPipe {
constructor(@Optional() private svc: MyService | null) {}
}
"#;
assert_metadata(code, |meta| {
let deps = meta.deps.as_ref().expect("Should have deps");
assert_eq!(deps.len(), 1);
let dep = &deps[0];
assert!(dep.optional);
let token = dep.token.as_ref().expect("token should resolve to MyService");
let token: &crate::output::ast::OutputExpression<'_> = token;
match token {
crate::output::ast::OutputExpression::ReadVar(var) => {
assert_eq!(var.name.as_str(), "MyService");
}
other => panic!("expected ReadVar token, got {:?}", other),
}
});
}

// =========================================================================
// Edge cases
// =========================================================================
Expand Down
2 changes: 2 additions & 0 deletions crates/oxc_angular_compiler/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
pub mod chars;
mod deferred_time;
mod parse_util;
mod type_extract;

pub use deferred_time::*;
pub use parse_util::*;
pub use type_extract::*;
Loading
Loading