From efe376af3b2cb3e023e5b0b10a472c22405ec4c2 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 4 Feb 2026 16:56:40 +0800 Subject: [PATCH] fix: setClassMetadata ctor params use namespace-prefixed types for imported deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Constructor parameter types in ɵsetClassMetadata were using bare names (e.g., SomeService) instead of namespace-prefixed references (e.g., i1.SomeService) for imported types. TypeScript erases type annotations at runtime, so imported types need namespace imports to remain available. The fix passes constructor dependency metadata and the namespace registry into build_ctor_params_metadata, and adds build_param_type_expression which uses namespace prefixes when the type annotation name matches the dep token name and the dep has a source module. When they differ (e.g., @Inject(DOCUMENT) doc: Document), falls back to bare name. Co-Authored-By: Claude Opus 4.5 --- .../src/class_metadata/builders.rs | 96 ++++++++++- .../src/component/transform.rs | 10 +- crates/oxc_angular_compiler/src/lib.rs | 8 +- .../tests/integration_test.rs | 163 ++++++++++++++++++ napi/angular-compiler/src/lib.rs | 7 +- 5 files changed, 274 insertions(+), 10 deletions(-) diff --git a/crates/oxc_angular_compiler/src/class_metadata/builders.rs b/crates/oxc_angular_compiler/src/class_metadata/builders.rs index fcbf4b671..b20fb3e17 100644 --- a/crates/oxc_angular_compiler/src/class_metadata/builders.rs +++ b/crates/oxc_angular_compiler/src/class_metadata/builders.rs @@ -10,6 +10,7 @@ use oxc_ast::ast::{ }; use oxc_span::Atom; +use crate::component::{NamespaceRegistry, R3DependencyMetadata}; use crate::output::ast::{ ArrowFunctionBody, ArrowFunctionExpr, LiteralArrayExpr, LiteralExpr, LiteralMapEntry, LiteralMapExpr, LiteralValue, OutputExpression, ReadPropExpr, ReadVarExpr, @@ -110,9 +111,16 @@ pub fn build_decorator_metadata_array<'a>( /// /// Creates: `() => [{ type: SomeService, decorators: [...] }, ...]` /// Returns `None` if the class has no constructor. +/// +/// For imported types, generates namespace-prefixed references (e.g., `i1.SomeService`) +/// using the constructor dependency metadata and namespace registry. This matches +/// Angular's behavior where type-only imports need namespace imports because +/// TypeScript types are erased at runtime. pub fn build_ctor_params_metadata<'a>( allocator: &'a Allocator, class: &Class<'a>, + constructor_deps: Option<&[R3DependencyMetadata<'a>]>, + namespace_registry: &mut NamespaceRegistry<'a>, ) -> Option> { // Find constructor let constructor = class.body.body.iter().find_map(|element| { @@ -126,11 +134,18 @@ pub fn build_ctor_params_metadata<'a>( let mut param_entries = AllocVec::new_in(allocator); - for param in constructor { + for (i, param) in constructor.iter().enumerate() { let mut map_entries = AllocVec::new_in(allocator); - // Extract type from TypeScript type annotation - let type_expr = extract_param_type_expression(allocator, param).unwrap_or_else(|| { + // Extract type from TypeScript type annotation, using namespace-prefixed + // references for imported types when constructor dependency info is available. + let type_expr = build_param_type_expression( + allocator, + param, + constructor_deps.and_then(|deps| deps.get(i)), + namespace_registry, + ) + .unwrap_or_else(|| { OutputExpression::Literal(Box::new_in( LiteralExpr { value: LiteralValue::Undefined, source_span: None }, allocator, @@ -257,7 +272,80 @@ pub fn build_prop_decorators_metadata<'a>( // Internal helper functions // ============================================================================ -/// Extract the type expression from a constructor parameter. +/// Build the type expression for a constructor parameter, using namespace-prefixed +/// references for imported types. +/// +/// TypeScript type annotations are erased at runtime, so imported types need namespace +/// imports (e.g., `i1.SomeService`) to be available as runtime values. +/// +/// The `dep.token_source_module` tracks where the injection token comes from. We only +/// use it for namespace prefix when the type annotation name matches the dep token name, +/// confirming that the dep's source module applies to the type. When they differ +/// (e.g., `@Inject(DOCUMENT) doc: Document`), we fall back to bare name since the type +/// may be a global or from a different module. +fn build_param_type_expression<'a>( + allocator: &'a Allocator, + param: &FormalParameter<'a>, + dep: Option<&R3DependencyMetadata<'a>>, + namespace_registry: &mut NamespaceRegistry<'a>, +) -> Option> { + // Extract the type name from the type annotation + let type_name = extract_param_type_name(param); + + // Use namespace prefix when the type annotation matches the dep token name + // and the dep has a source module (imported type). + if let Some(dep) = dep { + if let Some(ref source_module) = dep.token_source_module { + if let Some(ref token) = dep.token { + let type_matches_token = + type_name.as_ref().is_some_and(|tn| tn.as_str() == token.as_str()); + + if type_matches_token { + let name = type_name.unwrap_or_else(|| token.clone()); + let namespace = namespace_registry.get_or_assign(source_module); + return Some(OutputExpression::ReadProp(Box::new_in( + ReadPropExpr { + receiver: Box::new_in( + OutputExpression::ReadVar(Box::new_in( + ReadVarExpr { name: namespace, source_span: None }, + allocator, + )), + allocator, + ), + name, + optional: false, + source_span: None, + }, + allocator, + ))); + } + } + } + } + + // Fall back to extracting the bare type name from the type annotation + extract_param_type_expression(allocator, param) +} + +/// Extract the type name (as an Atom) from a constructor parameter's type annotation. +/// +/// Returns the simple type name from the annotation, if present. +/// Used to get the type name for namespace-prefixed references in metadata. +fn extract_param_type_name<'a>(param: &FormalParameter<'a>) -> Option> { + let type_annotation = param.type_annotation.as_ref()?; + match &type_annotation.type_annotation { + TSType::TSTypeReference(type_ref) => match &type_ref.type_name { + TSTypeName::IdentifierReference(id) => Some(id.name), + TSTypeName::QualifiedName(qualified) => Some(qualified.right.name), + TSTypeName::ThisExpression(_) => None, + }, + _ => None, + } +} + +/// Extract the type expression from a constructor parameter's type annotation. +/// +/// This is the fallback path for local types that don't need namespace prefixes. fn extract_param_type_expression<'a>( allocator: &'a Allocator, param: &FormalParameter<'a>, diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index d5c56b94a..d0329fdb2 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -779,6 +779,11 @@ pub fn transform_angular_file( ); // Build metadata from the class AST + // Pass constructor deps and namespace registry so that + // imported types get namespace-prefixed references + // (e.g., i1.SomeService instead of bare SomeService) + let ctor_deps_slice = + metadata.constructor_deps.as_ref().map(|v| v.as_slice()); let class_metadata = R3ClassMetadata { r#type: type_expr, decorators: build_decorator_metadata_array( @@ -786,7 +791,10 @@ pub fn transform_angular_file( &[decorator], ), ctor_parameters: build_ctor_params_metadata( - allocator, class, + allocator, + class, + ctor_deps_slice, + &mut file_namespace_registry, ), prop_decorators: build_prop_decorators_metadata( allocator, class, diff --git a/crates/oxc_angular_compiler/src/lib.rs b/crates/oxc_angular_compiler/src/lib.rs index 77a8c8ceb..eeed4bfda 100644 --- a/crates/oxc_angular_compiler/src/lib.rs +++ b/crates/oxc_angular_compiler/src/lib.rs @@ -57,10 +57,10 @@ pub use transform::{HtmlToR3Transform, html_to_r3::html_ast_to_r3_ast}; pub use component::{ AngularVersion, ChangeDetectionStrategy, CompiledComponent, ComponentMetadata, HmrTemplateCompileOutput, HostMetadata, HostMetadataInput, ImportInfo, ImportMap, - ResolvedResources, TemplateCompileOutput, TransformOptions, TransformResult, ViewEncapsulation, - build_import_map, compile_component_template, compile_for_hmr, compile_template_for_hmr, - compile_template_to_js, compile_template_to_js_with_options, extract_component_metadata, - transform_angular_file, + NamespaceRegistry, ResolvedResources, TemplateCompileOutput, TransformOptions, TransformResult, + ViewEncapsulation, build_import_map, compile_component_template, compile_for_hmr, + compile_template_for_hmr, compile_template_to_js, compile_template_to_js_with_options, + extract_component_metadata, transform_angular_file, }; // Re-export cross-file elision types when feature is enabled diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index dbc37245c..415c044d6 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -3977,3 +3977,166 @@ fn test_for_index_xref_with_i18n_attribute_binding() { // matching Angular TS which stores direct i18n.Message object references on BindingOp. insta::assert_snapshot!("for_index_xref_with_i18n_attribute_binding", js); } + +/// Tests that setClassMetadata uses namespace-prefixed type references for imported +/// constructor parameter types. +/// +/// Angular's TypeScript compiler distinguishes between local and imported types in +/// the ɵsetClassMetadata constructor parameter metadata: +/// - Local types use bare names: `{ type: LocalService }` +/// - Imported types use namespace-prefixed names: `{ type: i1.ImportedService }` +/// +/// This is because TypeScript type annotations are erased at runtime, so imported +/// types need namespace imports (i0, i1, i2...) to be available as runtime values. +/// The factory function (ɵfac) already handles this correctly via R3DependencyMetadata +/// and create_token_expression, but setClassMetadata was using bare names for all types. +#[test] +fn test_set_class_metadata_uses_namespace_for_imported_ctor_params() { + let allocator = Allocator::default(); + let source = r#" +import { Component } from '@angular/core'; +import { SomeService } from './some.service'; + +@Component({ + selector: 'test-comp', + template: '
hello
', + standalone: true, +}) +export class TestComponent { + constructor(private svc: SomeService) {} +} +"#; + + let options = ComponentTransformOptions { + emit_class_metadata: true, + ..ComponentTransformOptions::default() + }; + + let result = transform_angular_file(&allocator, "test.component.ts", source, &options, None); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + // Extract the setClassMetadata section specifically (not the factory function) + let metadata_section = result + .code + .split("ɵsetClassMetadata") + .nth(1) + .expect("setClassMetadata should be present in output"); + + // The ctor_parameters callback should use namespace-prefixed type for + // the imported SomeService: `{type:i1.SomeService}` not `{type:SomeService}` + assert!( + metadata_section.contains("i1.SomeService"), + "setClassMetadata ctor_parameters should use namespace-prefixed type (i1.SomeService) for imported constructor parameter. Metadata section:\n{}", + metadata_section + ); + assert!( + !metadata_section.contains("type:SomeService}"), + "setClassMetadata should NOT use bare type name for imported types. Metadata section:\n{}", + metadata_section + ); +} + +/// Tests that setClassMetadata uses namespace-prefixed type even when @Inject is present. +/// +/// When a constructor parameter has both a type annotation and @Inject decorator pointing +/// to the same imported class, the metadata `type` field should still use namespace prefix. +/// The factory correctly uses bare names for @Inject tokens with named imports, but the +/// metadata type always represents the TypeScript type annotation which is erased at runtime. +/// +/// Example: +/// - Factory: `ɵɵdirectiveInject(TagPickerComponent, 12)` (bare - ok, @Inject value import) +/// - Metadata: `{ type: i1.TagPickerComponent, decorators: [{type: Inject, ...}] }` (namespace) +#[test] +fn test_set_class_metadata_namespace_with_inject_decorator() { + let allocator = Allocator::default(); + let source = r#" +import { Component, Inject, Optional, SkipSelf } from '@angular/core'; +import { SomeService } from './some.service'; + +@Component({ + selector: 'test-comp', + template: '
hello
', + standalone: true, +}) +export class TestComponent { + constructor( + @Optional() @SkipSelf() @Inject(SomeService) private svc: SomeService + ) {} +} +"#; + + let options = ComponentTransformOptions { + emit_class_metadata: true, + ..ComponentTransformOptions::default() + }; + + let result = transform_angular_file(&allocator, "test.component.ts", source, &options, None); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + // Extract the setClassMetadata section + let metadata_section = result + .code + .split("ɵsetClassMetadata") + .nth(1) + .expect("setClassMetadata should be present in output"); + + // Even with @Inject(SomeService), the type field should use namespace prefix + // because the type annotation is erased by TypeScript + assert!( + metadata_section.contains("i1.SomeService"), + "setClassMetadata should use namespace-prefixed type even with @Inject. Metadata section:\n{}", + metadata_section + ); +} + +/// Tests that when @Inject token differs from the type annotation (e.g., @Inject(DOCUMENT) +/// on a parameter typed as Document), the metadata type uses bare name since the type +/// annotation may reference a global or different module than the injection token. +#[test] +fn test_set_class_metadata_inject_differs_from_type() { + let allocator = Allocator::default(); + let source = r#" +import { Component, Inject } from '@angular/core'; +import { DOCUMENT } from '@angular/common'; + +@Component({ + selector: 'test-comp', + template: '
hello
', + standalone: true, +}) +export class TestComponent { + constructor(@Inject(DOCUMENT) private doc: Document) {} +} +"#; + + let options = ComponentTransformOptions { + emit_class_metadata: true, + ..ComponentTransformOptions::default() + }; + + let result = transform_angular_file(&allocator, "test.component.ts", source, &options, None); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let metadata_section = result + .code + .split("ɵsetClassMetadata") + .nth(1) + .expect("setClassMetadata should be present in output"); + + // The type should be bare "Document" (global type), not namespace-prefixed + // even though the @Inject token (DOCUMENT) is from @angular/common + assert!( + metadata_section.contains("type:Document"), + "setClassMetadata should use bare type for globals when @Inject token differs. Metadata section:\n{}", + metadata_section + ); + // Should NOT add namespace prefix for Document + assert!( + !metadata_section.contains("i1.Document"), + "setClassMetadata should NOT namespace-prefix global types. Metadata section:\n{}", + metadata_section + ); +} diff --git a/napi/angular-compiler/src/lib.rs b/napi/angular-compiler/src/lib.rs index f76b41cfe..d276ab218 100644 --- a/napi/angular-compiler/src/lib.rs +++ b/napi/angular-compiler/src/lib.rs @@ -1928,7 +1928,12 @@ pub fn compile_class_metadata_sync( let decorators_expr = core_build_decorator_metadata_array(&allocator, &[decorator_ref]); // Build constructor parameters metadata - let ctor_params_expr = core_build_ctor_params_metadata(&allocator, class); + // This standalone API doesn't have full transform pipeline context (constructor deps + // and namespace registry), so imported types won't get namespace prefixes. + // The full transform_angular_file pipeline handles namespace prefixes correctly. + let mut namespace_registry = oxc_angular_compiler::NamespaceRegistry::new(&allocator); + let ctor_params_expr = + core_build_ctor_params_metadata(&allocator, class, None, &mut namespace_registry); // Build property decorators metadata let prop_decorators_expr = core_build_prop_decorators_metadata(&allocator, class);