Skip to content

Commit 7cdb9a4

Browse files
Brooooooklynclaude
andauthored
fix: setClassMetadata generates namespace-prefixed types for @Inject params with different type annotations (#15)
When a constructor parameter uses `@Inject(TOKEN) param: SomeType`, the type annotation (SomeType) differs from the dep token (TOKEN). Previously only the token was checked for namespace prefixing, leaving the type annotation as a bare reference. Now the import map is consulted to generate `i1.SomeType` for imported types, while skipping type-only imports that have no runtime value. Also extends CrossFileAnalyzer to resolve monorepo package imports via tsconfig path aliases (skipping node_modules), and separates barrel resolution from type-only detection to avoid path mismatches with Angular's output. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3661037 commit 7cdb9a4

6 files changed

Lines changed: 310 additions & 60 deletions

File tree

crates/oxc_angular_compiler/src/class_metadata/builders.rs

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use oxc_ast::ast::{
1010
};
1111
use oxc_span::Atom;
1212

13-
use crate::component::{NamespaceRegistry, R3DependencyMetadata};
13+
use crate::component::{ImportMap, NamespaceRegistry, R3DependencyMetadata};
1414
use crate::output::ast::{
1515
ArrowFunctionBody, ArrowFunctionExpr, LiteralArrayExpr, LiteralExpr, LiteralMapEntry,
1616
LiteralMapExpr, LiteralValue, OutputExpression, ReadPropExpr, ReadVarExpr,
@@ -121,6 +121,7 @@ pub fn build_ctor_params_metadata<'a>(
121121
class: &Class<'a>,
122122
constructor_deps: Option<&[R3DependencyMetadata<'a>]>,
123123
namespace_registry: &mut NamespaceRegistry<'a>,
124+
import_map: &ImportMap<'a>,
124125
) -> Option<OutputExpression<'a>> {
125126
// Find constructor
126127
let constructor = class.body.body.iter().find_map(|element| {
@@ -144,6 +145,7 @@ pub fn build_ctor_params_metadata<'a>(
144145
param,
145146
constructor_deps.and_then(|deps| deps.get(i)),
146147
namespace_registry,
148+
import_map,
147149
)
148150
.unwrap_or_else(|| {
149151
OutputExpression::Literal(Box::new_in(
@@ -278,16 +280,17 @@ pub fn build_prop_decorators_metadata<'a>(
278280
/// TypeScript type annotations are erased at runtime, so imported types need namespace
279281
/// imports (e.g., `i1.SomeService`) to be available as runtime values.
280282
///
281-
/// The `dep.token_source_module` tracks where the injection token comes from. We only
282-
/// use it for namespace prefix when the type annotation name matches the dep token name,
283-
/// confirming that the dep's source module applies to the type. When they differ
284-
/// (e.g., `@Inject(DOCUMENT) doc: Document`), we fall back to bare name since the type
285-
/// may be a global or from a different module.
283+
/// When the type annotation name matches the dep token name, the dep's `token_source_module`
284+
/// is used directly. When they differ (e.g., `@Inject(DARK_THEME) theme$: Observable<boolean>`),
285+
/// we look up the type annotation name in the `import_map` to find its source module
286+
/// independently. This matches Angular's behavior where type references in `setClassMetadata`
287+
/// always use namespace-prefixed imports regardless of whether `@Inject` is used.
286288
fn build_param_type_expression<'a>(
287289
allocator: &'a Allocator,
288290
param: &FormalParameter<'a>,
289291
dep: Option<&R3DependencyMetadata<'a>>,
290292
namespace_registry: &mut NamespaceRegistry<'a>,
293+
import_map: &ImportMap<'a>,
291294
) -> Option<OutputExpression<'a>> {
292295
// Extract the type name from the type annotation
293296
let type_name = extract_param_type_name(param);
@@ -323,7 +326,39 @@ fn build_param_type_expression<'a>(
323326
}
324327
}
325328

329+
// When the type annotation differs from the dep token (e.g., @Inject(TOKEN) param: SomeType),
330+
// look up the type annotation name in the import_map to find its source module independently.
331+
// Only generate namespace-prefixed references for non-type-only imports, since type-only
332+
// imports (`import type { X }` / `import { type X }`) are erased at runtime and don't
333+
// resolve to values. Angular's compiler uses typeToValue() which skips interfaces and
334+
// type aliases; checking is_type_only is the closest heuristic without a full type checker.
335+
if let Some(ref tn) = type_name {
336+
if let Some(import_info) = import_map.get(tn) {
337+
if import_info.is_type_only {
338+
// Type-only imports are erased at runtime — emit undefined.
339+
return None;
340+
}
341+
let namespace = namespace_registry.get_or_assign(&import_info.source_module);
342+
return Some(OutputExpression::ReadProp(Box::new_in(
343+
ReadPropExpr {
344+
receiver: Box::new_in(
345+
OutputExpression::ReadVar(Box::new_in(
346+
ReadVarExpr { name: namespace, source_span: None },
347+
allocator,
348+
)),
349+
allocator,
350+
),
351+
name: tn.clone(),
352+
optional: false,
353+
source_span: None,
354+
},
355+
allocator,
356+
)));
357+
}
358+
}
359+
326360
// Fall back to extracting the bare type name from the type annotation
361+
// (for local/global types not in the import_map)
327362
extract_param_type_expression(allocator, param)
328363
}
329364

crates/oxc_angular_compiler/src/component/cross_file_elision.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,18 +93,18 @@ impl CrossFileAnalyzer {
9393
import_name: &str,
9494
from_file: &Path,
9595
) -> bool {
96-
// Skip node_modules - assume they export values (conservative)
97-
if import_source.starts_with('@') || !import_source.starts_with('.') {
98-
// Package imports - check cache first
99-
// For @angular/core, @bitwarden/common, etc., we cannot easily analyze
100-
// Return false (conservative - assume value) unless we've already cached it
101-
return false;
102-
}
103-
10496
// Resolve the import path to a file
10597
let resolved = match from_file.parent() {
10698
Some(parent) => match self.resolver.resolve(parent, import_source) {
107-
Ok(resolution) => resolution.full_path().to_string_lossy().to_string(),
99+
Ok(resolution) => {
100+
let full_path = resolution.full_path();
101+
// Skip node_modules - pre-compiled packages don't have interface
102+
// declarations visible in their source. Assume value (conservative).
103+
if full_path.components().any(|c| c.as_os_str() == "node_modules") {
104+
return false;
105+
}
106+
full_path.to_string_lossy().to_string()
107+
}
108108
Err(_) => return false, // Cannot resolve - assume value (conservative)
109109
},
110110
None => return false,

0 commit comments

Comments
 (0)