From 378149216cea4d76b135c82914c8b36d40a469bb Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 22:04:50 +0800 Subject: [PATCH 01/18] fix(metadata): give const enum ctor params `type: undefined` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ngtsc's typeToValue has no value for a const enum outside local compilation, so full ɵsetClassMetadata and partial ɵɵngDeclareClassMetadata write `type: undefined` for a constructor parameter typed by one. oxc wrote the name, which for a `declare const enum` threw a ReferenceError when JIT or TestBed read it. - Same file: `const enum`, `declare const enum` and `ns.E` now give `undefined`. A name a namespace, interface or `declare global` also declares keeps the reference, and so does a class nested in a function or block, since one file can't be sure what it means there. - Imported: with `resolve_imported_values`, a named import the cross-file analyzer finds to be a const enum (also through `export { X } from` and `export *`) gives `undefined` and no `i1` import. The file read becomes a watch dependency. Namespace imports, packages, .d.ts-only modules and files with a bare namespace keep the reference. - Factories and DI tokens are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../src/class_metadata/builders.rs | 88 +++- .../src/component/cross_file_elision.rs | 125 ++++++ .../src/component/transform.rs | 65 +++ .../tests/const_enum_ctor_param_test.rs | 378 ++++++++++++++++++ 4 files changed, 651 insertions(+), 5 deletions(-) create mode 100644 crates/oxc_angular_compiler/tests/const_enum_ctor_param_test.rs diff --git a/crates/oxc_angular_compiler/src/class_metadata/builders.rs b/crates/oxc_angular_compiler/src/class_metadata/builders.rs index 653d447f3..078294f09 100644 --- a/crates/oxc_angular_compiler/src/class_metadata/builders.rs +++ b/crates/oxc_angular_compiler/src/class_metadata/builders.rs @@ -6,7 +6,7 @@ use oxc_allocator::{Allocator, Box, Vec as AllocVec}; use oxc_ast::ast::{ BindingPattern, ChainElement, Class, ClassElement, Declaration, Decorator, Expression, - ForStatementInit, ForStatementLeft, FormalParameter, ObjectPropertyKind, PropertyKey, + ForStatementInit, ForStatementLeft, FormalParameter, ObjectPropertyKind, Program, PropertyKey, Statement, TSNamespaceDeclarationBody, TSType, TSTypeName, VariableDeclaration, }; use oxc_span::GetSpan; @@ -449,6 +449,13 @@ pub fn build_ctor_params_metadata_in<'a>( // without it these checks are skipped. let class_type_params = type_param_names_of(class); let local_decls = consts.and_then(StringConsts::local_decls); + // A `const enum` is read by its name only for a class declared at the + // file top level: a class nested in a function or block may see another + // declaration of that name, which one file's top-level scan can't rule + // out, so it keeps today's output. + let const_enums_visible = consts + .and_then(StringConsts::program) + .is_some_and(|program| is_top_level_class(program, class)); let mut param_entries = AllocVec::new_in(&allocator); @@ -464,6 +471,7 @@ pub fn build_ctor_params_metadata_in<'a>( import_map, &class_type_params, local_decls, + const_enums_visible, ) .unwrap_or_else(|| { OutputExpression::Literal(Box::new_in( @@ -951,11 +959,18 @@ fn string_literal<'a>(allocator: &'a Allocator, value: Ident<'a>) -> OutputExpre /// `TSTypeReference` after `T | null` narrowing (parens, other unions, /// `typeof`, primitives, `this`), it's a bare class type parameter, a /// type-only or bare namespace import, or it ends at an interface or type -/// alias declared in the file. In local mode other value-less names (const -/// enums, vars, bare namespaces, missing members, undeclared names) emit as -/// written — upstream wraps them in `/* @ts-ignore */` rather than emitting +/// alias declared in the file. In local mode other value-less names (vars, +/// bare namespaces, missing members, undeclared names) emit as written — +/// upstream wraps them in `/* @ts-ignore */` rather than emitting /// `undefined`. The DI dependency is not consulted: upstream derives `type:` /// purely from the type annotation. +/// +/// `const enum`s follow full and partial compilation instead, the modes the +/// Angular CLI and ng-packagr run: there `typeToValue` gives +/// `noValueDeclaration` for a `ConstEnum` symbol, so `type:` is `undefined`. +/// That holds for one declared in the file and for a named import the +/// cross-file analysis found to be one ([`crate::ImportInfo::is_const_enum`]), +/// when `const_enums_visible` says the class sees the file-scope names. fn build_param_type_expression<'a>( allocator: &'a Allocator, param: &FormalParameter<'a>, @@ -963,6 +978,7 @@ fn build_param_type_expression<'a>( import_map: &ImportMap<'a>, type_params: &FxHashSet>, local_decls: Option<&LocalDecls<'a>>, + const_enums_visible: bool, ) -> Option> { // Upstream's getConstructorParameters first narrows `T | null` to `T` // (typescript.ts); `typeToValue` then requires a bare `TypeReferenceNode` @@ -993,6 +1009,11 @@ fn build_param_type_expression<'a>( if import_info.is_type_only || path.len() == 1 && !import_info.is_named_import { return None; } + // An imported `const enum` (`import { KeyCodes }`): no value in full + // or partial `typeToValue`, so no `i1` import either. + if const_enums_visible && path.len() == 1 && import_info.is_const_enum { + return None; + } // `import Foo from 'm'`: upstream emits the local name itself (LOCAL // with a `defaultImportStatement`), never `i1.default`. The specifier // kind matters — `import { default as Foo }` is IMPORTED and emits @@ -1034,7 +1055,7 @@ fn build_param_type_expression<'a>( // namespace names, missing members, undeclared or global names — emits // as written, matching the upstream `/* @ts-ignore */ type: X` fallback. if let Some(decls) = local_decls - && decls.is_type_only(&path) + && (decls.is_type_only(&path) || const_enums_visible && decls.is_const_enum(&path)) { return None; } @@ -1048,6 +1069,26 @@ fn build_param_type_expression<'a>( )) } +/// Whether `class` is declared in `program`'s body, not in a function, block +/// or namespace. +fn is_top_level_class(program: &Program<'_>, class: &Class<'_>) -> bool { + program.body.iter().any(|stmt| { + let top = match stmt { + Statement::ClassDeclaration(top) => top, + Statement::ExportDeclaration(export) => match &export.declaration { + Declaration::ClassDeclaration(top) => top, + _ => return false, + }, + Statement::ExportDefaultDeclaration(export) => match &export.declaration { + oxc_ast::ast::ExportDefaultDeclarationKind::ClassDeclaration(top) => top, + _ => return false, + }, + _ => return false, + }; + std::ptr::eq(top.as_ref(), class) + }) +} + /// The `TSTypeReference` a ctor-param annotation narrows to, mirroring /// `getConstructorParameters` (`typescript.ts`): `T | null` unions keep the /// single non-`null` member. Unlike `resolve_di_token_type` (DI tokens), this @@ -1156,6 +1197,9 @@ pub(crate) struct LocalDecls<'a> { /// Names a `declare global` block declares (root only). See /// [`Self::is_type_only_token`]. global_names: FxHashSet<&'a str>, + /// Names declared by a `const enum` (also in `value_decls`). See + /// [`Self::is_const_enum`]. + const_enums: FxHashSet<&'a str>, } impl<'a> LocalDecls<'a> { @@ -1242,6 +1286,9 @@ impl<'a> LocalDecls<'a> { } Statement::TSEnumDeclaration(decl) => { self.value_decls.insert(decl.id.name.as_str()); + if decl.r#const { + self.const_enums.insert(decl.id.name.as_str()); + } } Statement::TSImportEqualsDeclaration(decl) => { if decl.import_kind.is_type() { @@ -1350,6 +1397,9 @@ impl<'a> LocalDecls<'a> { } Declaration::TSEnumDeclaration(d) => { self.value_decls.insert(d.id.name.as_str()); + if d.r#const { + self.const_enums.insert(d.id.name.as_str()); + } } Declaration::TSImportEqualsDeclaration(d) => { if d.import_kind.is_type() { @@ -1434,6 +1484,7 @@ impl<'a> LocalDecls<'a> { fn merge(&mut self, other: LocalDecls<'a>) { self.type_decls.extend(other.type_decls); self.value_decls.extend(other.value_decls); + self.const_enums.extend(other.const_enums); for (name, nested) in other.namespaces { self.namespaces.entry(name).or_default().merge(nested); } @@ -1460,6 +1511,33 @@ impl<'a> LocalDecls<'a> { } } + /// Whether `path` — `["ns", ..., "Name"]` — ends at a `const enum` + /// (`declare const enum` too). ngtsc's `typeToValue` has no value for + /// it outside local compilation (`decl.flags & ts.SymbolFlags.ConstEnum` + /// gives `noValueDeclaration`), so full and partial metadata write + /// `type: undefined`. A name a namespace, interface, type alias or + /// `declare global` block also declares is left out: one file can't be + /// sure what it merges with, so it keeps today's output. + pub(crate) fn is_const_enum(&self, path: &[Ident<'_>]) -> bool { + match path.split_first() { + None => false, + Some((name, [])) => { + let name = name.as_str(); + self.const_enums.contains(name) + && !self.namespaces.contains_key(name) + && !self.type_decls.contains(name) + && !self.global_names.contains(name) + } + Some((name, rest)) => { + !self.global_names.contains(name.as_str()) + && self + .namespaces + .get(name.as_str()) + .is_some_and(|nested| nested.is_const_enum(rest)) + } + } + } + /// [`Self::is_type_only`] for a DI token of `class`'s constructor, leaving /// out names one file can't prove have no value: /// - the class's own name: a class nested in a block or function, named diff --git a/crates/oxc_angular_compiler/src/component/cross_file_elision.rs b/crates/oxc_angular_compiler/src/component/cross_file_elision.rs index 1a3c8d74e..fcc72a818 100644 --- a/crates/oxc_angular_compiler/src/component/cross_file_elision.rs +++ b/crates/oxc_angular_compiler/src/component/cross_file_elision.rs @@ -106,6 +106,9 @@ struct Inner { this: Weak, /// Memoized NgModule scope reads (see [`ng_module_scope`]). scope: ng_module_scope::ScopeState, + /// File path -> the names it exports a `const enum` under, filled by + /// `analyze_file` (see [`const_enum_export_names`]). + const_enum_exports: RefCell>>, } /// A file parsed once per analyzer: its program, and the consts the @@ -255,6 +258,7 @@ impl CrossFileAnalyzer { value_dependencies: RefCell::new(FxHashSet::default()), this: this.clone(), scope: ng_module_scope::ScopeState::default(), + const_enum_exports: RefCell::new(FxHashMap::default()), }), } } @@ -272,6 +276,20 @@ impl CrossFileAnalyzer { self.inner.is_type_only_import(import_source, import_name, from_file) } + /// Whether `import_name`, imported from `import_source` in `from_file`, + /// is a `const enum` where it is declared, following `export { X } from` + /// and `export *` chains. `false` when unsure (packages, `.d.ts`-only + /// modules, unresolvable paths, `export * as ns`). Files read are + /// recorded in [`Self::take_value_dependencies`]. + pub(crate) fn is_const_enum_import( + &self, + import_source: &str, + import_name: &str, + from_file: &Path, + ) -> bool { + self.inner.is_const_enum_import(import_source, import_name, from_file) + } + /// Resolve the actual source file path for an import, tracing through barrel exports. /// /// Returns the relative path from `from_file` to the actual source file where @@ -484,6 +502,68 @@ impl Inner { self.export_value_at(&resolved, import_name, 0) } + /// See [`CrossFileAnalyzer::is_const_enum_import`]. + fn is_const_enum_import( + &self, + import_source: &str, + import_name: &str, + from_file: &Path, + ) -> bool { + let Some(parent) = from_file.parent() else { return false }; + let Some(resolved) = self.resolve_module_spec(parent, import_source) else { + return false; + }; + self.const_enum_at(&resolved, import_name, 0) + } + + /// Whether `export_name` of `file_path` is a `const enum`, direct or + /// through re-exports, like [`Self::export_value_at`] follows them. + fn const_enum_at(&self, file_path: &str, export_name: &str, depth: u16) -> bool { + // A package's sources aren't read, like `is_type_only_import`. + if depth > MAX_EXPORT_CHAIN + || Path::new(file_path).components().any(|c| c.as_os_str() == "node_modules") + { + return false; + } + self.value_dependencies.borrow_mut().insert(file_path.to_string()); + if !self.cache.borrow().contains_key(file_path) { + if self.analyzing.borrow().contains(file_path) { + return false; // Circular import + } + self.analyze_file(file_path); + } + let export_info = { + let cache = self.cache.borrow(); + cache.get(file_path).and_then(|exports| exports.get(export_name).cloned()) + }; + let Some(export_info) = + export_info.or_else(|| self.find_in_star_exports(file_path, export_name)) + else { + return false; + }; + match export_info.re_export_source { + // `export * as ns` names a namespace, not one declaration. + Some((_, original_name)) if original_name == "*" => false, + Some((source_module, original_name)) => { + let Some(resolved) = Path::new(file_path) + .parent() + .and_then(|parent| self.resolve_module_spec(parent, &source_module)) + else { + return false; + }; + self.const_enum_at(&resolved, &original_name, depth + 1) + } + None => { + !export_info.is_type_only + && self + .const_enum_exports + .borrow() + .get(file_path) + .is_some_and(|names| names.contains(export_name)) + } + } + } + fn standalone_directive_selector( &self, import_source: &str, @@ -1024,6 +1104,9 @@ impl Inner { let source_type = SourceType::from_path(file_path).unwrap_or_default(); let parser_ret = Parser::new(&allocator, &source, source_type).parse(); self.analyze_program(&parser_ret.program, &mut exports); + self.const_enum_exports + .borrow_mut() + .insert(file_path.to_string(), const_enum_export_names(&parser_ret.program)); self.cache.borrow_mut().insert(file_path.to_string(), exports); self.analyzing.borrow_mut().remove(file_path); @@ -1309,6 +1392,7 @@ impl Inner { self.parsed.borrow_mut().clear(); self.value_memo.borrow_mut().clear(); self.selector_memo.borrow_mut().clear(); + self.const_enum_exports.borrow_mut().clear(); } /// Get the number of cached files. @@ -1317,6 +1401,47 @@ impl Inner { } } +/// The names `program` exports a top-level `const enum` under: +/// `export const enum E`, `export declare const enum E`, and `export { E }` / +/// `export { E as F }` of a top-level one. Empty when the file has a bare +/// `namespace`, `declare module` or `declare global` block: `analyze_statement` +/// lists their members as the file's own exports, so a name could mean either. +fn const_enum_export_names(program: &Program) -> FxHashSet { + let mut names = FxHashSet::default(); + let mut const_enums = FxHashSet::default(); + for stmt in &program.body { + match stmt { + Statement::TSNamespaceDeclaration(_) + | Statement::TSExternalModuleDeclaration(_) + | Statement::TSGlobalDeclaration(_) => return FxHashSet::default(), + Statement::TSEnumDeclaration(e) if e.r#const => { + const_enums.insert(e.id.name.as_str()); + } + Statement::ExportDeclaration(export) => { + if let Declaration::TSEnumDeclaration(e) = &export.declaration + && e.r#const + { + const_enums.insert(e.id.name.as_str()); + names.insert(e.id.name.to_string()); + } + } + _ => {} + } + } + for stmt in &program.body { + if let Statement::ExportNamedDeclaration(export) = stmt + && !export.export_kind.is_type() + { + for spec in &export.specifiers { + if !spec.export_kind.is_type() && const_enums.contains(spec.local.name().as_str()) { + names.insert(spec.exported.name().to_string()); + } + } + } + } + names +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index a16516720..d99082156 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -543,6 +543,12 @@ pub struct ImportInfo<'a> { /// declaration) while `{ default as Foo }` resolves IMPORTED /// (`i1.default`). pub is_default_import: bool, + /// Whether this named import is a `const enum` in the file it comes from, + /// as far as the cross-file analysis can tell (only with + /// `resolve_imported_values`). Read only by the class metadata `type:` of + /// a constructor parameter, which ngtsc writes as `undefined` for one. + /// DI tokens, `imports:` and `@defer` don't read it. + pub is_const_enum: bool, } /// Map from local identifier name to its import information. @@ -643,6 +649,7 @@ pub fn build_import_map<'a>( is_type_only, imported_name, is_default_import: false, + is_const_enum: false, }, ); } @@ -671,6 +678,7 @@ pub fn build_import_map<'a>( // binding name. imported_name: Some(Ident::from("default")), is_default_import: true, + is_const_enum: false, }, ); } @@ -696,6 +704,7 @@ pub fn build_import_map<'a>( is_type_only: decl_is_type_only, imported_name: None, is_default_import: false, + is_const_enum: false, }, ); } @@ -3509,6 +3518,18 @@ pub fn transform_angular_file( } } + // A named import used as a constructor parameter type that the other + // file declares as a `const enum`: class metadata writes `type: undefined` + // for it, like ngtsc's full and partial `typeToValue`. Gated like the + // other cross-file reads of `resolve_imported_values`, and only the + // names a constructor parameter type uses are looked up. + #[cfg(feature = "cross_file_elision")] + if options.resolve_imported_values + && let Some(analyzer) = &cross_file_analyzer + { + mark_const_enum_imports(&parser_ret.program, &mut import_map, analyzer, path); + } + // ngtsc's implicit signal `debugName` transform: `signal(0)` and the // other signal APIs get an ngDevMode `debugName` argument named after the // binding they initialize. ngtsc adds it to the `before` emit transforms @@ -5503,6 +5524,50 @@ fn set_component_scope_statement<'a>( ) } +/// Set [`ImportInfo::is_const_enum`] on the named imports a constructor +/// parameter of `program` names as its whole type (`kc: KeyCodes`, after +/// `T | null` narrowing) when `analyzer` finds a `const enum` behind them. +/// Type-only and default imports are left alone: their `type:` is decided +/// without this. +#[cfg(feature = "cross_file_elision")] +fn mark_const_enum_imports<'a>( + program: &Program<'a>, + import_map: &mut ImportMap<'a>, + analyzer: &CrossFileAnalyzer, + path: &str, +) { + let mut checked: rustc_hash::FxHashSet> = rustc_hash::FxHashSet::default(); + for site in crate::util::declared_classes(program) { + let Some(ctor) = crate::util::find_constructor(site.class) else { continue }; + for param in &ctor.value.params.items { + let Some(name) = param + .type_annotation + .as_ref() + .and_then(|annotation| { + crate::class_metadata::param_type_reference(&annotation.type_annotation) + }) + .and_then(|type_ref| crate::class_metadata::entity_path(&type_ref.type_name)) + .and_then(|path| if let [name] = path[..] { Some(name) } else { None }) + else { + continue; + }; + if !checked.insert(name) { + continue; + } + let Some(info) = import_map.get_mut(&name) else { continue }; + if !info.is_named_import || info.is_type_only || info.is_default_import { + continue; + } + let export_name = info.imported_name.unwrap_or(name); + info.is_const_enum = analyzer.is_const_enum_import( + info.source_module.as_str(), + export_name.as_str(), + std::path::Path::new(path), + ); + } + } +} + /// The analyzer behind both cross-file features (`cross_file_elision`'s /// type-only scan and `resolve_imported_values`), resolving modules from /// `path`'s directory (or `base_dir`) with `tsconfig_path`'s aliases. diff --git a/crates/oxc_angular_compiler/tests/const_enum_ctor_param_test.rs b/crates/oxc_angular_compiler/tests/const_enum_ctor_param_test.rs new file mode 100644 index 000000000..6d86633a7 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/const_enum_ctor_param_test.rs @@ -0,0 +1,378 @@ +//! A constructor parameter typed by a `const enum` gets `type: undefined` in +//! the class metadata (full `ɵsetClassMetadata`, partial +//! `ɵɵngDeclareClassMetadata`), like ngtsc's `typeToValue` outside local +//! compilation (issue #623). Each expected output was checked against +//! `@angular/compiler-cli` 22.2.3 in full and partial mode. +//! +//! - same file: `const enum`, `declare const enum`, `ns.E` +//! - imported: only with `resolve_imported_values`, when the other file can +//! be read and shows a `const enum` +//! - anything oxc can't be sure of keeps today's output, and factories don't +//! change + +// The expected output has object literals like `{type:Foo}`. +#![expect(clippy::literal_string_with_formatting_args)] + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{CompilationMode, TransformOptions, transform_angular_file}; + +const MODES: [CompilationMode; 2] = [CompilationMode::Full, CompilationMode::Partial]; + +/// The compiled `source`, without whitespace. +fn compile_at(path: &str, source: &str, options: &TransformOptions) -> String { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, path, source, Some(options), None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + result.code.chars().filter(|c| !c.is_whitespace()).collect() +} + +fn compile(source: &str, compilation_mode: CompilationMode) -> String { + compile_at("test.ts", source, &TransformOptions { compilation_mode, ..Default::default() }) +} + +/// The `ctorParameters` arrow body of the first class metadata in `code`. +fn ctor_params(code: &str) -> &str { + let start = code.find("()=>[{type:").expect("ctor parameters") + "()=>".len(); + let mut depth = 0; + for (i, c) in code[start..].char_indices() { + match c { + '[' | '{' | '(' => depth += 1, + ']' | '}' | ')' => { + depth -= 1; + if depth == 0 { + return &code[start..=start + i]; + } + } + _ => {} + } + } + &code[start..] +} + +fn assert_contains(code: &str, expected: &str) { + assert!(code.contains(expected), "Expected `{expected}` in:\n{code}"); +} + +fn assert_not_contains(code: &str, unexpected: &str) { + assert!(!code.contains(unexpected), "Did not expect `{unexpected}` in:\n{code}"); +} + +#[test] +fn same_file_const_enum_param_has_no_type() { + // ngtsc: `type: undefined` / `type: Plain` / `type: undefined`. + let source = "import { Injectable, Inject } from '@angular/core'; +export const enum KeyCodes { A = 1, B = 2 } +export enum Plain { X = 1 } +@Injectable() +export class Svc { + constructor(@Inject('KC') kc: KeyCodes, @Inject('P') p: Plain, @Inject('N') n: KeyCodes | null) {} +}"; + for mode in MODES { + let code = compile(source, mode); + assert_eq!( + ctor_params(&code), + "[{type:undefined,decorators:[{type:Inject,args:[\"KC\"]}]},\ + {type:Plain,decorators:[{type:Inject,args:[\"P\"]}]},\ + {type:undefined,decorators:[{type:Inject,args:[\"N\"]}]}]", + "{mode:?}" + ); + } +} + +#[test] +fn declare_and_local_const_enum_params_have_no_type() { + // A `declare const enum` is erased: `type: Local` threw a ReferenceError + // when JIT or TestBed read the metadata. ngtsc: `undefined` for both. + let source = "import { Injectable, Inject } from '@angular/core'; +declare const enum Local { A = 1 } +const enum NotExported { A = 1 } +@Injectable() +export class Svc { + constructor(@Inject('KC') kc: Local, @Inject('NE') ne: NotExported) {} +}"; + for mode in MODES { + let code = compile(source, mode); + assert_eq!( + ctor_params(&code), + "[{type:undefined,decorators:[{type:Inject,args:[\"KC\"]}]},\ + {type:undefined,decorators:[{type:Inject,args:[\"NE\"]}]}]", + "{mode:?}" + ); + } +} + +#[test] +fn namespace_const_enum_param_has_no_type() { + // ngtsc: `undefined`, `NS.P`, `undefined`, `undefined`. + let source = "import { Injectable, Inject } from '@angular/core'; +export namespace NS { export const enum E { A = 1 } export enum P { A = 1 } export namespace Deep { export const enum D { A = 1 } } } +declare namespace Amb { const enum E { A = 1 } } +@Injectable() +export class Svc { + constructor(@Inject('E') e: NS.E, @Inject('P') p: NS.P, @Inject('D') d: NS.Deep.D, @Inject('A') a: Amb.E) {} +}"; + for mode in MODES { + let code = compile(source, mode); + assert_eq!( + ctor_params(&code), + "[{type:undefined,decorators:[{type:Inject,args:[\"E\"]}]},\ + {type:NS.P,decorators:[{type:Inject,args:[\"P\"]}]},\ + {type:undefined,decorators:[{type:Inject,args:[\"D\"]}]},\ + {type:undefined,decorators:[{type:Inject,args:[\"A\"]}]}]", + "{mode:?}" + ); + } +} + +#[test] +fn directive_and_component_const_enum_params_have_no_type() { + let source = "import { Component, Directive, Inject, Optional } from '@angular/core'; +const enum KeyCodes { A = 1 } +@Directive({ selector: '[d]' }) +export class Dir { + constructor(@Inject('KC') @Optional() kc: KeyCodes) {} +} +@Component({ selector: 'c', template: '' }) +export class Cmp { + constructor(@Inject('KC') kc: KeyCodes) {} +}"; + for mode in MODES { + let code = compile(source, mode); + assert_contains( + &code, + "()=>[{type:undefined,decorators:[{type:Inject,args:[\"KC\"]},{type:Optional}]}]", + ); + assert_contains(&code, "()=>[{type:undefined,decorators:[{type:Inject,args:[\"KC\"]}]}]"); + assert_not_contains(&code, "type:KeyCodes"); + } +} + +#[test] +fn const_enum_merged_with_a_namespace_keeps_its_type() { + // ngtsc gives `undefined` here too, but one file can't be sure what the + // namespace merges with, so the reference stays. + let source = "import { Injectable, Inject } from '@angular/core'; +const enum M { A = 1 } +namespace M { export type T = 1; } +@Injectable() +export class Svc { constructor(@Inject('M') m: M) {} }"; + for mode in MODES { + let code = compile(source, mode); + assert_eq!( + ctor_params(&code), + "[{type:M,decorators:[{type:Inject,args:[\"M\"]}]}]", + "{mode:?}" + ); + } +} + +#[test] +fn const_enum_factory_dep_is_unchanged() { + // Only the metadata `type:` follows ngtsc; the factory keeps its token + // (ngtsc: `ɵɵinvalidFactory()` / `deps: "invalid"`). + let source = "import { Injectable } from '@angular/core'; +const enum Local { A = 1 } +@Injectable() +export class Svc { constructor(l: Local) {} }"; + let full = compile(source, CompilationMode::Full); + assert_contains(&full, "i0.ɵɵinject(Local)"); + assert_eq!(ctor_params(&full), "[{type:undefined}]"); + let partial = compile(source, CompilationMode::Partial); + assert_contains(&partial, "deps:[{token:Local}]"); + assert_eq!(ctor_params(&partial), "[{type:undefined}]"); +} + +#[cfg(feature = "cross_file_elision")] +mod cross_file { + use super::*; + use tempfile::TempDir; + + fn write(dir: &TempDir, name: &str, content: &str) -> String { + let path = dir.path().join(name); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, content).unwrap(); + path.to_string_lossy().into_owned() + } + + fn options(mode: CompilationMode, resolve_imported_values: bool) -> TransformOptions { + TransformOptions { compilation_mode: mode, resolve_imported_values, ..Default::default() } + } + + const KEYS: &str = "export const enum KeyCodes { A = 1, B = 2 } +export enum Plain { X = 1 } +const enum Local { A = 1 } +export { Local }; +export declare const enum Decl { A = 1 } +"; + + const IMPORTER: &str = "import { Injectable, Inject } from '@angular/core'; +import { KeyCodes, Plain, KeyCodes as KC, Local, Decl } from './keys'; +@Injectable() +export class Svc { + constructor(@Inject('KC') kc: KeyCodes, @Inject('P') p: Plain, @Inject('A') a: KC, @Inject('L') l: Local, @Inject('D') d: Decl) {} +}"; + + #[test] + fn imported_const_enum_param_has_no_type() { + // ngtsc: `undefined`, `i1.Plain`, then `undefined` three times. + let dir = TempDir::new().unwrap(); + write(&dir, "keys.ts", KEYS); + let path = write(&dir, "a.ts", IMPORTER); + for mode in MODES { + let code = compile_at(&path, IMPORTER, &options(mode, true)); + assert_eq!( + ctor_params(&code), + "[{type:undefined,decorators:[{type:Inject,args:[\"KC\"]}]},\ + {type:i1.Plain,decorators:[{type:Inject,args:[\"P\"]}]},\ + {type:undefined,decorators:[{type:Inject,args:[\"A\"]}]},\ + {type:undefined,decorators:[{type:Inject,args:[\"L\"]}]},\ + {type:undefined,decorators:[{type:Inject,args:[\"D\"]}]}]", + "{mode:?}" + ); + } + } + + #[test] + fn imported_const_enum_file_is_a_dependency() { + let dir = TempDir::new().unwrap(); + let keys = write(&dir, "keys.ts", KEYS); + let path = write(&dir, "a.ts", IMPORTER); + let allocator = Allocator::default(); + let result = transform_angular_file( + &allocator, + &path, + IMPORTER, + Some(&options(CompilationMode::Full, true)), + None, + ); + let keys = std::fs::canonicalize(keys).unwrap(); + assert!( + result + .dependencies + .iter() + .any(|dep| std::fs::canonicalize(dep).is_ok_and(|dep| dep == keys)), + "{:?}", + result.dependencies + ); + } + + #[test] + fn imported_const_enum_keeps_its_type_without_the_option() { + // Default output doesn't change, nor does `cross_file_elision` alone. + let dir = TempDir::new().unwrap(); + write(&dir, "keys.ts", KEYS); + let path = write(&dir, "a.ts", IMPORTER); + for mode in MODES { + for options in [ + options(mode, false), + TransformOptions { + compilation_mode: mode, + cross_file_elision: true, + ..Default::default() + }, + ] { + let code = compile_at(&path, IMPORTER, &options); + assert_contains( + ctor_params(&code), + "[{type:i1.KeyCodes,decorators:[{type:Inject,args:[\"KC\"]}]},", + ); + } + } + } + + #[test] + fn only_const_enum_import_adds_no_namespace_import() { + // ngtsc emits no `import * as i1 from "./keys"`. + let dir = TempDir::new().unwrap(); + write(&dir, "keys.ts", "export const enum KeyCodes { A = 1 }\n"); + let source = "import { Injectable, Inject } from '@angular/core'; +import { KeyCodes } from './keys'; +@Injectable() +export class Svc { + constructor(@Inject('KC') kc: KeyCodes) {} +}"; + let path = write(&dir, "a.ts", source); + for mode in MODES { + let code = compile_at(&path, source, &options(mode, true)); + assert_eq!( + ctor_params(&code), + "[{type:undefined,decorators:[{type:Inject,args:[\"KC\"]}]}]", + "{mode:?}" + ); + assert_not_contains(&code, "from\"./keys\""); + assert_not_contains(&code, "from'./keys'"); + } + } + + #[test] + fn re_exported_const_enum_param_has_no_type() { + // ngtsc: `undefined`, `i1.Plain`, `undefined`, `undefined`. + let dir = TempDir::new().unwrap(); + write( + &dir, + "keys.ts", + "export const enum KeyCodes { A = 1 }\nexport enum Plain { X = 1 }\n", + ); + write(&dir, "star.ts", "export const enum Star { A = 1 }\n"); + write( + &dir, + "barrel.ts", + "export { KeyCodes, Plain } from './keys'; +export { KeyCodes as Renamed } from './keys'; +export * from './star'; +", + ); + let source = "import { Injectable, Inject } from '@angular/core'; +import { KeyCodes, Plain, Star, Renamed } from './barrel'; +@Injectable() +export class Svc { + constructor(@Inject('KC') kc: KeyCodes, @Inject('P') p: Plain, @Inject('S') s: Star, @Inject('R') r: Renamed) {} +}"; + let path = write(&dir, "a.ts", source); + for mode in MODES { + let code = compile_at(&path, source, &options(mode, true)); + assert_eq!( + ctor_params(&code), + "[{type:undefined,decorators:[{type:Inject,args:[\"KC\"]}]},\ + {type:i1.Plain,decorators:[{type:Inject,args:[\"P\"]}]},\ + {type:undefined,decorators:[{type:Inject,args:[\"S\"]}]},\ + {type:undefined,decorators:[{type:Inject,args:[\"R\"]}]}]", + "{mode:?}" + ); + } + } + + #[test] + fn unsure_imported_const_enum_keeps_its_type() { + // A namespace import (ngtsc: `undefined`), a file whose bare + // `namespace` block blurs its export names, a `.d.ts`-only module + // and a missing one: oxc can't be sure, so `i{n}.X` stays. + let dir = TempDir::new().unwrap(); + write(&dir, "keys.ts", "export const enum KeyCodes { A = 1 }\n"); + write( + &dir, + "blurred.ts", + "export const enum Blurred { A = 1 }\nnamespace Inner { export class Blurred {} }\n", + ); + write(&dir, "dts.d.ts", "export declare const enum Typed { A = 1 }\n"); + let source = "import { Injectable, Inject } from '@angular/core'; +import * as k from './keys'; +import { Blurred } from './blurred'; +import { Typed } from './dts'; +import { Missing } from './missing'; +@Injectable() +export class Svc { + constructor(@Inject('K') a: k.KeyCodes, @Inject('B') b: Blurred, @Inject('T') t: Typed, @Inject('M') m: Missing) {} +}"; + let path = write(&dir, "a.ts", source); + for mode in MODES { + let code = compile_at(&path, source, &options(mode, true)); + let params = ctor_params(&code); + assert_contains(params, "{type:i1.KeyCodes,"); + assert_contains(params, ".Blurred,"); + assert_contains(params, ".Typed,"); + assert_contains(params, ".Missing,"); + assert_not_contains(params, "type:undefined"); + } + } +} From e53f94c044b5f72c0e1d0892835dbda6d5d2e8f5 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 22:05:56 +0800 Subject: [PATCH 02/18] docs(metadata): say const enums now give `type: undefined` The LocalDecls and local-reference comments still listed const enums as emitted as written. Co-Authored-By: Claude Opus 5.5 --- .../src/class_metadata/builders.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/oxc_angular_compiler/src/class_metadata/builders.rs b/crates/oxc_angular_compiler/src/class_metadata/builders.rs index 078294f09..aefcfe3dd 100644 --- a/crates/oxc_angular_compiler/src/class_metadata/builders.rs +++ b/crates/oxc_angular_compiler/src/class_metadata/builders.rs @@ -1048,10 +1048,11 @@ fn build_param_type_expression<'a>( )); } - // A local reference: `undefined` only when the path ends at an interface - // or type alias — the type-only declarations local `typeToValue` still + // A local reference: `undefined` when the path ends at an interface or + // type alias — the type-only declarations local `typeToValue` still // rejects (`isLocalCompilation` keeps only those three SyntaxKinds - // unavailable). Everything else — const enums, vars, functions, bare + // unavailable) — or at a `const enum`, which full and partial + // `typeToValue` reject too. Everything else — vars, functions, bare // namespace names, missing members, undeclared or global names — emits // as written, matching the upstream `/* @ts-ignore */ type: X` fallback. if let Some(decls) = local_decls @@ -1178,10 +1179,11 @@ pub fn type_param_names_of<'a>(class: &Class<'a>) -> FxHashSet> { /// comparison and single-file transform target — `experimental-local`, which /// matches what OXC can know from one file): only `TypeParameter`, /// `TypeAliasDeclaration`, and `InterfaceDeclaration` are `UNAVAILABLE`; -/// everything else — vars, functions, const enums, bare namespace names, -/// missing members, undeclared names — is emitted as written with an -/// upstream `/* @ts-ignore */` (comparisons ignore the comment, and `Ident` -/// output can't carry one anyway). `type_decls` holds interfaces and type +/// everything else — vars, functions, bare namespace names, missing +/// members, undeclared names — is emitted as written with an upstream +/// `/* @ts-ignore */` (comparisons ignore the comment, and `Ident` output +/// can't carry one anyway). `const_enums` follows full and partial +/// compilation instead (see [`Self::is_const_enum`]). `type_decls` holds interfaces and type /// aliases; `namespaces` records namespace bodies recursively, needed for /// `ns.Interface` lookups and for the merged `interface I` + `namespace I` /// case — the merged symbol's valueDeclaration is the namespace, so it emits. From bd099f5135c965f0715593f0e2c4b1f4f460c8fa Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 22:12:21 +0800 Subject: [PATCH 03/18] fix(ngmodule): leave directives and pipes out of the injector imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In full and partial compilation ngtsc builds the ɵinj `imports` from what each reference resolves to (NgModuleDecoratorHandler.resolve): it drops the directives and pipes of `imports`, and the directives, components and pipes of `exports`. Oxc copied ngtsc's local compilation and added both lists as written. The runtime's walkProviderTree skips those entries, so dropping them does not change which providers are registered. An entry is dropped only when Oxc knows what it is: - from this file: an `exports` entry that `declarations` also lists, a class of the file with an Angular @Directive or @Pipe decorator (aliases and namespace imports of the decorator count), and an exported @Component that `imports` or `declarations` also lists; - with cross-file analysis (resolveImportedValues): imported classes, `const` lists and package .d.ts types, read through the NgModule scope code. A list is dropped only when all its classes drop, and an exported component only when it is in the module's compilation scope. Anything that does not resolve, an NgModule, a forRoot() call, and a standalone component in `imports` stay. Classes nested in a function keep their lists. The files read become watch dependencies like other cross-file reads. Not done: ngtsc also drops a standalone component from `imports` when it brings in no providers, and rewrites a partly dropped list into the references it keeps. Co-Authored-By: Claude Opus 5.5 --- .../cross_file_elision/ng_module_scope.rs | 79 +++ .../src/component/transform.rs | 25 + .../src/ng_module/definition.rs | 9 +- .../src/ng_module/injector_imports.rs | 311 +++++++++++ .../oxc_angular_compiler/src/ng_module/mod.rs | 4 + .../tests/ng_module_injector_imports_test.rs | 483 ++++++++++++++++++ 6 files changed, 907 insertions(+), 4 deletions(-) create mode 100644 crates/oxc_angular_compiler/src/ng_module/injector_imports.rs create mode 100644 crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs diff --git a/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs b/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs index 26dd6a0c6..969cfded7 100644 --- a/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs +++ b/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs @@ -11,6 +11,9 @@ //! //! Anything that can't be read statically gives `None`, and the caller emits //! what it did before. +//! +//! The same reads tell which entries of an NgModule's `imports` and `exports` +//! are directives or pipes, which its `ɵinj` leaves out (#623). use std::cell::{OnceCell, RefCell}; use std::path::Path; @@ -27,6 +30,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; use super::{CrossFileAnalyzer, Inner, MAX_EXPORT_CHAIN}; use crate::component::metadata::{ComponentScope, ImportedName, ScopeEntry, ScopeKind}; use crate::directive::{Evaluator, StringConsts, find_angular_class_decorator}; +use crate::ng_module::{InjectorDrops, InjectorEntry}; /// A top-level declaration: the file declaring it and its name there /// (`"default"` for an anonymous `export default class`). @@ -277,6 +281,56 @@ impl CrossFileAnalyzer { }) .collect() } + + /// Which top-level `imports` and `exports` entries of the `@NgModule` + /// class `class_name` of `file` its `ɵinj` leaves out (#623), as ngtsc's + /// `NgModuleDecoratorHandler.resolve` does: an entry whose classes + /// (a `const` list flattened) are all directives or pipes, and in + /// `exports` also components. An exported component must be in the + /// module's compilation scope, so it stays reachable through `imports`. + /// An entry that doesn't resolve, or names an NgModule, stays. + pub(crate) fn injector_drops( + &self, + file: &Path, + source: &str, + class_name: &str, + imports: &[Option>], + exports: &[Option>], + ) -> InjectorDrops { + let file = self.inner.set_compiled_source(file, source); + let inner = &self.inner; + let module = Decl { file: file.clone(), name: class_name.into() }; + let scope = OnceCell::new(); + let drops = |entries: &[Option>], is_exports: bool| -> Vec { + entries + .iter() + .map(|entry| { + let Some(entry) = entry else { return false }; + let raw = RawRef { + file: file.clone(), + namespace: entry.namespace.map(Into::into), + name: entry.name.into(), + method: None, + }; + let Some(classes) = inner.resolve_refs(&[raw], None, 0) else { return false }; + !classes.is_empty() + && classes.iter().all(|(class, declared)| match &**declared { + Declared::Directive { is_component: false, .. } + | Declared::Pipe { .. } => true, + Declared::Directive { is_component: true, .. } => { + is_exports + && scope + .get_or_init(|| inner.compilation_scope(&module)) + .as_ref() + .is_some_and(|scope| scope.contains(&class.decl)) + } + Declared::NgModule { .. } | Declared::List(_) => false, + }) + }) + .collect() + }; + InjectorDrops { imports: drops(imports, false), exports: drops(exports, true) } + } } /// `items` with each class once, where it first appears. @@ -560,6 +614,31 @@ impl Inner { Some(out) } + /// The classes of the compilation scope of the NgModule `module`: its + /// declarations, the standalone classes it imports, and what the + /// NgModules it imports export (ngtsc's `LocalModuleScopeRegistry`). + fn compilation_scope(&self, module: &Decl) -> Option> { + let declared = self.declared(module)?; + let Declared::NgModule { declarations, imports, .. } = &*declared else { return None }; + let mut scope = FxHashSet::default(); + for (class, _) in self.resolve_refs(declarations, None, 0)? { + scope.insert(class.decl); + } + for (class, declared) in self.resolve_refs(imports, None, 0)? { + match &*declared { + Declared::NgModule { exports, .. } => scope.extend( + self.exported_scope(&class, exports)? + .iter() + .map(|item| item.class.decl.clone()), + ), + _ => { + scope.insert(class.decl); + } + } + } + Some(scope) + } + /// The directives and pipes the NgModule `module` exports, with those of /// the modules it exports. fn exported_scope(&self, module: &ClassRef, exports: &[RawRef]) -> Option> { diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index d99082156..f6b005ae0 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -4721,6 +4721,31 @@ pub fn transform_angular_file( ); } + // Like ngtsc in full and partial compilation, ɵinj leaves + // out the directives and pipes of `imports` and `exports` + // (#623). A nested class may name a binding of its + // function, so it keeps the lists as written. + if !site.nested + && let Some(entries) = + crate::ng_module::injector_entries(class, &string_consts) + { + #[cfg_attr(not(feature = "cross_file_elision"), allow(unused_mut))] + let mut drops = entries.local_drops(&parser_ret.program, &string_consts); + #[cfg(feature = "cross_file_elision")] + if options.resolve_imported_values + && let Some(analyzer) = &cross_file_analyzer + { + drops.merge(&analyzer.injector_drops( + std::path::Path::new(path), + source, + ng_module_metadata.class_name.as_str(), + entries.imports(), + entries.exports(), + )); + } + entries.apply(allocator, &mut ng_module_metadata, &drops); + } + // Compile NgModule and generate all definitions as external property assignments if let Some(definition) = generate_full_ng_module_definition( &allocator, diff --git a/crates/oxc_angular_compiler/src/ng_module/definition.rs b/crates/oxc_angular_compiler/src/ng_module/definition.rs index 175f0e7b1..a3b0665ef 100644 --- a/crates/oxc_angular_compiler/src/ng_module/definition.rs +++ b/crates/oxc_angular_compiler/src/ng_module/definition.rs @@ -339,10 +339,11 @@ fn build_injector_metadata<'a>( // Like ngtsc's local compilation (ng_module/src/handler.ts `analyze`), the // injector imports are the raw `imports` followed by the raw `exports`: an // array literal adds its elements (keeping `StoreModule.forRoot(...)` calls - // and spreads), any other expression is added as is. A full ngtsc build - // filters both lists by what each reference resolves to, which needs other - // files; the runtime ignores entries that are neither NgModules nor - // standalone components. + // and spreads), any other expression is added as is. A full or partial + // ngtsc build filters both lists by what each reference resolves to; the + // transform drops the entries it knows to be directives or pipes from + // these expressions first (`injector_imports.rs`). The runtime ignores + // entries that are neither NgModules nor standalone components. for (raw, names) in [ (&metadata.raw_imports_expr, &metadata.imports), (&metadata.raw_exports_expr, &metadata.exports), diff --git a/crates/oxc_angular_compiler/src/ng_module/injector_imports.rs b/crates/oxc_angular_compiler/src/ng_module/injector_imports.rs new file mode 100644 index 000000000..e7e3b3c47 --- /dev/null +++ b/crates/oxc_angular_compiler/src/ng_module/injector_imports.rs @@ -0,0 +1,311 @@ +//! Which `imports` and `exports` entries of an NgModule go into its `ɵinj`. +//! +//! ngtsc's `NgModuleDecoratorHandler.resolve` (compiler-cli +//! `ngtsc/annotations/ng_module/src/handler.ts`) builds the injector's +//! `imports` in full and partial compilation from what each reference +//! resolves to: it leaves out every directive and pipe of `imports`, and +//! every directive, component and pipe of `exports` (`isNgModule`). Only +//! local compilation adds the lists as written, which is what Oxc did. +//! +//! The runtime's `walkProviderTree` skips anything that is neither an +//! NgModule nor a standalone component, so leaving those entries out never +//! changes which providers are registered. Here an entry is left out only +//! when it is known to be one of them; anything unknown stays. +//! +//! Two sources say what an entry is: +//! - this file alone ([`InjectorEntries::local_drops`]): an `exports` entry +//! that `declarations` also lists, and a class of this file with an +//! Angular `@Directive`, `@Pipe` or `@Component` decorator; +//! - cross-file analysis, when it is on, which reads imported classes and +//! `const` lists too (`CrossFileAnalyzer::injector_drops`). +//! +//! Left out on purpose: ngtsc also drops a standalone component from +//! `imports` when it brings in no providers (`mayExportProviders`), and +//! rewrites a list that is only partly dropped into the references it keeps. +//! Oxc keeps such entries whole. + +use oxc_allocator::{Allocator, Box, Vec as OxcVec}; +use oxc_ast::ast::{ + ArrayExpressionElement, Class, Declaration, ExportDefaultDeclarationKind, Expression, + ObjectPropertyKind, Program, PropertyKey, Statement, +}; + +use super::decorator::{NgModuleMetadata, find_ng_module_decorator}; +use crate::directive::{StringConsts, find_angular_class_decorator}; +use crate::output::ast::{LiteralArrayExpr, OutputExpression}; +use crate::util::is_metadata_property; + +/// A top-level entry of `imports`, `exports` or `declarations` that names a +/// class or a `const` list: `A`, `ns.A`, `forwardRef(() => A)`, `...LIST`, +/// or the whole value (`imports: LIST`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct InjectorEntry<'s> { + pub namespace: Option<&'s str>, + pub name: &'s str, +} + +/// Per top-level entry of `imports` and of `exports`, whether `ɵinj` leaves +/// it out. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct InjectorDrops { + pub imports: Vec, + pub exports: Vec, +} + +impl InjectorDrops { + /// Leave out what `other` leaves out too. + #[cfg_attr(not(feature = "cross_file_elision"), allow(dead_code))] + pub(crate) fn merge(&mut self, other: &Self) { + for (mine, theirs) in + [(&mut self.imports, &other.imports), (&mut self.exports, &other.exports)] + { + for (drop, other_drop) in mine.iter_mut().zip(theirs) { + *drop |= *other_drop; + } + } + } +} + +/// One of the `imports` / `exports` / `declarations` lists, split the way +/// ngtsc's `analyze` splits `imports`: each array element (a spread by its +/// argument), or the whole value when it is not an array literal. +#[derive(Debug, Default)] +struct List<'s> { + /// `None` for an entry that names no class or list (a call, a literal...). + entries: Vec>>, + is_array: bool, +} + +/// The top-level entries of an `@NgModule`'s lists. +#[derive(Debug)] +pub struct InjectorEntries<'s> { + imports: List<'s>, + exports: List<'s>, + declarations: List<'s>, +} + +/// The entries of the Angular `@NgModule` decorator of `class`, or `None` +/// when it has none or its argument is not an object literal. +pub fn injector_entries<'s>( + class: &'s Class<'_>, + consts: &StringConsts<'_>, +) -> Option> { + let decorator = find_ng_module_decorator(&class.decorators, Some(consts))?; + let Expression::CallExpression(call) = &decorator.expression else { return None }; + let Expression::ObjectExpression(object) = call.arguments.first()?.as_expression()? else { + return None; + }; + let mut entries = InjectorEntries { + imports: List::default(), + exports: List::default(), + declarations: List::default(), + }; + // Like `extract_ng_module_metadata_in`, a later property wins. + for property in &object.properties { + let ObjectPropertyKind::ObjectProperty(property) = property else { continue }; + if !is_metadata_property(property) { + continue; + } + let key = match &property.key { + PropertyKey::StaticIdentifier(id) => id.name.as_str(), + PropertyKey::StringLiteral(lit) => lit.value.as_str().unwrap_or_default(), + _ => continue, + }; + let list = match key { + "imports" => &mut entries.imports, + "exports" => &mut entries.exports, + "declarations" => &mut entries.declarations, + _ => continue, + }; + *list = read_list(&property.value, consts); + } + Some(entries) +} + +fn read_list<'s>(value: &'s Expression<'_>, consts: &StringConsts<'_>) -> List<'s> { + let Expression::ArrayExpression(array) = value else { + return List { entries: vec![entry_ref(value, consts)], is_array: false }; + }; + let entries = array + .elements + .iter() + .map(|element| match element { + ArrayExpressionElement::SpreadElement(spread) => entry_ref(&spread.argument, consts), + ArrayExpressionElement::Elision(_) => None, + _ => entry_ref(element.to_expression(), consts), + }) + .collect(); + List { entries, is_array: true } +} + +/// The class or list `expr` names: `A`, `ns.A`, or either one in a +/// `forwardRef` arrow imported from `@angular/core` (the evaluator resolves +/// it to the class, as ngtsc's `forwardRefResolver` does). +fn entry_ref<'s>(expr: &'s Expression<'_>, consts: &StringConsts<'_>) -> Option> { + match expr { + Expression::Identifier(id) => { + Some(InjectorEntry { namespace: None, name: id.name.as_str() }) + } + Expression::StaticMemberExpression(member) => match &member.object { + Expression::Identifier(ns) => Some(InjectorEntry { + namespace: Some(ns.name.as_str()), + name: member.property.name.as_str(), + }), + _ => None, + }, + Expression::CallExpression(call) => { + let Expression::Identifier(callee) = &call.callee else { return None }; + crate::directive::angular_core_identifier(&callee.name, Some(consts), &["forwardRef"])?; + let Some(Expression::ArrowFunctionExpression(arrow)) = + call.arguments.first().and_then(|arg| arg.as_expression()) + else { + return None; + }; + match arrow.get_expression()? { + inner @ (Expression::Identifier(_) | Expression::StaticMemberExpression(_)) => { + entry_ref(inner, consts) + } + _ => None, + } + } + _ => None, + } +} + +/// What a class of this file is to an NgModule's lists, by its Angular +/// decorator. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum LocalKind { + DirectiveOrPipe, + Component, +} + +/// The kind of the top-level class `program` declares as `name`, when it has +/// exactly one Angular `@Directive`, `@Pipe` or `@Component` decorator and +/// no `@NgModule` one. A name the file imports is never a class it declares. +fn local_kind(program: &Program<'_>, consts: &StringConsts<'_>, name: &str) -> Option { + let class = program.body.iter().find_map(|stmt| { + let class = match stmt { + Statement::ClassDeclaration(class) => class, + Statement::ExportDeclaration(decl) => match &decl.declaration { + Declaration::ClassDeclaration(class) => class, + _ => return None, + }, + Statement::ExportDefaultDeclaration(decl) => match &decl.declaration { + ExportDefaultDeclarationKind::ClassDeclaration(class) => class, + _ => return None, + }, + _ => return None, + }; + class.id.as_ref().is_some_and(|id| id.name == name).then_some(class) + })?; + let has = |kind: &str| find_angular_class_decorator(&class.decorators, kind, consts).is_some(); + if has("NgModule") { + return None; + } + match (has("Directive"), has("Pipe"), has("Component")) { + (true, false, false) | (false, true, false) => Some(LocalKind::DirectiveOrPipe), + (false, false, true) => Some(LocalKind::Component), + _ => None, + } +} + +impl<'s> InjectorEntries<'s> { + /// The top-level `imports` entries, in order. + #[cfg_attr(not(feature = "cross_file_elision"), allow(dead_code))] + pub(crate) fn imports(&self) -> &[Option>] { + &self.imports.entries + } + + /// The top-level `exports` entries, in order. + #[cfg_attr(not(feature = "cross_file_elision"), allow(dead_code))] + pub(crate) fn exports(&self) -> &[Option>] { + &self.exports.entries + } + + /// What this file alone shows `ɵinj` leaves out: + /// - an `exports` entry `declarations` also lists: only directives, + /// components and pipes can be declared; + /// - an entry naming a class of this file with a `@Directive` or `@Pipe` + /// decorator; + /// - an `exports` entry naming a class of this file with a `@Component` + /// decorator that `imports` (where it stays) or `declarations` also + /// lists, as ngtsc requires of an exported class. + pub(crate) fn local_drops( + &self, + program: &Program<'_>, + consts: &StringConsts<'_>, + ) -> InjectorDrops { + let listed_in = |list: &List<'s>, entry: &InjectorEntry<'s>| { + list.entries.iter().flatten().any(|other| other == entry) + }; + let kind = |entry: &InjectorEntry<'s>| match entry.namespace { + None => local_kind(program, consts, entry.name), + Some(_) => None, + }; + let imports = self + .imports + .entries + .iter() + .map(|entry| { + entry.is_some_and(|entry| kind(&entry) == Some(LocalKind::DirectiveOrPipe)) + }) + .collect(); + let exports = self + .exports + .entries + .iter() + .map(|entry| { + entry.is_some_and(|entry| { + listed_in(&self.declarations, &entry) + || match kind(&entry) { + Some(LocalKind::DirectiveOrPipe) => true, + Some(LocalKind::Component) => listed_in(&self.imports, &entry), + None => false, + } + }) + }) + .collect(); + InjectorDrops { imports, exports } + } + + /// Leave the entries `drops` marks out of the `ɵinj` imports `metadata` + /// builds. A list whose converted expression doesn't line up with the + /// entries read here is kept as is. + pub(crate) fn apply<'a>( + &self, + allocator: &'a Allocator, + metadata: &mut NgModuleMetadata<'a>, + drops: &InjectorDrops, + ) { + apply_list(allocator, &mut metadata.raw_imports_expr, &self.imports, &drops.imports); + apply_list(allocator, &mut metadata.raw_exports_expr, &self.exports, &drops.exports); + } +} + +fn apply_list<'a>( + allocator: &'a Allocator, + raw: &mut Option>, + list: &List<'_>, + drops: &[bool], +) { + if drops.len() != list.entries.len() || !drops.contains(&true) { + return; + } + match raw { + Some(OutputExpression::LiteralArray(array)) + if list.is_array && array.entries.len() == drops.len() => + { + let mut drop = drops.iter(); + array.entries.retain(|_| drop.next() == Some(&false)); + } + // `imports: LIST`, all of whose classes are left out. + Some(expr) if !list.is_array && !matches!(expr, OutputExpression::LiteralArray(_)) => { + *expr = OutputExpression::LiteralArray(Box::new_in( + LiteralArrayExpr { entries: OxcVec::new_in(&allocator), source_span: None }, + &allocator, + )); + } + _ => {} + } +} diff --git a/crates/oxc_angular_compiler/src/ng_module/mod.rs b/crates/oxc_angular_compiler/src/ng_module/mod.rs index 940f378af..595d088dd 100644 --- a/crates/oxc_angular_compiler/src/ng_module/mod.rs +++ b/crates/oxc_angular_compiler/src/ng_module/mod.rs @@ -14,6 +14,7 @@ mod compiler; mod decorator; mod definition; +mod injector_imports; mod metadata; pub use compiler::{NgModuleCompileResult, compile_ng_module, compile_ng_module_from_metadata}; @@ -24,6 +25,9 @@ pub use definition::{ emit_ng_module_definition, generate_full_ng_module_definition, generate_ng_module_definition, generate_ng_module_definition_from_decorator, }; +pub(crate) use injector_imports::injector_entries; +#[cfg(feature = "cross_file_elision")] +pub(crate) use injector_imports::{InjectorDrops, InjectorEntry}; pub use metadata::{ R3NgModuleMetadata, R3NgModuleMetadataBuilder, R3Reference, R3SelectorScopeMode, }; diff --git a/crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs b/crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs new file mode 100644 index 000000000..69ccb86ad --- /dev/null +++ b/crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs @@ -0,0 +1,483 @@ +//! `ɵinj` imports of an NgModule (#623). In full and partial compilation, +//! ngtsc's `NgModuleDecoratorHandler.resolve` leaves the directives and pipes +//! of `imports` out of the injector, and of `exports` also the components: +//! the runtime's `walkProviderTree` skips them. Oxc does the same when it +//! knows what an entry is: from this file alone, or, with cross-file +//! analysis (`TransformOptions::resolve_imported_values`), from the files and +//! packages it imports. Anything else stays as written. +//! +//! Each expected output was checked against ngtsc 22.2.3. Where Oxc still +//! differs, the test says so: ngtsc also drops a standalone component from +//! `imports` when it brings in no providers (`mayExportProviders`), and +//! rewrites a list that is only partly dropped into the references it keeps. + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{CompilationMode, TransformOptions, transform_angular_file}; + +fn options(mode: CompilationMode) -> TransformOptions { + TransformOptions { compilation_mode: mode, ..Default::default() } +} + +/// Each `ɵinj` initializer of `code`, whitespace removed, in order, with +/// the partial declaration's `version` left out. +fn injectors(code: &str) -> Vec { + let compact: String = code.chars().filter(|c| !c.is_whitespace()).collect(); + compact + .split("ɵinj=") + .skip(1) + .map(|rest| { + let init = &rest[..rest.find(';').unwrap()]; + match init.find("version:\"") { + Some(start) => { + let end = start + 9 + init[start + 9..].find('"').unwrap() + 2; + format!("{}{}", &init[..start], &init[end..]) + } + None => init.to_string(), + } + }) + .collect() +} + +fn compile_injectors(path: &str, source: &str, options: &TransformOptions) -> Vec { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, path, source, Some(options), None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + injectors(&result.code) +} + +/// The full and the partial `ɵinj` of a module, from its `imports` list +/// (`None` for no `imports` key). +fn both(name: &str, imports: Option<&str>) -> [String; 2] { + let full = match imports { + Some(list) => format!("/*@__PURE__*/i0.ɵɵdefineInjector({{imports:[{list}]}})"), + None => "/*@__PURE__*/i0.ɵɵdefineInjector({})".to_string(), + }; + let head = format!("i0.ɵɵngDeclareInjector({{minVersion:\"12.0.0\",ngImport:i0,type:{name}"); + let partial = match imports { + Some(list) => format!("{head},imports:[{list}]}})"), + None => format!("{head}}})"), + }; + [full, partial] +} + +/// Check the modules of `source`, compiled as `path`, in full and partial +/// compilation; `expected` is `(module, imports)` per module, in order. +fn assert_injectors_at( + path: &str, + source: &str, + expected: &[(&str, Option<&str>)], + base: &TransformOptions, +) { + for (index, mode) in [CompilationMode::Full, CompilationMode::Partial].into_iter().enumerate() { + let options = TransformOptions { compilation_mode: mode, ..base.clone() }; + let got = compile_injectors(path, source, &options); + let want: Vec = + expected.iter().map(|(name, imports)| both(name, *imports)[index].clone()).collect(); + assert_eq!(got, want, "{mode:?}"); + } +} + +fn assert_injectors(source: &str, expected: &[(&str, Option<&str>)], base: &TransformOptions) { + assert_injectors_at("app.ts", source, expected, base); +} + +#[test] +fn exports_that_are_also_declared_are_left_out() { + // ngtsc: `ɵɵdefineInjector({})`, and no `imports` in partial. + assert_injectors( + r" +import { Directive, NgModule } from '@angular/core'; +import { Other } from './other'; +@Directive({ selector: '[ndir]', standalone: false }) export class NonStdDir {} +@NgModule({ declarations: [NonStdDir, Other], exports: [NonStdDir, Other] }) export class DirModule {} +", + &[("DirModule", None)], + &TransformOptions::default(), + ); +} + +#[test] +fn directives_and_pipes_of_this_file_are_left_out() { + // ngtsc gives `{}` for both: it also drops `LocalCmp`, a standalone + // component that brings in no providers. Oxc keeps components in + // `imports`. + assert_injectors( + r" +import { Directive, Pipe, NgModule, Component } from '@angular/core'; +@Directive({ selector: '[sdir]' }) export class StandaloneDir {} +@Pipe({ name: 'spipe' }) export class StandalonePipe { transform(v: any) { return v; } } +@Component({ selector: 'lc', template: '' }) export class LocalCmp {} +@NgModule({ imports: [StandaloneDir, StandalonePipe], exports: [StandaloneDir, StandalonePipe] }) export class ReexportModule {} +@NgModule({ imports: [StandaloneDir, StandalonePipe, LocalCmp], exports: [StandaloneDir] }) export class LocalModule {} +", + &[("ReexportModule", None), ("LocalModule", Some("LocalCmp"))], + &TransformOptions::default(), + ); +} + +#[test] +fn forward_refs_are_read_through() { + // ngtsc: `{ imports: [forwardRef(() => LateModule)] }` and `{}`. + assert_injectors( + r" +import { Directive, NgModule, forwardRef } from '@angular/core'; +@Directive({ selector: '[decl]', standalone: false }) export class DeclDir {} +@NgModule({ + imports: [forwardRef(() => LateDir), forwardRef(() => LateModule)], + declarations: [DeclDir], + exports: [forwardRef(() => DeclDir)], +}) +export class FwdModule {} +@Directive({ selector: '[late]' }) export class LateDir {} +@NgModule({}) export class LateModule {} +", + &[("FwdModule", Some("forwardRef(()=>LateModule)")), ("LateModule", None)], + &TransformOptions::default(), + ); +} + +#[test] +fn aliased_and_namespaced_decorators_are_recognized() { + // ngtsc: `{}` for LateModule, `{ imports: [LateModule, LateModule] }`. + assert_injectors( + r" +import { Directive as D, NgModule } from '@angular/core'; +import * as ng from '@angular/core'; +@D({ selector: '[alias]' }) export class AliasDir {} +@ng.Pipe({ name: 'nsp' }) export class NsPipe { transform(v: any) { return v; } } +@NgModule({}) export class LateModule {} +@NgModule({ imports: [AliasDir, NsPipe, LateModule], exports: [AliasDir, NsPipe, LateModule] }) +export class AliasModule {} +", + &[("LateModule", None), ("AliasModule", Some("LateModule,LateModule"))], + &TransformOptions::default(), + ); +} + +#[test] +fn an_exported_component_stays_unless_imported_or_declared() { + // ngtsc drops `Cmp` from `exports` and, as it has no providers, from + // `imports` too. A component exported without being imported or declared + // is an error in ngtsc; Oxc keeps it, as its providers may be needed. + assert_injectors( + r" +import { Component, NgModule } from '@angular/core'; +@Component({ selector: 'c', template: '' }) export class Cmp {} +@Component({ selector: 'o', template: '' }) export class Orphan {} +@NgModule({ imports: [Cmp], exports: [Cmp, Orphan] }) export class M {} +", + &[("M", Some("Cmp,Orphan"))], + &TransformOptions::default(), + ); +} + +#[test] +fn what_is_not_known_to_be_a_directive_or_pipe_stays() { + // A decorator not imported from `@angular/core`, an imported class (no + // cross-file analysis), a `forRoot()` call, a const list, a spread and + // a `forwardRef` that is not Angular's all stay as written. + assert_injectors( + r" +import { NgModule } from '@angular/core'; +import { Directive } from './my-decorators'; +import { forwardRef } from './my-forward-ref'; +import { ImportedDir, RouterModule, LIST } from './lib'; +@Directive({ selector: '[x]' }) export class NotAngular {} +@NgModule({ + imports: [NotAngular, ImportedDir, RouterModule.forRoot([]), LIST, ...LIST, forwardRef(() => NotAngular)], + exports: [ImportedDir, Unknown], +}) +export class M {} +", + &[( + "M", + Some( + "NotAngular,ImportedDir,RouterModule.forRoot([]),LIST,...LIST,forwardRef(()=>NotAngular),ImportedDir,Unknown", + ), + )], + &TransformOptions::default(), + ); +} + +#[test] +fn a_module_in_a_function_keeps_its_lists() { + // A nested class could name a binding of its function, which the file's + // top-level classes don't show, so its lists stay as written. + let got = compile_injectors( + "app.ts", + r" +import { Directive, NgModule } from '@angular/core'; +@Directive({ selector: '[d]' }) export class D {} +export function make() { + @NgModule({ imports: [D] }) class Inner {} + return Inner; +} +", + &options(CompilationMode::Full), + ); + assert_eq!(got, vec!["/*@__PURE__*/i0.ɵɵdefineInjector({imports:[D]})".to_string()]); +} + +#[cfg(feature = "cross_file_elision")] +mod cross_file { + //! With cross-file analysis, imported classes, `const` lists and package + //! declaration files are read too. + + use oxc_allocator::Allocator; + use oxc_angular_compiler::{TransformOptions, transform_angular_file}; + use tempfile::TempDir; + + use super::{assert_injectors_at, injectors}; + + fn write(dir: &TempDir, name: &str, content: &str) -> std::path::PathBuf { + let path = dir.path().join(name); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, content).unwrap(); + path + } + + fn cross_file() -> TransformOptions { + TransformOptions { resolve_imported_values: true, ..Default::default() } + } + + /// Compile `src/app.ts` in `dir` with `options` and check its modules. + fn check( + dir: &TempDir, + source: &str, + expected: &[(&str, Option<&str>)], + options: &TransformOptions, + ) { + let path = write(dir, "src/app.ts", source); + assert_injectors_at(path.to_str().unwrap(), source, expected, options); + } + + /// `src/dep.ts` of the ngtsc case: standalone classes and two modules. + fn dep(dir: &TempDir) { + write( + dir, + "src/dep.ts", + r" +import { Directive, Pipe, NgModule, Component } from '@angular/core'; +@Directive({ selector: '[sdir]' }) export class StandaloneDir {} +@Pipe({ name: 'spipe' }) export class StandalonePipe { transform(v: any) { return v; } } +@Component({ selector: 'scmp', template: '' }) export class StandaloneCmp {} +@Component({ selector: 'ncmp', template: '', standalone: false }) export class NonStdCmp {} +@Directive({ selector: '[ndir]', standalone: false }) export class NonStdDir {} +@NgModule({ declarations: [NonStdDir], exports: [NonStdDir] }) export class DirModule {} +@NgModule({ imports: [StandaloneDir, StandalonePipe], exports: [StandaloneDir, StandalonePipe] }) export class ReexportModule {} +@NgModule({ declarations: [NonStdCmp], exports: [NonStdCmp] }) export class CmpModule {} +@NgModule({}) export class PlainModule {} +export const DIRS = [StandaloneDir, StandalonePipe]; +export const MIXED = [StandaloneDir, PlainModule]; +", + ); + } + + /// `node_modules/@lib/common`: declaration files shaped like + /// `@angular/common`'s. + fn common_package(dir: &TempDir) { + write( + dir, + "node_modules/@lib/common/package.json", + r#"{ "name": "@lib/common", "exports": { ".": { "types": "./types/common.d.ts", "default": "./fesm/common.mjs" } } }"#, + ); + write(dir, "node_modules/@lib/common/fesm/common.mjs", "export {};\n"); + write( + dir, + "node_modules/@lib/common/types/common.d.ts", + "export { NgIf, AsyncPipe, NgClass, CommonModule } from './_chunk.js';\n", + ); + write( + dir, + "node_modules/@lib/common/types/_chunk.d.ts", + r#" +import * as i0 from '@angular/core'; +declare class NgIf { + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +declare class NgClass { + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +declare class AsyncPipe { + static ɵpipe: i0.ɵɵPipeDeclaration; +} +declare class CommonModule { + static ɵmod: i0.ɵɵNgModuleDeclaration; +} +export { NgIf, NgClass, AsyncPipe, CommonModule }; +"#, + ); + } + + #[test] + fn imported_directives_and_pipes_are_left_out() { + // ngtsc: `[DirModule, ReexportModule, LocalModule, DirModule, + // ReexportModule, LocalModule]`: it also drops `StandaloneCmp`, + // which brings in no providers. + let dir = TempDir::new().unwrap(); + dep(&dir); + check( + &dir, + r" +import { Directive, Pipe, NgModule } from '@angular/core'; +import { StandaloneDir, StandalonePipe, StandaloneCmp, DirModule, ReexportModule } from './dep'; +@Directive({ selector: '[localdir]' }) export class LocalDir {} +@Pipe({ name: 'lp' }) export class LocalPipe { transform(v: any) { return v; } } +@NgModule({ imports: [LocalDir] }) export class LocalModule {} +@NgModule({ + imports: [StandaloneDir, StandalonePipe, StandaloneCmp, DirModule, ReexportModule, LocalDir, LocalModule, LocalPipe], + exports: [StandaloneDir, DirModule, ReexportModule, LocalModule, LocalPipe], +}) +export class AppModule {} +", + &[ + ("LocalModule", None), + ( + "AppModule", + Some( + "StandaloneCmp,DirModule,ReexportModule,LocalModule,DirModule,ReexportModule,LocalModule", + ), + ), + ], + &cross_file(), + ); + } + + #[test] + fn namespaces_lists_and_reexported_components() { + // ngtsc, module by module: `[dep.PlainModule]`, `{}`, + // `[i1.PlainModule, i1.PlainModule]` (Oxc keeps a list that is only + // partly dropped whole), and `[CmpModule, CmpModule]` (it also drops + // `StandaloneCmp`, which brings in no providers). + let dir = TempDir::new().unwrap(); + dep(&dir); + check( + &dir, + r" +import { NgModule } from '@angular/core'; +import * as dep from './dep'; +import { DIRS, MIXED, StandaloneCmp, NonStdCmp, CmpModule } from './dep'; +@NgModule({ imports: [dep.StandaloneDir, dep.PlainModule, ...DIRS] }) +export class NsModule {} +@NgModule({ imports: DIRS, exports: DIRS }) +export class WholeListModule {} +@NgModule({ imports: [...MIXED], exports: MIXED }) +export class MixedModule {} +@NgModule({ imports: [StandaloneCmp, CmpModule], exports: [StandaloneCmp, NonStdCmp, CmpModule] }) +export class CmpReexportModule {} +", + &[ + ("NsModule", Some("dep.PlainModule")), + ("WholeListModule", None), + ("MixedModule", Some("...MIXED,MIXED")), + ("CmpReexportModule", Some("StandaloneCmp,CmpModule,CmpModule")), + ], + &cross_file(), + ); + } + + #[test] + fn aliased_imports_and_forward_refs() { + // ngtsc: `[ReexportModule, DirModule]`. + let dir = TempDir::new().unwrap(); + dep(&dir); + check( + &dir, + r" +import { NgModule, forwardRef } from '@angular/core'; +import { StandaloneDir as SD, StandalonePipe, DirModule, ReexportModule } from './dep'; +@NgModule({ imports: [SD, forwardRef(() => StandalonePipe), ReexportModule], exports: [forwardRef(() => SD), DirModule] }) +export class AliasModule {} +", + &[("AliasModule", Some("ReexportModule,DirModule"))], + &cross_file(), + ); + } + + #[test] + fn package_declaration_files_are_read() { + // ngtsc, with `@angular/common`: `[CommonModule, CommonModule]`. + let dir = TempDir::new().unwrap(); + common_package(&dir); + check( + &dir, + r" +import { NgModule } from '@angular/core'; +import { NgIf, AsyncPipe, CommonModule, NgClass } from '@lib/common'; +@NgModule({ imports: [NgIf, AsyncPipe, CommonModule], exports: [NgIf, CommonModule, NgClass] }) +export class PkgModule {} +", + &[("PkgModule", Some("CommonModule,CommonModule"))], + &cross_file(), + ); + } + + #[test] + fn unresolved_imports_stay() { + // A missing file, an `export *` that doesn't have the name, and an + // exported component that isn't in the module's scope. + let dir = TempDir::new().unwrap(); + dep(&dir); + write(&dir, "src/barrel.ts", "export * from './dep';\n"); + check( + &dir, + r" +import { NgModule } from '@angular/core'; +import { Missing } from './missing'; +import { NotThere } from './barrel'; +import { StandaloneCmp } from './dep'; +@NgModule({ imports: [Missing, NotThere], exports: [StandaloneCmp] }) +export class M {} +", + &[("M", Some("Missing,NotThere,StandaloneCmp"))], + &cross_file(), + ); + } + + #[test] + fn without_imported_values_imported_classes_stay() { + let dir = TempDir::new().unwrap(); + dep(&dir); + check( + &dir, + r" +import { NgModule } from '@angular/core'; +import { StandaloneDir, DirModule } from './dep'; +@NgModule({ imports: [StandaloneDir, DirModule] }) +export class M {} +", + &[("M", Some("StandaloneDir,DirModule"))], + &TransformOptions::default(), + ); + } + + #[test] + fn files_read_are_dependencies() { + let dir = TempDir::new().unwrap(); + dep(&dir); + write(&dir, "src/barrel.ts", "export * from './dep';\n"); + let source = r" +import { NgModule } from '@angular/core'; +import { StandaloneDir } from './barrel'; +@NgModule({ imports: [StandaloneDir] }) +export class M {} +"; + let path = write(&dir, "src/app.ts", source); + let allocator = Allocator::default(); + let result = transform_angular_file( + &allocator, + path.to_str().unwrap(), + source, + Some(&cross_file()), + None, + ); + assert_eq!(injectors(&result.code), vec!["/*@__PURE__*/i0.ɵɵdefineInjector({})"]); + let names: Vec<&str> = result + .dependencies + .iter() + .filter_map(|dep| std::path::Path::new(dep).file_name()?.to_str()) + .collect(); + assert!(names.contains(&"barrel.ts"), "{:?}", result.dependencies); + assert!(names.contains(&"dep.ts"), "{:?}", result.dependencies); + } +} From 646375a6a2ef711e79b420bb0b9b3bc83e4b4724 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 22:14:52 +0800 Subject: [PATCH 04/18] fix(ngmodule): list the NgModule of ModuleWithProviders imports in partial declarations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In partial mode, ngtsc lists the NgModule each ModuleWithProviders entry names in the `ɵɵngDeclareNgModule` imports (`resolveTypeList` with `createModuleWithProvidersResolver`): `imports: [mwp, Holder.forRoot()]` becomes `imports: [DepModule, DepModule]`. Oxc listed the class the method is called on (`Holder`) and kept constants as is. When a JIT linker inlines that list into `ɵɵdefineNgModule`, the runtime got a class that is no module and a ModuleWithProviders object, which poisons the module scope. Same file, always on: a static method of a class in this file is read in ngtsc's order (a body that is a single `return { ngModule: T }` first, then a `ModuleWithProviders` return type), and top-level constants of such calls or of `{ ngModule: T }` are followed. T must be a class declared before the module, or a binding whose import is kept anyway; otherwise the entry keeps its old output, since a type-only import is removed and a later class would be read before it exists. With cross-file analysis (`resolveImportedValues`), a method or constant of another file is read through ng_module_scope.rs, and the module is named through a namespace import (`i1.DepModule`), as ngtsc does. Only the partial declaration changes: the result goes to a separate `partial_imports` list, so the injector imports and the full-mode `ɵɵsetNgModuleScope` keep the entries as written. Full output is the same. Co-Authored-By: Claude Opus 5.5 --- .../cross_file_elision/ng_module_scope.rs | 96 ++++ .../src/component/transform.rs | 92 ++++ .../src/ng_module/decorator.rs | 7 + .../src/ng_module/definition.rs | 14 +- .../oxc_angular_compiler/src/ng_module/mod.rs | 1 + .../src/ng_module/module_with_providers.rs | 457 ++++++++++++++++++ .../tests/partial_ng_module_mwp_test.rs | 399 +++++++++++++++ 7 files changed, 1065 insertions(+), 1 deletion(-) create mode 100644 crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs create mode 100644 crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs diff --git a/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs b/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs index 969cfded7..070e8d264 100644 --- a/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs +++ b/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs @@ -30,6 +30,9 @@ use rustc_hash::{FxHashMap, FxHashSet}; use super::{CrossFileAnalyzer, Inner, MAX_EXPORT_CHAIN}; use crate::component::metadata::{ComponentScope, ImportedName, ScopeEntry, ScopeKind}; use crate::directive::{Evaluator, StringConsts, find_angular_class_decorator}; +use crate::ng_module::module_with_providers::{ + MwpInit, MwpModule, module_with_providers_init, static_method_ng_module, top_level_const_init, +}; use crate::ng_module::{InjectorDrops, InjectorEntry}; /// A top-level declaration: the file declaring it and its name there @@ -333,6 +336,45 @@ impl CrossFileAnalyzer { } } +impl CrossFileAnalyzer { + /// For each `ModuleWithProviders` entry of an NgModule's `imports` in + /// `file` (`source` is its text), the NgModule it names, as `file` + /// would import it, like ngtsc's `resolveTypeList`. An entry is + /// `(namespace, name, method)`: `name.method(...)` (or + /// `namespace.name.method(...)`), or with no method a constant `name` + /// holding one. The method or constant may be in another file; the + /// NgModule is named through an import of the file it comes from, as + /// ngtsc names a class it found in another file (`i1.DepModule`). + /// `None` for an entry that can't be read, or that names no NgModule. + pub(crate) fn module_with_providers_imports( + &self, + file: &Path, + source: &str, + entries: &[(Option<&str>, &str, Option<&str>)], + ) -> Vec> { + let file = self.inner.set_compiled_source(file, source); + entries + .iter() + .map(|(namespace, name, method)| { + let raw = RawRef { + file: file.clone(), + namespace: namespace.map(Into::into), + name: (*name).into(), + method: None, + }; + let holder = self.inner.resolve_ref(&raw, None, 0)?; + let module = match method { + Some(method) => self.inner.mwp_method_module(&holder, method)?, + None => self.inner.mwp_const_module(&holder)?, + }; + let declared = self.inner.declared(&module.decl)?; + let Declared::NgModule { .. } = &*declared else { return None }; + self.inner.imported_name(&module, &file) + }) + .collect() + } +} + /// `items` with each class once, where it first appears. fn dedupe(items: Vec) -> Vec { let mut seen = FxHashSet::default(); @@ -574,6 +616,45 @@ impl Inner { }) } + /// The NgModule `holder.method(...)` returns a `ModuleWithProviders` + /// of, read in ngtsc's order (see `static_method_ng_module`). + fn mwp_method_module(&self, holder: &ClassRef, method: &str) -> Option { + let file = &holder.decl.file; + let raw = self.parsed_file(file)?.with(|_, program, _| { + let class = find_class(program, &holder.decl.name)?; + match static_method_ng_module(class, method)? { + MwpModule::Value(value) => value_ref(value, file), + MwpModule::Type(name) => type_name_ref(file, name), + } + })?; + self.resolve_ref(&raw, holder.owner.as_deref(), 1) + } + + /// The NgModule the top-level `const` `decl` holds a + /// `ModuleWithProviders` of: `Class.method(...)` or `{ ngModule: T }`. + fn mwp_const_module(&self, decl: &ClassRef) -> Option { + let file = &decl.decl.file; + let (raw, method) = self.parsed_file(file)?.with(|_, program, _| { + match module_with_providers_init(top_level_const_init(program, &decl.decl.name)?)? { + MwpInit::Call { namespace, class, method } => Some(( + RawRef { + file: file.clone(), + namespace: namespace.map(Into::into), + name: class.into(), + method: None, + }, + Some(method.to_string()), + )), + MwpInit::Module(value) => Some((value_ref(value, file)?, None)), + } + })?; + let class = self.resolve_ref(&raw, decl.owner.as_deref(), 1)?; + match method { + Some(method) => self.mwp_method_module(&class, &method), + None => Some(class), + } + } + /// What the declaration `decl` is to a module scope. fn declared(&self, decl: &Decl) -> Option> { if let Some(declared) = self.scope.declared.borrow().get(decl) { @@ -1037,6 +1118,21 @@ fn type_name_ref(file: &str, name: &TSTypeName<'_>) -> Option { Some(RawRef { file: file.into(), namespace, name, method: None }) } +/// The class a value names: `A` or `ns.A`. +fn value_ref(value: &Expression<'_>, file: &str) -> Option { + let (namespace, name) = match value.get_inner_expression() { + Expression::Identifier(id) => (None, id.name.to_string()), + Expression::StaticMemberExpression(member) => { + let Expression::Identifier(ns) = member.object.get_inner_expression() else { + return None; + }; + (Some(ns.name.to_string()), member.property.name.to_string()) + } + _ => return None, + }; + Some(RawRef { file: file.into(), namespace, name, method: None }) +} + /// Whether a type name is `name` or `ns.name`. fn type_name_ends_with(type_name: &TSTypeName<'_>, name: &str) -> bool { match type_name { diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index f6b005ae0..83f22863f 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -4721,6 +4721,29 @@ pub fn transform_angular_file( ); } + // A partial declaration lists the NgModule of each + // `ModuleWithProviders` import, like ngtsc. A class in a + // function may see local bindings: it stays as before. + if matches!(options.compilation_mode, crate::CompilationMode::Partial) + && !site.nested + { + ng_module_metadata.partial_imports = partial_module_with_providers_imports( + allocator, + &parser_ret.program, + class, + &ng_module_metadata, + &string_consts, + &import_map, + &import_elision_mut, + #[cfg(feature = "cross_file_elision")] + cross_file_analyzer + .as_ref() + .filter(|_| options.resolve_imported_values) + .map(|analyzer| (analyzer, path, source)), + &mut file_namespace_registry, + ); + } + // Like ngtsc in full and partial compilation, ɵinj leaves // out the directives and pipes of `imports` and `exports` // (#623). A nested class may name a binding of its @@ -5513,6 +5536,75 @@ fn scope_dependencies<'a>( Some(dependencies) } +/// The partial `ɵɵngDeclareNgModule` imports of the NgModule `class`, with +/// each `ModuleWithProviders` entry listed as its NgModule (see +/// `ng_module::module_with_providers`), or `None` to list them as before. +/// With cross-file analysis, a method or constant of another file is read +/// too, and its NgModule is named through a namespace import, like +/// `ɵɵsetComponentScope` names classes. +#[expect(clippy::too_many_arguments)] +fn partial_module_with_providers_imports<'a>( + allocator: &'a Allocator, + program: &Program<'_>, + class: &oxc_ast::ast::Class<'_>, + metadata: &crate::ng_module::NgModuleMetadata<'a>, + string_consts: &crate::directive::StringConsts<'_>, + import_map: &ImportMap<'_>, + import_elision: &ImportElisionAnalyzer<'_>, + #[cfg(feature = "cross_file_elision")] analyzer: Option<(&CrossFileAnalyzer, &str, &str)>, + registry: &mut NamespaceRegistry<'a>, +) -> Option>> { + use crate::ng_module::module_with_providers::{Binding, partial_imports}; + #[cfg(feature = "cross_file_elision")] + use crate::ng_module::module_with_providers::{NamedClass, ResolveImported}; + // Gives the closure the signature for every lifetime of its entries. + #[cfg(feature = "cross_file_elision")] + fn resolver(f: F) -> F + where + F: Fn(&[(Option<&str>, &str, Option<&str>)]) -> Vec>, + { + f + } + let binding = |name: &str| match import_map.get(&Ident::from(name)) { + Some(info) if info.is_type_only => Binding::Other, + Some(info) if !info.is_named_import => Binding::Namespace, + Some(_) => Binding::Import { kept: !import_elision.should_elide(name) }, + None => Binding::Other, + }; + #[cfg(feature = "cross_file_elision")] + let resolve = analyzer.map(|(analyzer, path, source)| { + resolver(move |entries| { + use super::metadata::ImportedName; + analyzer + .module_with_providers_imports(std::path::Path::new(path), source, entries) + .into_iter() + .map(|name| { + name.map(|name| match name { + ImportedName::Local(name) => NamedClass::Local(name), + ImportedName::Module { specifier, name } => { + NamedClass::Module { specifier, name } + } + }) + }) + .collect() + }) + }); + #[cfg(feature = "cross_file_elision")] + let resolve_imported = resolve.as_ref().map(|resolve| resolve as ResolveImported<'_>); + #[cfg(not(feature = "cross_file_elision"))] + let resolve_imported = None; + partial_imports( + allocator, + program, + class, + string_consts, + &metadata.imports, + binding, + resolve_imported, + |specifier| registry.get_or_assign(&Ident::from(allocator.alloc_str(specifier))), + ) +} + /// `i0.ɵɵsetComponentScope(Cmp, () => [directives], () => [pipes]);` for /// `scope`, with the modules its classes come from imported as namespaces. /// The lists are read when the component first renders: a module in an diff --git a/crates/oxc_angular_compiler/src/ng_module/decorator.rs b/crates/oxc_angular_compiler/src/ng_module/decorator.rs index 9ac2067e2..cc89ae2b3 100644 --- a/crates/oxc_angular_compiler/src/ng_module/decorator.rs +++ b/crates/oxc_angular_compiler/src/ng_module/decorator.rs @@ -57,6 +57,12 @@ pub struct NgModuleMetadata<'a> { /// See [`Self::declarations_scope_expr`]. pub exports_scope_expr: Option>, + /// The partial `ɵɵngDeclareNgModule` imports with each + /// `ModuleWithProviders` listed as its NgModule, set by the transform + /// when it could read one (see `module_with_providers`). `None` lists + /// [`Self::imports`]. Full mode never reads it. + pub partial_imports: Option>>, + /// Providers expression as OutputExpression. pub providers: Option>, @@ -92,6 +98,7 @@ impl<'a> NgModuleMetadata<'a> { declarations_scope_expr: None, imports_scope_expr: None, exports_scope_expr: None, + partial_imports: None, providers: None, bootstrap: Vec::new_in(&allocator), schemas: Vec::new_in(&allocator), diff --git a/crates/oxc_angular_compiler/src/ng_module/definition.rs b/crates/oxc_angular_compiler/src/ng_module/definition.rs index a3b0665ef..3f36d6639 100644 --- a/crates/oxc_angular_compiler/src/ng_module/definition.rs +++ b/crates/oxc_angular_compiler/src/ng_module/definition.rs @@ -198,7 +198,7 @@ pub fn generate_full_ng_module_definition<'a>( angular_version: Option, ) -> Option> { let version = allocator.alloc_str(&crate::partial::declaration_version(angular_version)); - let r3_metadata = metadata.to_r3_metadata(allocator)?; + let mut r3_metadata = metadata.to_r3_metadata(allocator)?; // IMPORTANT: Generate ɵfac BEFORE ɵmod and ɵinj to match Angular's namespace index assignment order. // Angular processes results in order [fac, mod, inj, ...] during the transform phase @@ -223,6 +223,18 @@ pub fn generate_full_ng_module_definition<'a>( // statement upstream-mandated; the linker re-creates remote // scoping at link time if needed. See // `compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts:971`. + // + // Like ngtsc's `resolveTypeList`, the declaration lists the + // NgModule of each `ModuleWithProviders` entry when the transform + // read it. Only here: the full-mode scope keeps the raw list. + if let Some(imports) = &metadata.partial_imports { + r3_metadata.imports = OxcVec::from_iter_in( + imports.iter().map(|import| { + crate::ng_module::R3Reference::value_only(import.clone_in(allocator)) + }), + &allocator, + ); + } let fac_definition = compile_declare_factory_for_ng_module( allocator, core_namespace, diff --git a/crates/oxc_angular_compiler/src/ng_module/mod.rs b/crates/oxc_angular_compiler/src/ng_module/mod.rs index 595d088dd..8d0d5e43f 100644 --- a/crates/oxc_angular_compiler/src/ng_module/mod.rs +++ b/crates/oxc_angular_compiler/src/ng_module/mod.rs @@ -16,6 +16,7 @@ mod decorator; mod definition; mod injector_imports; mod metadata; +pub(crate) mod module_with_providers; pub use compiler::{NgModuleCompileResult, compile_ng_module, compile_ng_module_from_metadata}; pub use decorator::{NgModuleMetadata, extract_ng_module_metadata, find_ng_module_decorator_span}; diff --git a/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs b/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs new file mode 100644 index 000000000..ab2b7dda9 --- /dev/null +++ b/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs @@ -0,0 +1,457 @@ +//! `ModuleWithProviders` entries in a partial `ɵɵngDeclareNgModule`'s +//! `imports`. +//! +//! ngtsc evaluates each `imports` entry, and an entry that gives a +//! `ModuleWithProviders` (`Dep.forRoot()`, or a constant holding one) is +//! listed as the NgModule it names, not as written: `imports: [mwp, +//! Holder.forRoot()]` becomes `imports: [DepModule, DepModule]`. See +//! `NgModuleDecoratorHandler.resolveTypeList` and +//! `createModuleWithProvidersResolver` in +//! `packages/compiler-cli/src/ngtsc/annotations/ng_module/src/`. +//! +//! Oxc used to list the class a method is called on (`Holder`), which is +//! not an NgModule at all when another class holds the method. When a JIT +//! linker inlines that list into `ɵɵdefineNgModule`, the runtime gets a +//! class that is no module and a `ModuleWithProviders` object. +//! +//! Only the partial declaration changes: the injector keeps the raw +//! entries (it needs their providers), and the full-mode +//! `ɵɵsetNgModuleScope` keeps the raw list, which the runtime unwraps. +//! +//! This file reads what one file can tell. With cross-file analysis, a +//! method or constant of another file is read by +//! `CrossFileAnalyzer::module_with_providers_imports`. + +use oxc_ast::ast::{ + ArrayExpressionElement, Class, ClassElement, Declaration, Expression, MethodDefinitionKind, + ObjectPropertyKind, Program, PropertyKey, Statement, TSType, TSTypeName, +}; + +use crate::directive::StringConsts; + +use super::decorator::find_ng_module_decorator; + +/// Where the NgModule of a `ModuleWithProviders` is written. +pub enum MwpModule<'b, 'a> { + /// The `ngModule` value of a returned object literal. + Value(&'b Expression<'a>), + /// `T` of a `ModuleWithProviders` return type. + Type(&'b TSTypeName<'a>), +} + +/// The NgModule the static method `method` of `class` returns a +/// `ModuleWithProviders` of, read in ngtsc's order. Its evaluator first +/// runs a body that is a single `return` (`visitFunctionBody`), and an +/// object literal with `ngModule` there is the answer. Any other body gives +/// a dynamic value, and then `createModuleWithProvidersResolver` reads the +/// `ModuleWithProviders` return type. `None` when unsure: a single +/// `return` of something else (ngtsc would evaluate it further), overloads, +/// a parameter or type parameter in place of the module. +pub fn static_method_ng_module<'b, 'a>( + class: &'b Class<'a>, + method: &str, +) -> Option> { + let mut found = class.body.body.iter().filter_map(|element| match element { + ClassElement::MethodDefinition(m) + if m.r#static + && m.kind == MethodDefinitionKind::Method + && !m.computed + && m.key.static_name().is_some_and(|key| key == method) => + { + Some(m) + } + _ => None, + }); + let function = &found.next()?.value; + if found.next().is_some() { + return None; + } + if let Some(body) = &function.body + && let [Statement::ReturnStatement(ret)] = body.statements.as_slice() + { + let value = ng_module_value(ret.argument.as_ref()?)?; + if let Expression::Identifier(id) = value + && function.params.items.iter().any(|param| { + param.pattern.get_binding_identifiers().iter().any(|b| b.name == id.name) + }) + { + return None; + } + return Some(MwpModule::Value(value)); + } + let return_type = &function.return_type.as_ref()?.type_annotation; + let TSType::TSTypeReference(reference) = return_type else { return None }; + let is_mwp = match &reference.type_name { + TSTypeName::IdentifierReference(id) => id.name == "ModuleWithProviders", + TSTypeName::QualifiedName(name) => name.right.name == "ModuleWithProviders", + TSTypeName::ThisExpression(_) => false, + }; + if !is_mwp { + return None; + } + let [TSType::TSTypeReference(module)] = reference.type_arguments.as_ref()?.params.as_slice() + else { + return None; + }; + if let TSTypeName::IdentifierReference(id) = &module.type_name { + let is_type_param = [&function.type_parameters, &class.type_parameters] + .into_iter() + .flatten() + .any(|params| params.params.iter().any(|p| p.name.name == id.name)); + if is_type_param { + return None; + } + } + Some(MwpModule::Type(&module.type_name)) +} + +/// The `ngModule` value of an object literal, `None` for anything else or +/// when a spread or computed key could set it. +fn ng_module_value<'b, 'a>(expr: &'b Expression<'a>) -> Option<&'b Expression<'a>> { + let Expression::ObjectExpression(object) = expr.get_inner_expression() else { return None }; + let mut value = None; + for property in &object.properties { + let ObjectPropertyKind::ObjectProperty(property) = property else { return None }; + if property.computed { + return None; + } + if property.key.static_name().is_some_and(|key| key == "ngModule") { + value = Some(property.value.get_inner_expression()); + } + } + value +} + +/// How a constant's initializer gives a `ModuleWithProviders`. +pub enum MwpInit<'b, 'a> { + /// `Class.method(...)` or `ns.Class.method(...)`. + Call { namespace: Option<&'b str>, class: &'b str, method: &'b str }, + /// `{ ngModule: T, ... }`: the `T`. + Module(&'b Expression<'a>), +} + +/// How `init` gives a `ModuleWithProviders`, if it is one of the shapes +/// ngtsc can evaluate to one. +pub fn module_with_providers_init<'b, 'a>(init: &'b Expression<'a>) -> Option> { + match init.get_inner_expression() { + Expression::CallExpression(call) => { + let Expression::StaticMemberExpression(member) = call.callee.get_inner_expression() + else { + return None; + }; + let method = member.property.name.as_str(); + match member.object.get_inner_expression() { + Expression::Identifier(class) => { + Some(MwpInit::Call { namespace: None, class: class.name.as_str(), method }) + } + Expression::StaticMemberExpression(class) => { + let Expression::Identifier(ns) = class.object.get_inner_expression() else { + return None; + }; + Some(MwpInit::Call { + namespace: Some(ns.name.as_str()), + class: class.property.name.as_str(), + method, + }) + } + _ => None, + } + } + expr @ Expression::ObjectExpression(_) => ng_module_value(expr).map(MwpInit::Module), + _ => None, + } +} + +/// The initializer of the top-level `const name = ...` of `program`. +pub fn top_level_const_init<'b, 'a>( + program: &'b Program<'a>, + name: &str, +) -> Option<&'b Expression<'a>> { + program.body.iter().find_map(|stmt| { + let declaration = match stmt { + Statement::ExportDeclaration(decl) => &decl.declaration, + _ => stmt.as_declaration()?, + }; + let Declaration::VariableDeclaration(vars) = declaration else { return None }; + if !vars.kind.is_const() { + return None; + } + vars.declarations + .iter() + .find(|var| var.id.get_binding_identifier().is_some_and(|id| id.name == name))? + .init + .as_ref() + }) +} + +/// The top-level class `program` declares as `name`. +fn top_level_class<'b, 'a>(program: &'b Program<'a>, name: &str) -> Option<&'b Class<'a>> { + program.body.iter().find_map(|stmt| { + let class = match stmt { + Statement::ClassDeclaration(class) => class, + Statement::ExportDeclaration(decl) => match &decl.declaration { + Declaration::ClassDeclaration(class) => class, + _ => return None, + }, + Statement::ExportDefaultDeclaration(decl) => match &decl.declaration { + oxc_ast::ast::ExportDefaultDeclarationKind::ClassDeclaration(class) => class, + _ => return None, + }, + _ => return None, + }; + class.id.as_ref().is_some_and(|id| id.name == name).then_some(&**class) + }) +} + +/// One `imports` entry, in the order and shape the decorator parser lists +/// them (`extract_reference_array`): its spreads and other expressions are +/// left out. +pub struct ImportEntry<'b> { + /// The name the partial declaration lists for it today. + pub name: &'b str, + /// The `method` of `name.method(...)`, `None` for a plain name or a + /// `forwardRef`. + pub method: Option<&'b str>, + /// Whether it is written as `forwardRef(() => name)`. + pub forward_ref: bool, +} + +/// The `imports` entries of the `@NgModule` of `class`, `None` when its +/// `imports` is not an array literal. +pub fn ng_module_import_entries<'b>( + class: &'b Class<'_>, + consts: &StringConsts<'_>, +) -> Option>> { + let decorator = find_ng_module_decorator(&class.decorators, Some(consts))?; + let Expression::CallExpression(call) = &decorator.expression else { return None }; + let Some(oxc_ast::ast::Argument::ObjectExpression(config)) = call.arguments.first() else { + return None; + }; + // The parser keeps the last `imports` key. + let imports = config.properties.iter().rev().find_map(|property| { + let ObjectPropertyKind::ObjectProperty(property) = property else { return None }; + let key = match &property.key { + PropertyKey::StaticIdentifier(id) => id.name.as_str(), + PropertyKey::StringLiteral(lit) => lit.value.as_str()?, + _ => return None, + }; + (crate::util::is_metadata_property(property) && key == "imports").then_some(&property.value) + })?; + let Expression::ArrayExpression(array) = imports else { return None }; + let mut entries = Vec::new(); + for element in &array.elements { + match element { + ArrayExpressionElement::Identifier(id) => { + entries.push(ImportEntry { + name: id.name.as_str(), + method: None, + forward_ref: false, + }); + } + ArrayExpressionElement::CallExpression(call) => match &call.callee { + Expression::Identifier(id) if id.name == "forwardRef" => { + if let Some(oxc_ast::ast::Argument::ArrowFunctionExpression(arrow)) = + call.arguments.first() + && let Some(Expression::Identifier(inner)) = arrow.get_expression() + { + entries.push(ImportEntry { + name: inner.name.as_str(), + method: None, + forward_ref: true, + }); + } + } + Expression::StaticMemberExpression(member) => { + if let Expression::Identifier(id) = &member.object { + entries.push(ImportEntry { + name: id.name.as_str(), + method: Some(member.property.name.as_str()), + forward_ref: false, + }); + } + } + _ => {} + }, + _ => {} + } + } + Some(entries) +} + +/// The NgModule a `ModuleWithProviders` entry names, when this file alone +/// tells it: the entry is `Class.method(...)` or a top-level `const` of one +/// (or of `{ ngModule: T }`), `Class` is declared in this file, and `T` is +/// a class this file declares before `module_class`, or a binding whose +/// import the output keeps anyway (`is_kept_import`). ngtsc then names `T` +/// as this file writes it (`DepModule`, not `i1.DepModule`). +/// +/// A class declared later would be read before it exists, and an import +/// only used as a type is removed, so those stay as they were. +pub fn resolve_local_entry<'b>( + program: &'b Program<'_>, + module_class: &Class<'_>, + entry: &ImportEntry<'_>, + is_kept_import: impl Fn(&str) -> bool, +) -> Option<&'b str> { + if entry.forward_ref { + return None; + } + let module = match entry.method { + Some(method) => method_module(program, entry.name, method)?, + None => match module_with_providers_init(top_level_const_init(program, entry.name)?)? { + MwpInit::Call { namespace: None, class, method } => { + method_module(program, class, method)? + } + MwpInit::Call { namespace: Some(_), .. } => return None, + MwpInit::Module(value) => match value { + Expression::Identifier(id) => id.name.as_str(), + _ => return None, + }, + }, + }; + let declared_before = top_level_class(program, module) + .is_some_and(|class| class.span.end <= module_class.span.start); + (declared_before || is_kept_import(module)).then_some(module) +} + +/// The name `program` writes the NgModule of `class.method(...)` by, when +/// `class` is declared there. +fn method_module<'b>(program: &'b Program<'_>, class: &str, method: &str) -> Option<&'b str> { + match static_method_ng_module(top_level_class(program, class)?, method)? { + MwpModule::Value(Expression::Identifier(id)) + | MwpModule::Type(TSTypeName::IdentifierReference(id)) => Some(id.name.as_str()), + _ => None, + } +} + +/// How the transform sees a binding of the compiled file. +pub enum Binding { + /// A value import the output keeps anyway (`kept`), by name or default. + Import { kept: bool }, + /// `import * as ns`. + Namespace, + /// Anything else: a declaration, a type-only import, nothing. + Other, +} + +/// How the compiled file names a class cross-file analysis found. +#[cfg_attr(not(feature = "cross_file_elision"), expect(dead_code))] +pub enum NamedClass { + /// A class the file declares, by its name. + Local(String), + /// The export `name` of the module `specifier`. + Module { specifier: String, name: String }, +} + +/// Cross-file analysis of `(namespace, name, method)` entries: the +/// NgModule each one names, if it can tell. +pub type ResolveImported<'r> = + &'r dyn Fn(&[(Option<&str>, &str, Option<&str>)]) -> std::vec::Vec>; + +/// The partial `ɵɵngDeclareNgModule` imports of the `@NgModule` `class` +/// with each `ModuleWithProviders` entry listed as its NgModule, or `None` +/// when none was read (or the entries don't line up with `current`, the +/// names listed today). An entry this file can't settle goes to +/// `resolve_imported` (cross-file analysis: `(namespace, name, method)` +/// entries), and an entry no one settles keeps today's name. +#[expect(clippy::too_many_arguments)] +pub fn partial_imports<'a>( + allocator: &'a oxc_allocator::Allocator, + program: &Program<'_>, + class: &Class<'_>, + consts: &StringConsts<'_>, + current: &[oxc_str::Ident<'a>], + binding: impl Fn(&str) -> Binding, + resolve_imported: Option>, + mut namespace_of: impl FnMut(&str) -> oxc_str::Ident<'a>, +) -> Option>> { + use crate::output::ast::{OutputExpression, ReadPropExpr, ReadVarExpr}; + use oxc_allocator::Box; + + let entries = ng_module_import_entries(class, consts)?; + if entries.len() != current.len() + || entries.iter().zip(current).any(|(entry, name)| entry.name != name.as_str()) + { + return None; + } + let read_var = |name: oxc_str::Ident<'a>| { + OutputExpression::ReadVar(Box::new_in(ReadVarExpr { name, source_span: None }, &allocator)) + }; + let mut resolved: std::vec::Vec>> = entries + .iter() + .map(|entry| { + let is_kept = |name: &str| matches!(binding(name), Binding::Import { kept: true }); + resolve_local_entry(program, class, entry, is_kept) + .map(|name| read_var(oxc_str::Ident::from(allocator.alloc_str(name)))) + }) + .collect(); + + // The entries whose method or constant may be in another file. + if let Some(resolve_imported) = resolve_imported { + let is_import = |name: &str| matches!(binding(name), Binding::Import { .. }); + let mut asked = std::vec::Vec::new(); + let mut questions = std::vec::Vec::new(); + for (index, entry) in entries.iter().enumerate() { + if resolved[index].is_some() || entry.forward_ref { + continue; + } + let question = match entry.method { + Some(method) if is_import(entry.name) => (None, entry.name, Some(method)), + Some(_) => continue, + None if is_import(entry.name) => (None, entry.name, None), + None => { + let Some(MwpInit::Call { namespace, class, method }) = + top_level_const_init(program, entry.name) + .and_then(module_with_providers_init) + else { + continue; + }; + let imported = match namespace { + Some(ns) => matches!(binding(ns), Binding::Namespace), + None => is_import(class), + }; + if !imported { + continue; + } + (namespace, class, Some(method)) + } + }; + asked.push(index); + questions.push(question); + } + if !questions.is_empty() { + for (index, answer) in asked.into_iter().zip(resolve_imported(&questions)) { + resolved[index] = answer.map(|class| match class { + NamedClass::Local(name) => { + read_var(oxc_str::Ident::from(allocator.alloc_str(&name))) + } + NamedClass::Module { specifier, name } => { + OutputExpression::ReadProp(Box::new_in( + ReadPropExpr { + receiver: Box::new_in( + read_var(namespace_of(&specifier)), + &allocator, + ), + name: oxc_str::Ident::from(allocator.alloc_str(&name)), + optional: false, + source_span: None, + }, + &allocator, + )) + } + }); + } + } + } + + if resolved.iter().all(Option::is_none) { + return None; + } + Some(oxc_allocator::Vec::from_iter_in( + resolved + .into_iter() + .zip(current) + .map(|(resolved, name)| resolved.unwrap_or_else(|| read_var(*name))), + &allocator, + )) +} diff --git a/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs b/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs new file mode 100644 index 000000000..4fa44d0f9 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs @@ -0,0 +1,399 @@ +//! `ModuleWithProviders` entries in a partial `ɵɵngDeclareNgModule`'s +//! `imports` (#623). ngtsc lists the NgModule each one names +//! (`resolveTypeList`): `imports: [mwp, Holder.forRoot()]` becomes +//! `imports: [DepModule, DepModule]`. The injector and the full-mode +//! `ɵɵsetNgModuleScope` keep the entries as written. +//! +//! Each expected list below was checked against ngtsc 22.2.3 on the same +//! input. An entry oxc can't settle keeps its old output. + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{CompilationMode, TransformOptions, transform_angular_file}; + +fn compact(code: &str) -> String { + code.chars().filter(|c| !c.is_whitespace()).collect() +} + +fn compile_in(path: &str, source: &str, options: &TransformOptions) -> String { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, path, source, Some(options), None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + compact(&result.code) +} + +fn partial(source: &str) -> String { + let options = + TransformOptions { compilation_mode: CompilationMode::Partial, ..Default::default() }; + compile_in("app.ts", source, &options) +} + +/// ngtsc partial: `imports: [DepModule, DepModule]`; the injector keeps +/// `[mwp, Holder.forRoot()]`. +const SAME_FILE: &str = r" +import { NgModule, ModuleWithProviders } from '@angular/core'; +@NgModule({}) +export class DepModule {} +export class Holder { + static forRoot(): ModuleWithProviders { return { ngModule: DepModule, providers: [] }; } +} +const mwp = Holder.forRoot(); +@NgModule({ imports: [mwp, Holder.forRoot()] }) +export class AppModule {} +"; + +#[test] +fn same_file_method_and_constant_list_the_module() { + let code = partial(SAME_FILE); + assert!( + code.contains("type:AppModule,imports:[DepModule,DepModule]}"), + "ɵmod should list DepModule twice:\n{code}" + ); + assert!( + code.contains("type:AppModule,imports:[mwp,Holder.forRoot()]}"), + "ɵinj should keep the entries as written:\n{code}" + ); +} + +#[test] +fn full_mode_scope_keeps_the_entries_as_written() { + // ngtsc's full output reads `[DepModule, DepModule]` here, but the + // runtime unwraps the raw list (`convertToTypeArray`), like ngtsc's + // local compilation. Full output doesn't change. + let options = TransformOptions::default(); + let code = compile_in("app.ts", SAME_FILE, &options); + assert!( + code.contains("ɵɵsetNgModuleScope(AppModule,{imports:[mwp,Holder.forRoot()]})"), + "full-mode scope should be raw:\n{code}" + ); +} + +#[test] +fn reads_the_method_in_ngtsc_order() { + // ngtsc evaluates a body that is a single `return` first, so `both` + // gives BModule; any other body falls back to the return type + // (`multi` gives CModule); with no return type the body still counts + // (`plain` gives AModule). Constants follow the same rules. + let code = partial( + r" +import { NgModule, ModuleWithProviders, InjectionToken } from '@angular/core'; +export const TOK = new InjectionToken('tok'); +@NgModule({}) +export class AModule {} +@NgModule({}) +export class BModule {} +@NgModule({}) +export class CModule {} +export class Holder { + static both(): ModuleWithProviders { return { ngModule: BModule, providers: [] }; } + static multi(v: string): ModuleWithProviders { + const providers = [{ provide: TOK, useValue: v }]; + return { ngModule: CModule, providers }; + } + static plain() { return { ngModule: AModule, providers: [] }; } +} +const lit: ModuleWithProviders = { ngModule: BModule, providers: [] }; +const viaCall = Holder.multi('x'); +@NgModule({ imports: [Holder.both(), Holder.multi('y'), Holder.plain(), lit, viaCall, AModule] }) +export class AppModule {} +", + ); + assert!( + code.contains("type:AppModule,imports:[BModule,CModule,AModule,BModule,CModule,AModule]}"), + "{code}" + ); +} + +#[test] +fn local_method_names_an_imported_module_as_written() { + // The module comes from this file's own text, so ngtsc names it as + // written, through the existing import. + let code = partial( + r" +import { NgModule, ModuleWithProviders } from '@angular/core'; +import { DepModule } from './dep'; +export class LocalHolder { + static typed(): ModuleWithProviders { const p: any[] = []; return { ngModule: DepModule, providers: p }; } + static body() { return { ngModule: DepModule, providers: [] }; } +} +@NgModule({ imports: [LocalHolder.typed(), LocalHolder.body()] }) +export class AppModule {} +", + ); + assert!(code.contains("type:AppModule,imports:[DepModule,DepModule]}"), "{code}"); + assert!(code.contains("import{DepModule}from'./dep';"), "{code}"); +} + +#[test] +fn unsure_entries_keep_their_old_output() { + // - `later`: the module is declared after AppModule, so naming it would + // read it before it exists (ngtsc wraps the list in `() => [...]`). + // - `typeOnly`: `DepModule` is only a type here, so its import is + // removed; ngtsc names it anyway and drops the import. + // - `chained`: the body returns another call, which ngtsc evaluates + // further. + let code = partial( + r" +import { NgModule, ModuleWithProviders } from '@angular/core'; +import { DepModule } from './dep'; +export class Holder { + static later(): ModuleWithProviders { const p: any[] = []; return { ngModule: LaterModule, providers: p }; } + static typeOnly(): ModuleWithProviders { const p: any[] = []; return { ngModule: null as any, providers: p }; } + static chained() { return Other.forRoot(); } +} +export class Other { static forRoot(): ModuleWithProviders { return { ngModule: AModule, providers: [] }; } } +@NgModule({}) +export class AModule {} +@NgModule({ imports: [Holder.later(), Holder.typeOnly(), Holder.chained()] }) +export class AppModule {} +@NgModule({}) +export class LaterModule {} +", + ); + assert!(code.contains("type:AppModule,imports:[Holder,Holder,Holder]}"), "{code}"); + assert!(!code.contains("'./dep'"), "the type-only import stays removed:\n{code}"); +} + +#[test] +fn a_parameter_or_type_parameter_keeps_its_old_output() { + // The module comes from the call's argument or type argument, which + // oxc doesn't follow. + let code = partial( + r" +import { NgModule, ModuleWithProviders, Type } from '@angular/core'; +@NgModule({}) +export class AModule {} +export class Holder { + static of(ngModule: Type) { return { ngModule, providers: [] }; } + static generic(): ModuleWithProviders { const m: any = null; return m; } +} +@NgModule({ imports: [Holder.of(AModule), Holder.generic()] }) +export class AppModule {} +", + ); + assert!(code.contains("type:AppModule,imports:[Holder,Holder]}"), "{code}"); +} + +#[test] +fn a_class_in_a_function_keeps_its_old_output() { + let code = partial( + r" +import { NgModule, ModuleWithProviders } from '@angular/core'; +@NgModule({}) +export class DepModule {} +export class Holder { + static forRoot(): ModuleWithProviders { return { ngModule: DepModule, providers: [] }; } +} +export function make() { + @NgModule({ imports: [Holder.forRoot()] }) + class AppModule {} + return AppModule; +} +", + ); + assert!(code.contains("type:AppModule,imports:[Holder]}"), "{code}"); +} + +#[test] +fn an_imported_holder_needs_cross_file_analysis() { + // Without `resolve_imported_values`, oxc can't read another file. + let code = partial( + r" +import { NgModule } from '@angular/core'; +import { OtherModuleHolder, mwp } from './dep'; +@NgModule({ imports: [mwp, OtherModuleHolder.forRoot()] }) +export class AppModule {} +", + ); + assert!(code.contains("type:AppModule,imports:[mwp,OtherModuleHolder]}"), "{code}"); +} + +#[cfg(feature = "cross_file_elision")] +mod cross_file { + use super::*; + use tempfile::TempDir; + + fn write(dir: &TempDir, name: &str, content: &str) -> std::path::PathBuf { + let path = dir.path().join(name); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, content).unwrap(); + path + } + + fn compile(dir: &TempDir, name: &str, source: &str, mode: CompilationMode) -> String { + let path = write(dir, name, source); + let options = TransformOptions { + compilation_mode: mode, + resolve_imported_values: true, + ..Default::default() + }; + compile_in(path.to_str().unwrap(), source, &options) + } + + const DEP: &str = r" +import { NgModule, ModuleWithProviders, Component } from '@angular/core'; +@Component({ selector: 'dep-c', template: 'd', standalone: false }) +export class DepCmp {} +@NgModule({ declarations: [DepCmp], exports: [DepCmp] }) +export class DepModule { + static forRoot(): ModuleWithProviders { + return { ngModule: DepModule, providers: [] }; + } +} +export class OtherModuleHolder { + static forRoot(): ModuleWithProviders { return { ngModule: DepModule, providers: [] }; } +} +export const mwp = DepModule.forRoot(); +"; + + const APP: &str = r" +import { NgModule, Component } from '@angular/core'; +import { OtherModuleHolder, mwp } from './dep'; +@Component({ selector: 'app-c', template: '', standalone: false }) +export class AppCmp {} +@NgModule({ declarations: [AppCmp], imports: [mwp, OtherModuleHolder.forRoot()] }) +export class AppModule {} +"; + + #[test] + fn imported_method_and_constant_name_the_module_through_an_import() { + // ngtsc partial: `import * as i1 from "./dep"` and + // `imports: [i1.DepModule, i1.DepModule]`. + let dir = TempDir::new().unwrap(); + write(&dir, "dep.ts", DEP); + let code = compile(&dir, "app.ts", APP, CompilationMode::Partial); + assert!(code.contains("import*asi1from'./dep';"), "{code}"); + assert!( + code.contains("declarations:[AppCmp],imports:[i1.DepModule,i1.DepModule]}"), + "{code}" + ); + assert!( + code.contains("type:AppModule,imports:[mwp,OtherModuleHolder.forRoot()]}"), + "ɵinj should keep the entries as written:\n{code}" + ); + } + + #[test] + fn full_mode_scope_stays_raw_with_cross_file_analysis() { + let dir = TempDir::new().unwrap(); + write(&dir, "dep.ts", DEP); + let code = compile(&dir, "app.ts", APP, CompilationMode::Full); + assert!( + code.contains( + "ɵɵsetNgModuleScope(AppModule,{declarations:[AppCmp],imports:[mwp,OtherModuleHolder.forRoot()]})" + ), + "{code}" + ); + } + + #[test] + fn a_class_imported_by_name_is_kept_as_written() { + // ngtsc partial: `[DepModule, i1.DepModule, i1.DepModule, i1.DepModule]`: + // a plain entry stays as written, and an NgModule found in another + // file is named through `i1` even when the file imports it by name. + let dir = TempDir::new().unwrap(); + write(&dir, "dep.ts", DEP); + let code = compile( + &dir, + "app.ts", + r" +import { NgModule, Component } from '@angular/core'; +import { DepModule, OtherModuleHolder, mwp } from './dep'; +@Component({ selector: 'app-c', template: '', standalone: false }) +export class AppCmp {} +@NgModule({ declarations: [AppCmp], imports: [DepModule, mwp, OtherModuleHolder.forRoot(), DepModule.forRoot()] }) +export class AppModule {} +", + CompilationMode::Partial, + ); + assert!( + code.contains("imports:[DepModule,i1.DepModule,i1.DepModule,i1.DepModule]}"), + "{code}" + ); + } + + #[test] + fn a_declaration_file_method_reads_its_return_type() { + // ngtsc partial: `import * as i1 from "./lib"`, `imports: [i1.LibModule]`. + let dir = TempDir::new().unwrap(); + write( + &dir, + "lib.d.ts", + r" +import * as i0 from '@angular/core'; +import { ModuleWithProviders } from '@angular/core'; +export declare class LibModule { + static forRoot(): ModuleWithProviders; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵmod: i0.ɵɵNgModuleDeclaration; + static ɵinj: i0.ɵɵInjectorDeclaration; +} +", + ); + let code = compile( + &dir, + "app.ts", + r" +import { NgModule } from '@angular/core'; +import { LibModule } from './lib'; +@NgModule({ imports: [LibModule.forRoot()] }) +export class AppModule {} +", + CompilationMode::Partial, + ); + assert!(code.contains("import*asi1from'./lib';"), "{code}"); + assert!(code.contains("type:AppModule,imports:[i1.LibModule]}"), "{code}"); + } + + #[test] + fn a_local_constant_of_an_imported_method() { + let dir = TempDir::new().unwrap(); + write(&dir, "dep.ts", DEP); + let code = compile( + &dir, + "app.ts", + r" +import { NgModule } from '@angular/core'; +import { OtherModuleHolder } from './dep'; +const local = OtherModuleHolder.forRoot(); +@NgModule({ imports: [local] }) +export class AppModule {} +", + CompilationMode::Partial, + ); + assert!(code.contains("type:AppModule,imports:[i1.DepModule]}"), "{code}"); + } + + #[test] + fn an_unreadable_entry_keeps_its_old_output() { + // No return type and a body ngtsc can't evaluate, and a method that + // names a class which is no NgModule. + let dir = TempDir::new().unwrap(); + write( + &dir, + "dep.ts", + r" +import { NgModule, ModuleWithProviders } from '@angular/core'; +@NgModule({}) +export class DepModule {} +export class NotAModule {} +export class Holder { + static loose() { const p: any[] = []; return { ngModule: DepModule, providers: p }; } + static wrong(): ModuleWithProviders { return { ngModule: NotAModule, providers: [] }; } +} +", + ); + let code = compile( + &dir, + "app.ts", + r" +import { NgModule } from '@angular/core'; +import { Holder, DepModule } from './dep'; +@NgModule({ imports: [Holder.loose(), Holder.wrong(), DepModule] }) +export class AppModule {} +", + CompilationMode::Partial, + ); + assert!(code.contains("type:AppModule,imports:[Holder,Holder,DepModule]}"), "{code}"); + assert!(!code.contains("import*asi1"), "{code}"); + } +} From d6bdde22dce731be9402703b7ef4a32ebb8aa0dc Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 21:57:55 +0800 Subject: [PATCH 05/18] feat(component): add the forbidOrphanComponents option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ngtsc's `forbidOrphanComponents` compiler option adds `forbidOrphanRendering: true` to the `ɵsetClassDebugInfo` call of every component, standalone or not. In dev mode with JIT support the runtime then throws NG0981 when a non-standalone component is rendered before its NgModule was loaded. oxc had no way to turn this on. Add `forbid_orphan_components` to the Rust TransformOptions (default false), `forbidOrphanComponents` to the napi TransformOptions, and a matching vite plugin option. Full mode only: partial output has no class debug info, as in ngtsc. The default output does not change. Checked against ngtsc 22.2.3 on a file with a non-standalone component, a standalone component, a directive and a pipe. Co-Authored-By: Claude Opus 5.5 --- .../src/component/transform.rs | 15 ++- .../tests/forbid_orphan_components_test.rs | 103 ++++++++++++++++++ napi/angular-compiler/index.d.ts | 10 ++ napi/angular-compiler/src/lib.rs | 10 ++ .../test/forbid-orphan-components.test.ts | 76 +++++++++++++ napi/angular-compiler/vite-plugin/index.ts | 13 +++ 6 files changed, 226 insertions(+), 1 deletion(-) create mode 100644 crates/oxc_angular_compiler/tests/forbid_orphan_components_test.rs create mode 100644 napi/angular-compiler/test/forbid-orphan-components.test.ts diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 83f22863f..4fabda89b 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -244,6 +244,15 @@ pub struct TransformOptions { /// Default: `false`, as in the `tsconfig.json` that `ng new` generates. ngtsc itself /// defaults to `true` when the option is not set. pub enable_i18n_legacy_message_id_format: bool, + + /// Angular's `forbidOrphanComponents` compiler option: every component's + /// `ɵsetClassDebugInfo` call gets `forbidOrphanRendering: true`, so that in dev + /// mode (with JIT support) the runtime throws NG0981 when a non-standalone + /// component is rendered before its NgModule was loaded. + /// + /// Full mode only, like ngtsc: partial output has no class debug info. Default: + /// `false`, as in ngtsc (`!!options.forbidOrphanComponents`). + pub forbid_orphan_components: bool, } /// Input for host metadata when passed via TransformOptions. @@ -300,6 +309,7 @@ impl Default for TransformOptions { minify_component_styles: false, compilation_mode: crate::CompilationMode::Full, enable_i18n_legacy_message_id_format: false, + forbid_orphan_components: false, } } } @@ -7310,7 +7320,10 @@ fn compile_component_full<'a>( // Build the debug info with the actual class declaration line number let debug_info = R3ClassDebugInfo::new(component_type, metadata.class_name.clone()) .with_file_path(Ident::from_in(file_path, &allocator)) - .with_line_number(class_line_number); + .with_line_number(class_line_number) + // ngtsc's ComponentDecoratorHandler passes `forbidOrphanComponents` to + // `extractClassDebugInfo` for every component, standalone or not. + .with_forbid_orphan_rendering(options.forbid_orphan_components); // Compile to IIFE-wrapped expression let debug_info_expr = diff --git a/crates/oxc_angular_compiler/tests/forbid_orphan_components_test.rs b/crates/oxc_angular_compiler/tests/forbid_orphan_components_test.rs new file mode 100644 index 000000000..a18fc9473 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/forbid_orphan_components_test.rs @@ -0,0 +1,103 @@ +//! The `forbidOrphanComponents` option (`angularCompilerOptions`) adds +//! `forbidOrphanRendering: true` to every component's `ɵsetClassDebugInfo` +//! call, standalone or not. ngtsc's ComponentDecoratorHandler passes the flag +//! to `extractClassDebugInfo`, and `compileClassDebugInfo` writes the field +//! only when it is set. Directives and pipes get no debug info, and partial +//! output has none at all. +//! +//! Ported from Angular's `ngtsc_spec.ts` "should set forbidOrphanRendering +//! debug info for component when the option forbidOrphanComponents is set" +//! (and the standalone and option-unset cases next to it). The expected calls +//! are what ngtsc 22.2.3 prints for the same input. + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{CompilationMode, TransformOptions, transform_angular_file}; + +const SOURCE: &str = "import { Component, Directive, NgModule, Pipe } from '@angular/core'; + +@Directive({ selector: '[d]', standalone: false }) +export class Dir {} + +@Pipe({ name: 'p', standalone: false }) +export class P { transform(v: any) { return v; } } + +@Component({ + selector: 'comp', + template: '{{ 1 | p }}', + standalone: false, +}) +export class Comp {} + +@NgModule({ declarations: [Comp, Dir, P] }) +export class Mod {} + +@Component({ selector: 'sa', template: 'sa', standalone: true }) +class Sa {} + +export { Sa }; +"; + +fn compile(options: &TransformOptions) -> String { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, "b.ts", SOURCE, Some(options), None); + assert!(result.diagnostics.is_empty(), "{:?}", result.diagnostics); + result.code +} + +/// The `ɵsetClassDebugInfo(...)` calls, without whitespace (the emitter wraps +/// long lines), so they compare with ngtsc's printed output. +fn debug_info_calls(code: &str) -> Vec { + code.match_indices("ɵsetClassDebugInfo(") + .map(|(start, _)| { + let rest = &code[start..]; + let end = rest.find("})").expect("call end") + 2; + rest[..end].chars().filter(|c| !c.is_whitespace()).collect() + }) + .collect() +} + +#[test] +fn sets_forbid_orphan_rendering_on_every_component_when_the_option_is_set() { + // ngtsc 22.2.3, `forbidOrphanComponents: true`, full mode: + // i0.ɵsetClassDebugInfo(Comp, { className: "Comp", filePath: "b.ts", lineNumber: 14, forbidOrphanRendering: true }) + // i0.ɵsetClassDebugInfo(Sa, { className: "Sa", filePath: "b.ts", lineNumber: 20, forbidOrphanRendering: true }) + let options = TransformOptions { forbid_orphan_components: true, ..Default::default() }; + let code = compile(&options); + assert_eq!( + debug_info_calls(&code), + [ + r#"ɵsetClassDebugInfo(Comp,{className:"Comp",filePath:"b.ts",lineNumber:14,forbidOrphanRendering:true})"#, + r#"ɵsetClassDebugInfo(Sa,{className:"Sa",filePath:"b.ts",lineNumber:20,forbidOrphanRendering:true})"#, + ], + "{code}" + ); +} + +#[test] +fn leaves_debug_info_unchanged_when_the_option_is_unset() { + // ngtsc 22.2.3, option unset, full mode: the same calls without the field. + let code = compile(&TransformOptions::default()); + assert_eq!( + debug_info_calls(&code), + [ + r#"ɵsetClassDebugInfo(Comp,{className:"Comp",filePath:"b.ts",lineNumber:14})"#, + r#"ɵsetClassDebugInfo(Sa,{className:"Sa",filePath:"b.ts",lineNumber:20})"#, + ], + "{code}" + ); + assert!(!code.contains("forbidOrphanRendering"), "{code}"); +} + +#[test] +fn partial_mode_emits_no_debug_info_with_the_option_set() { + // ngtsc 22.2.3, `forbidOrphanComponents: true`, partial mode: compilePartial + // passes no debug info, so there is no `ɵsetClassDebugInfo` call. + let options = TransformOptions { + forbid_orphan_components: true, + compilation_mode: CompilationMode::Partial, + ..Default::default() + }; + let code = compile(&options); + assert!(!code.contains("setClassDebugInfo"), "{code}"); + assert!(!code.contains("forbidOrphanRendering"), "{code}"); +} diff --git a/napi/angular-compiler/index.d.ts b/napi/angular-compiler/index.d.ts index b9b942e4f..3c8c52a2b 100644 --- a/napi/angular-compiler/index.d.ts +++ b/napi/angular-compiler/index.d.ts @@ -1107,6 +1107,16 @@ export interface TransformOptions { * Defaults to `false`, as in the `tsconfig.json` that `ng new` generates. */ enableI18nLegacyMessageIdFormat?: boolean + /** + * The `forbidOrphanComponents` Angular compiler option + * (`angularCompilerOptions.forbidOrphanComponents` in `tsconfig.json`). + * + * When `true`, each component's `ɵsetClassDebugInfo` call gets + * `forbidOrphanRendering: true`, so in dev mode (with JIT support) the runtime + * throws NG0981 when a non-standalone component is rendered before its NgModule + * was loaded. Full compilation mode only. Defaults to `false`, as in ngtsc. + */ + forbidOrphanComponents?: boolean /** The CSS selector that identifies this component in a template. */ selector?: string /** diff --git a/napi/angular-compiler/src/lib.rs b/napi/angular-compiler/src/lib.rs index 7c0a1c650..f5135906c 100644 --- a/napi/angular-compiler/src/lib.rs +++ b/napi/angular-compiler/src/lib.rs @@ -169,6 +169,15 @@ pub struct TransformOptions { #[napi(js_name = "enableI18nLegacyMessageIdFormat")] pub enable_i18n_legacy_message_id_format: Option, + /// The `forbidOrphanComponents` Angular compiler option + /// (`angularCompilerOptions.forbidOrphanComponents` in `tsconfig.json`). + /// + /// When `true`, each component's `ɵsetClassDebugInfo` call gets + /// `forbidOrphanRendering: true`, so in dev mode (with JIT support) the runtime + /// throws NG0981 when a non-standalone component is rendered before its NgModule + /// was loaded. Full compilation mode only. Defaults to `false`, as in ngtsc. + pub forbid_orphan_components: Option, + // Component metadata fields for full compilation testing // These mirror Angular's @Component decorator options. /// The CSS selector that identifies this component in a template. @@ -277,6 +286,7 @@ impl From for RustTransformOptions { enable_i18n_legacy_message_id_format: options .enable_i18n_legacy_message_id_format .unwrap_or(false), + forbid_orphan_components: options.forbid_orphan_components.unwrap_or(false), // Component metadata overrides selector: options.selector, standalone: options.standalone, diff --git a/napi/angular-compiler/test/forbid-orphan-components.test.ts b/napi/angular-compiler/test/forbid-orphan-components.test.ts new file mode 100644 index 000000000..c93268161 --- /dev/null +++ b/napi/angular-compiler/test/forbid-orphan-components.test.ts @@ -0,0 +1,76 @@ +import type { Plugin, PluginOption } from 'vite' +import { describe, expect, it } from 'vitest' + +import { angular } from '../vite-plugin/index.js' +import type { PluginOptions } from '../vite-plugin/index.js' + +const COMPONENT_SOURCE = ` + import { Component } from '@angular/core'; + + @Component({ + selector: 'app-root', + template: '
Hello
', + standalone: false, + }) + export class AppComponent {} +` + +function getAngularPlugin(options: PluginOptions = {}): Plugin { + const plugin = (angular(options) as PluginOption[]).find( + (candidate): candidate is Plugin => + !!candidate && + typeof candidate === 'object' && + 'name' in candidate && + candidate.name === '@oxc-angular/vite', + ) + + if (!plugin) { + throw new Error('Failed to find @oxc-angular/vite plugin') + } + + return plugin +} + +async function transform(options: PluginOptions = {}): Promise { + const plugin = getAngularPlugin(options) + + if (!plugin.transform || typeof plugin.transform === 'function') { + throw new Error('Expected plugin transform handler') + } + + const result = await plugin.transform.handler.call( + { + error(message: string) { + throw new Error(message) + }, + warn() {}, + } as any, + COMPONENT_SOURCE, + 'app.component.ts', + ) + + if (!result || typeof result !== 'object' || !('code' in result)) { + throw new Error('Expected transform result with code') + } + + return result.code as string +} + +describe('@oxc-angular/vite forbidOrphanComponents (ngtsc parity)', () => { + // ngtsc adds `forbidOrphanRendering: true` to every component's debug info + // only when `forbidOrphanComponents` is set. + it('leaves debug info without forbidOrphanRendering by default', async () => { + const code = await transform() + + expect(code).toContain('setClassDebugInfo') + expect(code).not.toContain('forbidOrphanRendering') + }) + + it('passes the option to the transform', async () => { + const code = await transform({ forbidOrphanComponents: true }) + + expect(code.replace(/\s+/g, '')).toMatch( + /ɵsetClassDebugInfo\(AppComponent,\{[^}]*forbidOrphanRendering:true\}\)/, + ) + }) +}) diff --git a/napi/angular-compiler/vite-plugin/index.ts b/napi/angular-compiler/vite-plugin/index.ts index 9deb9cc66..a4348f67f 100644 --- a/napi/angular-compiler/vite-plugin/index.ts +++ b/napi/angular-compiler/vite-plugin/index.ts @@ -169,6 +169,17 @@ export interface PluginOptions { * `tsconfig.json` that `ng new` generates. */ enableI18nLegacyMessageIdFormat?: boolean + + /** + * Angular's `forbidOrphanComponents` compiler option: each component's + * `ɵsetClassDebugInfo` call gets `forbidOrphanRendering: true`, so in dev mode + * (with JIT support) the runtime throws NG0981 when a non-standalone component + * is rendered before its NgModule was loaded. + * + * Only affects `compilationMode: 'full'` (partial output has no class debug + * info). Default: `false`, as in ngtsc. + */ + forbidOrphanComponents?: boolean } // Match all TypeScript files - we'll filter by @Component/@Directive decorator in the handler @@ -327,6 +338,7 @@ export function angular(options: PluginOptions = {}): Plugin[] { compilationMode: options.compilationMode ?? 'full', preserveWhitespaces: options.preserveWhitespaces, enableI18nLegacyMessageIdFormat: options.enableI18nLegacyMessageIdFormat ?? false, + forbidOrphanComponents: options.forbidOrphanComponents ?? false, tsconfig: options.tsconfig, } @@ -588,6 +600,7 @@ export function angular(options: PluginOptions = {}): Plugin[] { compilationMode: pluginOptions.compilationMode, preserveWhitespaces: pluginOptions.preserveWhitespaces, enableI18nLegacyMessageIdFormat: pluginOptions.enableI18nLegacyMessageIdFormat, + forbidOrphanComponents: pluginOptions.forbidOrphanComponents, // Evaluate decorator metadata imported from other files the way // ngtsc's program-wide checker does (`inputs: INPUTS`, `@Input(OPTS)`, // `templateUrl: TEMPLATE_URL`). Read files land in From 19ea85fdd9a6d39ae037385c27b65cdebd7bb033 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 22:23:33 +0800 Subject: [PATCH 06/18] feat(compiler): add enableTemplateSourceLocations and rootDirs options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Angular's `enableTemplateSourceLocations` option ends each view's create block with one `ɵɵattachSourceLocations(path, [[slot, offset, line, column], ...])` call. oxc had a port of the phase, but it ran whenever advancedOptimizations was off, made one op per element with 1-based lines and UTF-8 offsets, and reify threw the ops away. Now it works like ngtsc full mode: - off by default; when off the phase does no work and output is the same - one call per view, at the end of the create block, not chained - 0-based line and column and UTF-16 offsets, counted like Angular's lexer cursor in the raw source: from the .ts file start for an inline literal (escapes sit at their backslash), from the .html start for templateUrl, and from the template start for any other inline value - slots come from the element ops' own slots - only for full compilation (the linker hard-codes it off) and Angular 19.1.0-next.3 or later (or no version given) The path is project-relative, like ngtsc's getProjectRelativePath, using a new `root_dirs` option. With no root dirs nothing is emitted (ngtsc would fall back to rootDir or the working directory, which oxc does not know). An inline literal whose raw text cannot be mapped (legacy octal or `\u` surrogate escapes) also emits nothing. With root dirs set, `ɵsetClassDebugInfo`'s `filePath` is also made project-relative, and left out for a file outside every root, as ngtsc's extractClassDebugInfo does. Without root dirs it stays the path given. oxc never lowercases paths; ngtsc does on a case-insensitive file system. The napi TransformOptions gain the same two options (`enableTemplateSourceLocations`, `rootDirs`). Refs #623. Co-Authored-By: Claude Opus 5.5 --- .../oxc_angular_compiler/src/component/mod.rs | 1 + .../src/component/source_locations.rs | 276 ++++++++++++++ .../src/component/transform.rs | 86 ++++- crates/oxc_angular_compiler/src/ir/ops.rs | 26 +- .../src/pipeline/compilation.rs | 15 +- .../src/pipeline/ingest.rs | 11 +- .../phases/attach_source_locations.rs | 231 +++++++----- .../src/pipeline/phases/mod.rs | 2 +- .../src/pipeline/phases/reify/mod.rs | 47 ++- .../tests/template_source_locations_test.rs | 343 ++++++++++++++++++ napi/angular-compiler/index.d.ts | 20 + napi/angular-compiler/src/lib.rs | 22 ++ 12 files changed, 951 insertions(+), 129 deletions(-) create mode 100644 crates/oxc_angular_compiler/src/component/source_locations.rs create mode 100644 crates/oxc_angular_compiler/tests/template_source_locations_test.rs diff --git a/crates/oxc_angular_compiler/src/component/mod.rs b/crates/oxc_angular_compiler/src/component/mod.rs index 8683a5c80..ba0433b42 100644 --- a/crates/oxc_angular_compiler/src/component/mod.rs +++ b/crates/oxc_angular_compiler/src/component/mod.rs @@ -18,6 +18,7 @@ mod import_elision; mod metadata; mod namespace_registry; pub(crate) mod signal_debug_name; +mod source_locations; mod transform; #[cfg(feature = "cross_file_elision")] diff --git a/crates/oxc_angular_compiler/src/component/source_locations.rs b/crates/oxc_angular_compiler/src/component/source_locations.rs new file mode 100644 index 000000000..28a6df239 --- /dev/null +++ b/crates/oxc_angular_compiler/src/component/source_locations.rs @@ -0,0 +1,276 @@ +//! What ngtsc's `enableTemplateSourceLocations` and `rootDirs` options need: +//! the project-relative path of a file, and where a template's text sits in +//! the source its positions are counted in. + +use oxc_allocator::Allocator; +use oxc_str::Ident; + +use super::metadata::{AngularVersion, ComponentMetadata, Prerelease}; +use super::transform::ResolvedResources; +use crate::pipeline::phases::attach_source_locations::{TemplateSource, utf16_len}; + +/// Whether the runtime has `ɵɵattachSourceLocations`: it first shipped in +/// `19.1.0-next.3` (angular/angular#58982). An unknown version is the latest. +pub(super) fn supports_attach_source_locations(version: Option) -> bool { + version.is_none_or(|v| match (v.major, v.minor, v.patch) { + (19, 1, 0) => v.prerelease.is_none_or(|p| p >= Prerelease::Next(3)), + triple => triple > (19, 1, 0), + }) +} + +/// `file` relative to the first of `root_dirs` that contains it, or `None` +/// when none does. +/// +/// Mirrors ngtsc's `getProjectRelativePath` (compiler-cli `util/src/path.ts`), +/// with root dirs resolved against the working directory like `getRootDirs`. +/// Unlike ngtsc, the path is never lowercased: ngtsc's `getCanonicalFileName` +/// lowercases it on a case-insensitive file system (macOS, Windows), so the +/// two only match there for lowercase paths. +pub(super) fn project_relative_path(file: &str, root_dirs: &[String]) -> Option { + let file = resolve(file); + root_dirs.iter().find_map(|root| { + let rel = relative(&resolve(root), &file); + (!rel.starts_with("..")).then_some(rel) + }) +} + +/// The path segments of `path` made absolute against the working directory, +/// with `.` and `..` resolved and `\` read as `/`. +fn resolve(path: &str) -> Vec { + let path = path.replace('\\', "/"); + let bytes = path.as_bytes(); + let is_absolute = path.starts_with('/') + || (bytes.len() >= 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':'); + let full = if is_absolute { + path + } else { + // Without a working directory (wasm), a relative path stays relative: + // two such paths still compare against the same unknown base. + match std::env::current_dir() { + Ok(cwd) => format!("{}/{path}", cwd.to_string_lossy().replace('\\', "/")), + Err(_) => path, + } + }; + let mut segments: Vec = Vec::new(); + for segment in full.split('/') { + match segment { + "" | "." => {} + ".." => { + segments.pop(); + } + _ => segments.push(segment.to_string()), + } + } + segments +} + +/// Node's `path.relative(from, to)` on resolved segments. +fn relative(from: &[String], to: &[String]) -> String { + let common = from.iter().zip(to).take_while(|(a, b)| a == b).count(); + let mut parts: Vec<&str> = vec![".."; from.len() - common]; + parts.extend(to[common..].iter().map(String::as_str)); + parts.join("/") +} + +/// The template path and source text for `ɵɵattachSourceLocations`, the way +/// ngtsc's `ComponentDecoratorHandler.analyze` and `extractTemplate` give them: +/// +/// - a `templateUrl` template: the `.html` file, positions from its start; +/// - an inline string or no-substitution template literal: the `.ts` file, +/// positions in its raw text; +/// - any other inline template (a variable, a concatenation): the `.ts` +/// file, positions in the template value from 0. +/// +/// `None` when there is nothing to emit: no root dirs (ngtsc's root is +/// `rootDir`, else the working directory; oxc gets no root here and emits +/// nothing, like ngtsc for a file outside its root), a file outside the root +/// dirs, a `templateUrl` that is not relative to the component file, or an +/// inline literal whose raw text oxc cannot map to its value. +pub(super) fn template_source<'a>( + allocator: &'a Allocator, + metadata: &ComponentMetadata<'_>, + template: &'a str, + file_path: &str, + source: &str, + root_dirs: Option<&[String]>, + resolved_resources: Option<&ResolvedResources>, +) -> Option<(Ident<'a>, TemplateSource<'a>)> { + let root_dirs = root_dirs?; + let whole = + |text| TemplateSource { text, start: 0, start_line: 0, start_column: 0, raw_offsets: None }; + + // `templateResource.path` (the resolved `templateUrl`) when the template + // is an external file, as `TemplateOrigin::of_component` decides it. + if let Some(url) = &metadata.template_url + && resolved_resources.is_some() + { + let url = url.as_str().replace('\\', "/"); + if url.starts_with('/') { + // ngtsc looks a rooted URL up in each root dir; oxc cannot tell which. + return None; + } + let file = file_path.replace('\\', "/"); + let dir = file.rfind('/').map_or("", |i| &file[..i]); + let path = project_relative_path(&format!("{dir}/{url}"), root_dirs)?; + return Some((Ident::from(allocator.alloc_str(&path)), whole(template))); + } + + if metadata.template.as_ref().is_none_or(|inline| inline.as_str() != template) { + return None; + } + let path = Ident::from(allocator.alloc_str(&project_relative_path(file_path, root_dirs)?)); + let Some(span) = metadata.template_span else { + return Some((path, whole(template))); + }; + let raw = source.get(span.start as usize..span.end as usize)?; + let raw_offsets = if raw == template { + None + } else { + let is_template_literal = source.as_bytes().get(span.start as usize - 1) == Some(&b'`'); + let (cooked, offsets) = cook_literal(raw, span.start, is_template_literal)?; + if cooked != template { + return None; + } + Some(&*allocator.alloc_slice_copy(&offsets)) + }; + let (start_line, start_column) = typescript_line_and_character(source, span.start as usize); + Some(( + path, + TemplateSource { + text: allocator.alloc_str(source), + start: span.start, + start_line, + start_column, + raw_offsets, + }, + )) +} + +/// The 0-based line and UTF-16 character of byte `offset`, as TypeScript's +/// `getLineAndCharacterOfPosition` gives them: `\r\n`, `\r`, `\n`, U+2028 and +/// U+2029 each end a line. +fn typescript_line_and_character(source: &str, offset: usize) -> (u32, u32) { + let (mut line, mut character) = (0, 0); + let mut chars = source[..offset.min(source.len())].chars().peekable(); + while let Some(c) = chars.next() { + match c { + '\r' | '\n' | '\u{2028}' | '\u{2029}' => { + if c == '\r' && chars.peek() == Some(&'\n') { + chars.next(); + } + line += 1; + character = 0; + } + _ => character += utf16_len(c), + } + } + (line, character) +} + +/// The value of a string or template literal from its raw text (between the +/// quotes), with the byte offset in the source of the character each value +/// byte comes from: an escaped character comes from its backslash, as in +/// Angular's `EscapedCharacterCursor`. +/// +/// `None` for escapes oxc does not map (legacy octal, `\u` surrogates); the +/// caller then emits no source locations. +fn cook_literal(raw: &str, base: u32, is_template_literal: bool) -> Option<(String, Vec)> { + let mut cooked = String::with_capacity(raw.len()); + let mut offsets = Vec::with_capacity(raw.len()); + let mut push = |c: char, at: usize| { + cooked.push(c); + let at = base.saturating_add(u32::try_from(at).unwrap_or(u32::MAX)); + offsets.extend(std::iter::repeat_n(at, c.len_utf8())); + }; + let mut chars = raw.char_indices().peekable(); + while let Some((at, c)) = chars.next() { + match c { + '\\' => { + let (_, escape) = chars.next()?; + let value = match escape { + 'n' => '\n', + 'r' => '\r', + 't' => '\t', + 'b' => '\u{8}', + 'f' => '\u{c}', + 'v' => '\u{b}', + '0' if !chars.peek().is_some_and(|(_, c)| c.is_ascii_digit()) => '\0', + '0'..='9' => return None, + 'x' => { + let hex: String = + (0..2).filter_map(|_| chars.next().map(|(_, c)| c)).collect(); + char::from_u32(u32::from_str_radix(&hex, 16).ok()?)? + } + 'u' => { + let hex: String = if chars.peek().is_some_and(|&(_, c)| c == '{') { + chars.next(); + chars.by_ref().map(|(_, c)| c).take_while(|&c| c != '}').collect() + } else { + (0..4).filter_map(|_| chars.next().map(|(_, c)| c)).collect() + }; + // A surrogate (half of a pair) has no `char`. + char::from_u32(u32::from_str_radix(&hex, 16).ok()?)? + } + // A line continuation adds nothing. + '\r' => { + chars.next_if(|&(_, c)| c == '\n'); + continue; + } + '\n' | '\u{2028}' | '\u{2029}' => continue, + other => other, + }; + push(value, at); + } + // A template literal's value reads `\r\n` and `\r` as `\n`. + '\r' if is_template_literal => { + chars.next_if(|&(_, c)| c == '\n'); + push('\n', at); + } + _ => push(c, at), + } + } + Some((cooked, offsets)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn relative_to_the_first_root_that_contains_the_file() { + let roots = ["/w/lib".to_string(), "/w".to_string()]; + assert_eq!(project_relative_path("/w/src/a.ts", &roots).as_deref(), Some("src/a.ts")); + assert_eq!(project_relative_path("/w/lib/b.ts", &roots).as_deref(), Some("b.ts")); + assert_eq!(project_relative_path("/x/a.ts", &roots), None); + assert_eq!(project_relative_path("/w/./src/../a.ts", &roots).as_deref(), Some("a.ts")); + assert_eq!( + project_relative_path("C:\\w\\src\\a.ts", &["C:/w".to_string()]).as_deref(), + Some("src/a.ts") + ); + } + + #[test] + fn cooks_escapes_to_their_backslash() { + let (cooked, offsets) = cook_literal( + r"a\nb\x3Cc\u{41}\ +d", + 10, + false, + ) + .unwrap(); + assert_eq!(cooked, "a\nb>, } /// Input for host metadata when passed via TransformOptions. @@ -310,6 +337,8 @@ impl Default for TransformOptions { compilation_mode: crate::CompilationMode::Full, enable_i18n_legacy_message_id_format: false, forbid_orphan_components: false, + enable_template_source_locations: false, + root_dirs: None, } } } @@ -7031,13 +7060,28 @@ fn compile_component_full<'a>( DeferBlockDepsEmitMode::PerBlock }; - // Enable debug locations when NOT using advanced optimizations - // (advanced optimizations strip debug info for smaller output) - let enable_debug_locations = !options.advanced_optimizations; - - // Build relative paths for debug locations - let relative_template_path = - if enable_debug_locations { Some(Ident::from_in(file_path, &allocator)) } else { None }; + // Angular's `enableTemplateSourceLocations`, which ngtsc reads only for + // full compilation (the linker hard-codes it off). The path and source + // come from `ComponentDecoratorHandler.analyze`; with neither, the phase + // does nothing. + let template_source = if options.enable_template_source_locations + && matches!(options.compilation_mode, crate::CompilationMode::Full) + && super::source_locations::supports_attach_source_locations(options.angular_version) + { + super::source_locations::template_source( + allocator, + metadata, + template, + file_path, + source, + options.root_dirs.as_deref(), + resolved_resources, + ) + } else { + None + }; + let enable_debug_locations = template_source.is_some(); + let (relative_template_path, template_source) = template_source.unzip(); let relative_context_file_path = Some(Ident::from_in(file_path, &allocator)); @@ -7048,7 +7092,7 @@ fn compile_component_full<'a>( defer_block_deps_emit_mode, relative_template_path, enable_debug_locations, - template_source: if enable_debug_locations { Some(template) } else { None }, + template_source, // PerComponent mode: pass the shared resolver expression so the // ingest stage attaches it to every defer block's create op. all_deferrable_deps_fn: defer_resolver_fn, @@ -7318,12 +7362,22 @@ fn compile_component_full<'a>( let class_line_number = line_number_at(source, metadata.class_name_pos as usize); // Build the debug info with the actual class declaration line number - let debug_info = R3ClassDebugInfo::new(component_type, metadata.class_name.clone()) - .with_file_path(Ident::from_in(file_path, &allocator)) + // ngtsc's `extractClassDebugInfo` gives the path relative to the + // project root, and none for a file outside it. With no root dirs, + // keep the path as given. + let debug_file_path = match &options.root_dirs { + Some(roots) => super::source_locations::project_relative_path(file_path, roots) + .filter(|path| !path.is_empty()), + None => Some(file_path.to_string()), + }; + let mut debug_info = R3ClassDebugInfo::new(component_type, metadata.class_name.clone()) .with_line_number(class_line_number) // ngtsc's ComponentDecoratorHandler passes `forbidOrphanComponents` to // `extractClassDebugInfo` for every component, standalone or not. .with_forbid_orphan_rendering(options.forbid_orphan_components); + if let Some(path) = debug_file_path { + debug_info = debug_info.with_file_path(Ident::from_in(path.as_str(), &allocator)); + } // Compile to IIFE-wrapped expression let debug_info_expr = @@ -7754,7 +7808,8 @@ pub fn compile_template_to_js_with_options<'a>( DeferBlockDepsEmitMode::PerBlock }; - let enable_debug_locations = !options.advanced_optimizations; + // No template path here, so `attachSourceLocations` has nothing to emit. + let enable_debug_locations = false; let ingest_options = IngestOptions { mode, @@ -7763,7 +7818,7 @@ pub fn compile_template_to_js_with_options<'a>( defer_block_deps_emit_mode, relative_template_path: None, enable_debug_locations, - template_source: Some(template), + template_source: None, all_deferrable_deps_fn: None, defer_blocks: FxHashMap::default(), pool_starting_index: 0, // Standalone template compilation starts from 0 @@ -7959,7 +8014,8 @@ pub fn compile_template_for_hmr<'a>( // PerBlock's empty map would have produced anyway. let defer_block_deps_emit_mode = DeferBlockDepsEmitMode::PerComponent; - let enable_debug_locations = !options.advanced_optimizations; + // No template path here, so `attachSourceLocations` has nothing to emit. + let enable_debug_locations = false; let ingest_options = IngestOptions { mode, @@ -7968,7 +8024,7 @@ pub fn compile_template_for_hmr<'a>( defer_block_deps_emit_mode, relative_template_path: None, enable_debug_locations, - template_source: Some(template), + template_source: None, all_deferrable_deps_fn: None, defer_blocks: FxHashMap::default(), pool_starting_index: 0, // HMR template compilation starts from 0 @@ -8762,7 +8818,7 @@ pub fn compile_template_for_linker<'a>( defer_block_deps_emit_mode: DeferBlockDepsEmitMode::PerBlock, relative_template_path: None, enable_debug_locations: false, - template_source: Some(template), + template_source: None, all_deferrable_deps_fn: None, defer_blocks, pool_starting_index, diff --git a/crates/oxc_angular_compiler/src/ir/ops.rs b/crates/oxc_angular_compiler/src/ir/ops.rs index bcbdc4779..e749b4fef 100644 --- a/crates/oxc_angular_compiler/src/ir/ops.rs +++ b/crates/oxc_angular_compiler/src/ir/ops.rs @@ -1494,18 +1494,30 @@ pub struct ExtractedAttributeOp<'a> { pub trusted_value_fn: Option>, } -/// Source location for debugging. +/// The source locations of a view's elements, reified into one +/// `ɵɵattachSourceLocations(path, [[slot, offset, line, column], ...])` call. +/// +/// Ported from Angular's `SourceLocationOp` in `ir/src/ops/create.ts`. #[derive(Debug)] pub struct SourceLocationOp<'a> { /// Common operation metadata. pub base: CreateOpBase<'a>, - /// Target element. - pub target: XrefId, - /// Template URL. - pub template_url: Ident<'a>, - /// Line number. + /// The template's path, relative to the project root. + pub template_path: Ident<'a>, + /// Where each element of the view starts. + pub locations: Vec<'a, SourceLocation>, +} + +/// Where one element starts in the template's source. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SourceLocation { + /// The element's slot. + pub slot: u32, + /// UTF-16 offset in the source. + pub offset: u32, + /// 0-based line. pub line: u32, - /// Column number. + /// 0-based UTF-16 column. pub column: u32, } diff --git a/crates/oxc_angular_compiler/src/pipeline/compilation.rs b/crates/oxc_angular_compiler/src/pipeline/compilation.rs index 5176c5643..1d48d9f22 100644 --- a/crates/oxc_angular_compiler/src/pipeline/compilation.rs +++ b/crates/oxc_angular_compiler/src/pipeline/compilation.rs @@ -182,16 +182,17 @@ pub struct ComponentCompilationJob<'a> { pub relative_context_file_path: Option>, /// Relocation entries. pub relocation_entries: Vec<'a, RelocationEntry>, - /// Whether to attach debug source locations. - /// When enabled, the `attach_source_locations` phase will generate - /// `ɵɵsourceLocation` calls for debugging. + /// Whether to attach debug source locations (Angular's + /// `enableTemplateSourceLocations`). When enabled, the + /// `attach_source_locations` phase emits `ɵɵattachSourceLocations` calls. pub enable_debug_locations: bool, - /// Relative path to the template file for source location debugging. - /// Required when `enable_debug_locations` is true. + /// The template's path relative to the project root, as ngtsc's + /// `getProjectRelativePath` gives it. Required when `enable_debug_locations` is true. pub relative_template_path: Option>, - /// Template source text for computing line/column from byte offsets. + /// The text the template's element positions count in. /// Required when `enable_debug_locations` is true. - pub template_source: Option<&'a str>, + pub template_source: + Option>, /// Defer block metadata. /// /// Controls how defer block dependencies are emitted (per-block or per-component). diff --git a/crates/oxc_angular_compiler/src/pipeline/ingest.rs b/crates/oxc_angular_compiler/src/pipeline/ingest.rs index 118dfea2d..550264596 100644 --- a/crates/oxc_angular_compiler/src/pipeline/ingest.rs +++ b/crates/oxc_angular_compiler/src/pipeline/ingest.rs @@ -85,18 +85,19 @@ pub struct IngestOptions<'a> { /// PerComponent is used in local/JIT compilation. pub defer_block_deps_emit_mode: DeferBlockDepsEmitMode, - /// Relative path to the template file for source location debugging. + /// The template's path relative to the project root, for source locations. pub relative_template_path: Option>, - /// Whether to enable debug source locations. + /// Whether to enable debug source locations (Angular's `enableTemplateSourceLocations`). /// - /// When enabled, the compiler generates `ɵɵsourceLocation` calls for debugging. + /// When enabled, the compiler generates `ɵɵattachSourceLocations` calls for debugging. pub enable_debug_locations: bool, - /// Template source text for computing line/column from byte offsets. + /// The text the template's element positions count in. /// /// Required when `enable_debug_locations` is true. - pub template_source: Option<&'a str>, + pub template_source: + Option>, /// Reference to the deferrable dependencies function for PerComponent mode. /// diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/attach_source_locations.rs b/crates/oxc_angular_compiler/src/pipeline/phases/attach_source_locations.rs index 9f2690892..900dc36e6 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/attach_source_locations.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/attach_source_locations.rs @@ -1,121 +1,172 @@ //! Attach source locations phase. //! -//! Attaches debug source location information to operations. When enabled, -//! this phase collects the source locations of all elements in the template -//! and generates `ɵɵsourceLocation` calls that expose this information -//! in the DOM for debugging purposes. +//! With Angular's `enableTemplateSourceLocations` option, each view's create +//! block ends with one `ɵɵattachSourceLocations(path, [[slot, offset, line, +//! column], ...])` call that lists where each of its elements starts in the +//! source. The runtime writes it into a `data-ng-source-location` attribute. //! //! Ported from Angular's `template/pipeline/src/phases/attach_source_locations.ts`. -use oxc_str::Ident; - -use crate::ir::ops::{CreateOp, CreateOpBase, SourceLocationOp, XrefId}; +use crate::ir::ops::{CreateOp, CreateOpBase, SourceLocation, SourceLocationOp}; use crate::pipeline::compilation::ComponentCompilationJob; +use oxc_allocator::Vec as OxcVec; -/// Collected element source location for debugging. -#[derive(Debug, Clone)] -struct ElementSourceLocation { - /// Target element's xref ID. - target: XrefId, - /// Line number (1-indexed). - line: u32, - /// Column number (0-indexed). - column: u32, +/// The text Angular's template lexer reads a template from, so element +/// positions can be given the way ngtsc gives them. +/// +/// ngtsc (`extractTemplate` in compiler-cli's component resources) lexes an +/// inline string or no-substitution template literal straight out of the +/// `.ts` source, so its positions are those of the raw source. Any other +/// template (a `templateUrl` file, or a value read from a variable) is lexed +/// on its own, from position 0. +#[derive(Debug, Clone, Copy)] +pub struct TemplateSource<'a> { + /// The text the positions count in: the whole `.ts` source for an inline + /// literal, the template itself otherwise. + pub text: &'a str, + /// The byte offset in `text` where the template starts. + pub start: u32, + /// The 0-based line and UTF-16 column of `start`, from TypeScript's + /// `getLineAndCharacterOfPosition` (ngtsc's `getTemplateRange`). + pub start_line: u32, + pub start_column: u32, + /// For an inline literal written with escapes, the byte offset in `text` + /// of each byte of the cooked template. `None` when the cooked template + /// is the raw text from `start` on. + pub raw_offsets: Option<&'a [u32]>, } -/// Computes line (1-indexed) and column (0-indexed) from a byte offset. -fn offset_to_line_column(source: &str, offset: u32) -> (u32, u32) { - let offset = offset as usize; - let bytes = source.as_bytes(); +impl TemplateSource<'_> { + /// The UTF-16 offset, 0-based line and 0-based UTF-16 column of each byte + /// offset in the cooked template, in the order given. + /// + /// Line and column are counted the way Angular's lexer cursor + /// (`PlainCharacterCursor.advanceState`) counts them over the raw text: + /// `\n` starts a line, `\r` moves nothing, and any other UTF-16 unit + /// moves one column. An escape sequence is counted by its raw characters, + /// and a character it stands for sits at its backslash. + fn locate(&self, offsets: &[u32]) -> std::vec::Vec<(u32, u32, u32)> { + let raw = |offset: u32| match self.raw_offsets { + Some(map) => map.get(offset as usize).copied().unwrap_or(self.start), + None => self.start + offset, + } as usize; - let mut line = 1u32; - let mut col = 0u32; + // One forward pass over the text, visiting the offsets in order. + let mut order: std::vec::Vec = (0..offsets.len()).collect(); + order.sort_unstable_by_key(|&i| raw(offsets[i])); + let mut result = vec![(0, 0, 0); offsets.len()]; - for (i, &byte) in bytes.iter().enumerate() { - if i >= offset { - break; - } - if byte == b'\n' { - line += 1; - col = 0; - } else { - col += 1; + let start = self.start as usize; + let mut pos = start; + let mut utf16 = + u32::try_from(self.text[..start].encode_utf16().count()).unwrap_or(u32::MAX); + let (mut line, mut column) = (self.start_line, self.start_column); + for i in order { + let target = raw(offsets[i]).min(self.text.len()); + if target > pos { + for c in self.text[pos..target].chars() { + let units = utf16_len(c); + utf16 += units; + match c { + '\n' => { + line += 1; + column = 0; + } + '\r' => {} + _ => column += units, + } + } + pos = target; + } + result[i] = (utf16, line, column); } + result } +} - (line, col) +/// The UTF-16 units `c` takes: two above the Basic Multilingual Plane. +pub fn utf16_len(c: char) -> u32 { + if u32::from(c) > 0xFFFF { 2 } else { 1 } } /// Attaches source location information for debugging. /// -/// This phase collects element definitions from the create block and -/// outputs operations that expose their source locations in the DOM. -/// This is only enabled when `job.enable_debug_locations` is true and -/// `job.relative_template_path` is set. -/// -/// The collected information includes: -/// - Target xref of the element -/// - Source offset, line, and column +/// For each view, collects the element ops of the create block and pushes a +/// single `SourceLocation` op at its end, as Angular's `attachSourceLocations` +/// does. It only runs when `job.enable_debug_locations` is true and both the +/// template path and the template source are known. pub fn attach_source_locations(job: &mut ComponentCompilationJob<'_>) { - // Early return if debug locations are disabled or path is not set if !job.enable_debug_locations { return; } - let template_path = match &job.relative_template_path { - Some(path) => path.clone(), - None => return, - }; - let template_source = match job.template_source { - Some(source) => source, - None => return, + let (Some(template_path), Some(source)) = (job.relative_template_path, job.template_source) + else { + return; }; + let allocator = job.allocator; - // Collect view xrefs to avoid borrow issues - let view_xrefs: Vec<_> = job.all_views().map(|v| v.xref).collect(); - + let view_xrefs: std::vec::Vec<_> = job.all_views().map(|v| v.xref).collect(); for view_xref in view_xrefs { - if let Some(view) = job.view_mut(view_xref) { - // Collect element source locations from create ops - let mut locations: Vec = Vec::new(); + let Some(view) = job.view_mut(view_xref) else { continue }; - for op in view.create.iter() { - match op { - CreateOp::ElementStart(elem) => { - if let Some(span) = elem.base.source_span { - let (line, column) = offset_to_line_column(template_source, span.start); - locations.push(ElementSourceLocation { - target: elem.xref, - line, - column, - }); - } - } - CreateOp::Element(elem) => { - if let Some(span) = elem.base.source_span { - let (line, column) = offset_to_line_column(template_source, span.start); - locations.push(ElementSourceLocation { - target: elem.xref, - line, - column, - }); - } - } - _ => {} - } + // Angular reads `op.startSourceSpan.start` of every `ElementStart` and + // `Element` op, and the slot the op was given. + let mut elements: std::vec::Vec<(u32, u32)> = std::vec::Vec::new(); + for op in view.create.iter() { + let (slot, span) = match op { + CreateOp::ElementStart(elem) => (elem.slot, elem.base.source_span), + CreateOp::Element(elem) => (elem.slot, elem.base.source_span), + _ => continue, + }; + if let (Some(slot), Some(span)) = (slot, span) { + elements.push((slot.0, span.start)); } + } + if elements.is_empty() { + continue; + } - // Create SourceLocationOp for each element with location info - // Note: Angular creates a single op with an array, but our IR uses per-element ops - for loc in locations { - let source_op = CreateOp::SourceLocation(SourceLocationOp { - base: CreateOpBase::default(), - target: loc.target, - template_url: Ident::from(template_path.as_str()), - line: loc.line, - column: loc.column, - }); - view.create.push(source_op); - } + let offsets: std::vec::Vec = elements.iter().map(|&(_, offset)| offset).collect(); + let positions = source.locate(&offsets); + let mut locations = OxcVec::with_capacity_in(elements.len(), &allocator); + for (&(slot, _), (offset, line, column)) in elements.iter().zip(positions) { + locations.push(SourceLocation { slot, offset, line, column }); } + view.create.push(CreateOp::SourceLocation(SourceLocationOp { + base: CreateOpBase::default(), + template_path, + locations, + })); + } +} + +#[cfg(test)] +mod tests { + use super::TemplateSource; + + fn source(text: &str, start: u32) -> TemplateSource<'_> { + TemplateSource { text, start, start_line: 0, start_column: 0, raw_offsets: None } + } + + #[test] + fn counts_lines_like_the_angular_lexer() { + // `\r` moves no column; `\n` starts a line. + let text = "
\r\n \r"; + let positions = source(text, 0).locate(&[0, 9, 16]); + assert_eq!(positions, vec![(0, 0, 0), (9, 1, 2), (16, 1, 8)]); + } + + #[test] + fn counts_utf16_units() { + // `é` is one UTF-16 unit, `😀` two; both take more UTF-8 bytes. + let text = "é😀"; + let offset = u32::try_from(text.find("").unwrap()).unwrap(); + assert_eq!(source(text, 0).locate(&[offset]), vec![(3, 0, 3)]); + } + + #[test] + fn keeps_the_given_order() { + let text = "\n"; + assert_eq!(source(text, 0).locate(&[8, 0]), vec![(8, 1, 0), (0, 0, 0)]); } } diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/mod.rs b/crates/oxc_angular_compiler/src/pipeline/phases/mod.rs index 20fb89bd2..4d4b32461 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/mod.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/mod.rs @@ -13,7 +13,7 @@ mod allocate_slots; mod any_cast; mod apply_i18n_expressions; mod assign_i18n_slot_dependencies; -mod attach_source_locations; +pub(crate) mod attach_source_locations; mod attribute_extraction; mod binding_specialization; mod chaining; diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs b/crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs index 04b44f554..7742bb8d4 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs @@ -27,9 +27,9 @@ use crate::ir::expression::{ }; use crate::ir::ops::{CreateOp, UpdateOp, XrefId}; use crate::output::ast::{ - ArrowFunctionBody, ArrowFunctionExpr, FnParam, FunctionExpr, InvokeFunctionExpr, LiteralExpr, - LiteralValue, OutputExpression, OutputStatement, ReadPropExpr, ReadVarExpr, ReturnStatement, - clone_output_statement, + ArrowFunctionBody, ArrowFunctionExpr, FnParam, FunctionExpr, InvokeFunctionExpr, + LiteralArrayExpr, LiteralExpr, LiteralValue, OutputExpression, OutputStatement, ReadPropExpr, + ReadVarExpr, ReturnStatement, clone_output_statement, }; use crate::pipeline::compilation::{ ComponentCompilationJob, HostBindingCompilationJob, TemplateCompilationMode, @@ -1069,6 +1069,46 @@ fn reify_create_op<'a>( // Emit ɵɵcontrolCreate instruction for control binding initialization Some(create_control_create_stmt(allocator, core_namespace)) } + CreateOp::SourceLocation(op) => { + // `ɵɵattachSourceLocations(path, [[slot, offset, line, column], ...])`, + // as Angular's reify builds it from `op.locations`. + let number = |value: u32| { + OutputExpression::Literal(Box::new_in( + LiteralExpr { + value: LiteralValue::Number(f64::from(value)), + source_span: None, + }, + &allocator, + )) + }; + let mut locations = OxcVec::with_capacity_in(op.locations.len(), &allocator); + for location in &op.locations { + let mut entries = OxcVec::with_capacity_in(4, &allocator); + entries.push(number(location.slot)); + entries.push(number(location.offset)); + entries.push(number(location.line)); + entries.push(number(location.column)); + locations.push(OutputExpression::LiteralArray(Box::new_in( + LiteralArrayExpr { entries, source_span: None }, + &allocator, + ))); + } + let mut args = OxcVec::with_capacity_in(2, &allocator); + args.push(OutputExpression::Literal(Box::new_in( + LiteralExpr { value: LiteralValue::String(op.template_path), source_span: None }, + &allocator, + ))); + args.push(OutputExpression::LiteralArray(Box::new_in( + LiteralArrayExpr { entries: locations, source_span: None }, + &allocator, + ))); + Some(utils::create_instruction_call_stmt( + allocator, + core_namespace, + Identifiers::ATTACH_SOURCE_LOCATIONS, + args, + )) + } // Non-emitting operations CreateOp::ListEnd(_) | CreateOp::I18nMessage(_) @@ -1077,7 +1117,6 @@ fn reify_create_op<'a>( | CreateOp::IcuEnd(_) | CreateOp::IcuPlaceholder(_) | CreateOp::ExtractedAttribute(_) - | CreateOp::SourceLocation(_) | CreateOp::Statement(_) => None, } } diff --git a/crates/oxc_angular_compiler/tests/template_source_locations_test.rs b/crates/oxc_angular_compiler/tests/template_source_locations_test.rs new file mode 100644 index 000000000..15d30e7e8 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/template_source_locations_test.rs @@ -0,0 +1,343 @@ +//! Angular's `enableTemplateSourceLocations` option: each view's create +//! block ends with `ɵɵattachSourceLocations(path, [[slot, offset, line, +//! column], ...])`, and the `rootDirs` option makes paths project-relative. +//! +//! Every expected string below is ngtsc 22.2.3 full-mode output for the same +//! file (`enableTemplateSourceLocations: true`, `rootDirs` set to the file's +//! directory), with whitespace removed. Ported from Angular's +//! `attach_source_locations.ts` phase and compiler-cli's +//! `getProjectRelativePath`. + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{ + AngularVersion, CompilationMode, ResolvedResources, TransformOptions, transform_angular_file, +}; + +const ROOT: &str = "/w"; + +fn options() -> TransformOptions { + TransformOptions { + enable_template_source_locations: true, + root_dirs: Some(vec![ROOT.to_string()]), + ..Default::default() + } +} + +fn compile_with( + file: &str, + source: &str, + options: &TransformOptions, + resources: Option<&ResolvedResources>, +) -> String { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, file, source, Some(options), resources); + assert!(!result.has_errors(), "{:?}", result.diagnostics); + result.code.chars().filter(|c| !c.is_whitespace()).collect() +} + +fn compile(file: &str, source: &str) -> String { + compile_with(file, source, &options(), None) +} + +/// Each `ɵɵattachSourceLocations(...)` and `ɵsetClassDebugInfo(...)` call, in order. +fn calls(code: &str) -> Vec<&str> { + let mut found = Vec::new(); + let mut rest = code; + while let Some(start) = + ["attachSourceLocations(", "ɵsetClassDebugInfo("].iter().filter_map(|p| rest.find(p)).min() + { + // `ɵsetClassDebugInfo(Type, {...})` ends at its object, the other at `);`. + let (close, keep) = if rest[start..].starts_with("attach") { (");", 1) } else { ("})", 2) }; + let end = start + rest[start..].find(close).unwrap() + keep; + found.push(&rest[start..end]); + rest = &rest[end..]; + } + found +} + +const VIEWS: &str = r#"import { Component } from '@angular/core'; + +@Component({ + selector: 'app-views', + template: `
+ {{ name }} + @if (name) { hix } @else { no } + @for (item of items; track item) {
  • {{ item }}
    • } @empty {

    empty

    } + c + t + @switch (name) { @case ('a') { a } @default { d } } + @defer { later } @placeholder { p } + +
    +
    +@let x = 1; +

    {{ x }}

    `, +}) +export class ViewsComponent { + name = 'a'; + items = [1, 2]; +} + +@Component({ selector: 'app-two', template: '

    two

    ' }) +export class TwoComponent {} + +@Component({ selector: 'app-text', template: 'only text {{ 1 }}' }) +export class TextComponent {} +"#; + +#[test] +fn one_call_per_view_with_slots_offsets_lines_and_columns() { + let code = compile("/w/views.component.ts", VIEWS); + assert_eq!( + calls(&code), + [ + r#"attachSourceLocations("views.component.ts",[[0,208,6,15],[2,217,6,24]])"#, + r#"attachSourceLocations("views.component.ts",[[0,236,6,43]])"#, + r#"attachSourceLocations("views.component.ts",[[0,287,7,37]])"#, + r#"attachSourceLocations("views.component.ts",[[0,318,7,68]])"#, + r#"attachSourceLocations("views.component.ts",[[0,391,9,18]])"#, + r#"attachSourceLocations("views.component.ts",[[0,447,10,33]])"#, + r#"attachSourceLocations("views.component.ts",[[0,469,10,55]])"#, + r#"attachSourceLocations("views.component.ts",[[0,501,11,11]])"#, + r#"attachSourceLocations("views.component.ts",[[0,541,11,51]])"#, + r#"attachSourceLocations("views.component.ts",[[0,95,4,13],[2,118,5,2],[4,141,5,25],[11,349,8,16],[21,560,12,2],[22,565,12,7],[23,597,13,2],[24,621,16,0]])"#, + r#"ɵsetClassDebugInfo(ViewsComponent,{className:"ViewsComponent",filePath:"views.component.ts",lineNumber:19})"#, + r#"attachSourceLocations("views.component.ts",[[0,751,23,45]])"#, + r#"ɵsetClassDebugInfo(TwoComponent,{className:"TwoComponent",filePath:"views.component.ts",lineNumber:25})"#, + r#"ɵsetClassDebugInfo(TextComponent,{className:"TextComponent",filePath:"views.component.ts",lineNumber:28})"#, + ] + ); +} + +#[test] +fn the_call_ends_the_create_block_unchained() { + let code = compile("/w/views.component.ts", VIEWS); + // ngtsc: `ɵɵdomElementEnd(); ɵɵattachSourceLocations(...); } if (rf & 2) {` + // (oxc writes the test as `(rf & 2)`, as for any template). + assert!( + code.contains( + r#"i0.ɵɵdomElementEnd();i0.ɵɵattachSourceLocations("views.component.ts",[[0,95,4,13],"# + ), + "{code}" + ); + assert!(code.contains("[24,621,16,0]]);}if("), "{code}"); + assert!(code.contains("[[0,751,23,45]]);}},"), "{code}"); +} + +#[test] +fn escaped_literals_count_in_the_raw_source() { + // Offsets are UTF-16 units of the raw `.ts` text: `\n` does not start a + // line, and an escaped `<` (`\x3C`) sits at its backslash. + let source = r#"import { Component } from '@angular/core'; +// héllo 😀 +@Component({ selector: 'app-esc', template: 'é😀x\n

    \ty

    \x3Cb>z    \u00e9\u{1F600}\ +cont' }) +export class EscComponent {} + +@Component({ selector: 'app-esc2', template: "\"q\"\r\n\'" }) +export class Esc2Component {} + +@Component({ selector: 'app-esc3', template: `\`\n 😀\u{41}` }) +export class Esc3Component {} +"#; + assert_eq!( + calls(&compile("/w/esc.component.ts", source)), + [ + r#"attachSourceLocations("esc.component.ts",[[1,103,2,48],[3,113,2,58],[5,123,2,68],[8,140,2,85],[10,158,3,0]])"#, + r#"ɵsetClassDebugInfo(EscComponent,{className:"EscComponent",filePath:"esc.component.ts",lineNumber:5})"#, + r#"attachSourceLocations("esc.component.ts",[[0,254,6,46],[2,270,6,62]])"#, + r#"ɵsetClassDebugInfo(Esc2Component,{className:"Esc2Component",filePath:"esc.component.ts",lineNumber:8})"#, + r#"attachSourceLocations("esc.component.ts",[[0,361,9,46],[2,380,9,65]])"#, + r#"ɵsetClassDebugInfo(Esc3Component,{className:"Esc3Component",filePath:"esc.component.ts",lineNumber:11})"#, + ] + ); +} + +#[test] +fn crlf_source() { + let source = "import { Component } from '@angular/core';\r\n\r\n@Component({\r\n selector: 'app-crlf',\r\n template: `
    \r\n a\r\n
    `,\r\n})\r\nexport class CrlfComponent {}\r\n"; + assert_eq!( + calls(&compile("/w/crlf.component.ts", source)), + [ + r#"attachSourceLocations("crlf.component.ts",[[0,98,4,13],[1,107,5,2]])"#, + r#"ɵsetClassDebugInfo(CrlfComponent,{className:"CrlfComponent",filePath:"crlf.component.ts",lineNumber:9})"#, + ] + ); +} + +#[test] +fn template_url_counts_in_the_html_file() { + // `\r` moves no column, as in Angular's lexer cursor. + let source = r"import { Component } from '@angular/core'; + +@Component({ selector: 'app-url', templateUrl: './tpl/url.component.html' }) +export class UrlComponent {} + +@Component({ selector: 'app-url2', templateUrl: 'tpl/two.html' }) +export class Url2Component {} +"; + let mut resources = ResolvedResources::default(); + resources.templates.insert( + "./tpl/url.component.html".to_string(), + "
    \r\n é😀\r\n
    \n

    \rx

    \n".to_string(), + ); + resources + .templates + .insert("tpl/two.html".to_string(), "

    bom

    \n

    x

    ".to_string()); + assert_eq!( + calls(&compile_with("/w/url.component.ts", source, &options(), Some(&resources))), + [ + r#"attachSourceLocations("tpl/url.component.html",[[0,0,0,0],[1,9,1,2],[3,34,3,0],[4,38,3,3]])"#, + r#"ɵsetClassDebugInfo(UrlComponent,{className:"UrlComponent",filePath:"url.component.ts",lineNumber:4})"#, + r#"attachSourceLocations("tpl/two.html",[[0,0,0,0],[2,15,1,2]])"#, + r#"ɵsetClassDebugInfo(Url2Component,{className:"Url2Component",filePath:"url.component.ts",lineNumber:7})"#, + ] + ); +} + +#[test] +fn a_template_not_written_as_one_literal_counts_from_its_start() { + let source = r"import { Component } from '@angular/core'; + +const TPL = ` +
    x
    `; + +@Component({ selector: 'app-ind', template: TPL }) +export class IndComponent {} + +@Component({ selector: 'app-cat', template: 'a' + '\nb' }) +export class CatComponent {} + +@Component({ selector: 'app-paren', template: ('

    p

    ') }) +export class ParenComponent {} +"; + assert_eq!( + calls(&compile("/w/ind.component.ts", source)), + [ + r#"attachSourceLocations("ind.component.ts",[[0,3,1,2],[1,8,1,7]])"#, + r#"ɵsetClassDebugInfo(IndComponent,{className:"IndComponent",filePath:"ind.component.ts",lineNumber:7})"#, + r#"attachSourceLocations("ind.component.ts",[[0,0,0,0],[2,9,1,0]])"#, + r#"ɵsetClassDebugInfo(CatComponent,{className:"CatComponent",filePath:"ind.component.ts",lineNumber:10})"#, + r#"attachSourceLocations("ind.component.ts",[[0,0,0,0]])"#, + r#"ɵsetClassDebugInfo(ParenComponent,{className:"ParenComponent",filePath:"ind.component.ts",lineNumber:13})"#, + ] + ); +} + +#[test] +fn i18n_elements() { + let source = r#"import { Component } from '@angular/core'; + +@Component({ + selector: 'app-i18n', + template: `
    Hello world and {{ n }}
    +

    x

    +{n, plural, =1 {one} other {many}}`, +}) +export class I18nComponent { n = 1; } +"#; + assert_eq!( + calls(&compile("/w/i18n.component.ts", source)), + [ + r#"attachSourceLocations("i18n.component.ts",[[0,94,4,13],[2,110,4,29],[3,127,4,46],[4,148,5,0],[6,178,6,0]])"#, + r#"ɵsetClassDebugInfo(I18nComponent,{className:"I18nComponent",filePath:"i18n.component.ts",lineNumber:9})"#, + ] + ); +} + +const IN_SRC: &str = "import { Component } from '@angular/core'; + +@Component({ selector: 'app-in', template: '
    in
    ' }) +export class InComponent {} +"; + +const IN_LIB: &str = "import { Component } from '@angular/core'; + +@Component({ selector: 'app-out', template: '
    out
    ' }) +export class OutComponent {} +"; + +#[test] +fn paths_are_relative_to_the_first_root_that_holds_the_file() { + let roots = |dirs: &[&str]| TransformOptions { + root_dirs: Some(dirs.iter().map(ToString::to_string).collect()), + ..options() + }; + // rootDirs: [src]. A file outside every root gets no source locations and + // no `filePath`. + let options = roots(&["/w/src"]); + assert_eq!( + calls(&compile_with("/w/src/in.component.ts", IN_SRC, &options, None)), + [ + r#"attachSourceLocations("in.component.ts",[[0,88,2,44]])"#, + r#"ɵsetClassDebugInfo(InComponent,{className:"InComponent",filePath:"in.component.ts",lineNumber:4})"#, + ] + ); + assert_eq!( + calls(&compile_with("/w/lib/out.component.ts", IN_LIB, &options, None)), + [r#"ɵsetClassDebugInfo(OutComponent,{className:"OutComponent"})"#] + ); + // rootDirs: [lib, src]. + let options = roots(&["/w/lib", "/w/src"]); + assert_eq!( + calls(&compile_with("/w/lib/out.component.ts", IN_LIB, &options, None)), + [ + r#"attachSourceLocations("out.component.ts",[[0,89,2,45]])"#, + r#"ɵsetClassDebugInfo(OutComponent,{className:"OutComponent",filePath:"out.component.ts",lineNumber:4})"#, + ] + ); + // A root above the file keeps the directories in between. + assert!( + compile_with("/w/src/in.component.ts", IN_SRC, &roots(&["/w"]), None) + .contains(r#"attachSourceLocations("src/in.component.ts","#) + ); +} + +#[test] +fn off_by_default_and_without_a_root() { + let default = TransformOptions::default(); + let code = compile_with("/w/views.component.ts", VIEWS, &default, None); + assert!(!code.contains("attachSourceLocations"), "{code}"); + // With no root dirs, `filePath` stays the path given to the transform. + assert!(code.contains(r#"filePath:"/w/views.component.ts""#), "{code}"); + + // The option alone, with no root to make the path relative, emits nothing + // and changes nothing. + let no_root = TransformOptions { enable_template_source_locations: true, ..Default::default() }; + assert_eq!(compile_with("/w/views.component.ts", VIEWS, &no_root, None), code); + + // A root alone only changes `filePath`. + let root_only = + TransformOptions { root_dirs: Some(vec![ROOT.to_string()]), ..Default::default() }; + assert_eq!( + compile_with("/w/views.component.ts", VIEWS, &root_only, None), + code.replace(r#"filePath:"/w/views.component.ts""#, r#"filePath:"views.component.ts""#) + ); +} + +#[test] +fn partial_mode_emits_none() { + // The linker hard-codes `enableTemplateSourceLocations: false`. + let options = TransformOptions { compilation_mode: CompilationMode::Partial, ..options() }; + let code = compile_with("/w/views.component.ts", VIEWS, &options, None); + assert!(!code.contains("attachSourceLocations"), "{code}"); +} + +#[test] +fn needs_angular_19_1() { + let with_version = |version| TransformOptions { angular_version: Some(version), ..options() }; + let old = compile_with( + "/w/views.component.ts", + VIEWS, + &with_version(AngularVersion::new(19, 0, 5)), + None, + ); + assert!(!old.contains("attachSourceLocations"), "{old}"); + let new = compile_with( + "/w/views.component.ts", + VIEWS, + &with_version(AngularVersion::new(19, 1, 0)), + None, + ); + assert!(new.contains("attachSourceLocations"), "{new}"); +} diff --git a/napi/angular-compiler/index.d.ts b/napi/angular-compiler/index.d.ts index 3c8c52a2b..3d8de289a 100644 --- a/napi/angular-compiler/index.d.ts +++ b/napi/angular-compiler/index.d.ts @@ -1117,6 +1117,26 @@ export interface TransformOptions { * was loaded. Full compilation mode only. Defaults to `false`, as in ngtsc. */ forbidOrphanComponents?: boolean + /** + * The `enableTemplateSourceLocations` Angular compiler option: end each + * template view with `ɵɵattachSourceLocations(path, [[slot, offset, line, + * column], ...])`, which writes a `data-ng-source-location` attribute on + * each element (in production too). Full compilation only, Angular 19.1+. + * + * Needs `rootDirs`: with no root, nothing is emitted. Defaults to `false`. + */ + enableTemplateSourceLocations?: boolean + /** + * The `rootDirs` compiler option: the project roots that output paths are + * relative to, like ngtsc's `getProjectRelativePath`. Relative entries + * resolve against the working directory. + * + * When set, `ɵsetClassDebugInfo`'s `filePath` is relative to the first + * root that contains the file (and left out when none does). When unset, + * `filePath` is the path given to the transform. Paths are never + * lowercased (ngtsc lowercases them on case-insensitive file systems). + */ + rootDirs?: Array /** The CSS selector that identifies this component in a template. */ selector?: string /** diff --git a/napi/angular-compiler/src/lib.rs b/napi/angular-compiler/src/lib.rs index f5135906c..ebe0c7369 100644 --- a/napi/angular-compiler/src/lib.rs +++ b/napi/angular-compiler/src/lib.rs @@ -178,6 +178,24 @@ pub struct TransformOptions { /// was loaded. Full compilation mode only. Defaults to `false`, as in ngtsc. pub forbid_orphan_components: Option, + /// The `enableTemplateSourceLocations` Angular compiler option: end each + /// template view with `ɵɵattachSourceLocations(path, [[slot, offset, line, + /// column], ...])`, which writes a `data-ng-source-location` attribute on + /// each element (in production too). Full compilation only, Angular 19.1+. + /// + /// Needs `rootDirs`: with no root, nothing is emitted. Defaults to `false`. + pub enable_template_source_locations: Option, + + /// The `rootDirs` compiler option: the project roots that output paths are + /// relative to, like ngtsc's `getProjectRelativePath`. Relative entries + /// resolve against the working directory. + /// + /// When set, `ɵsetClassDebugInfo`'s `filePath` is relative to the first + /// root that contains the file (and left out when none does). When unset, + /// `filePath` is the path given to the transform. Paths are never + /// lowercased (ngtsc lowercases them on case-insensitive file systems). + pub root_dirs: Option>, + // Component metadata fields for full compilation testing // These mirror Angular's @Component decorator options. /// The CSS selector that identifies this component in a template. @@ -287,6 +305,10 @@ impl From for RustTransformOptions { .enable_i18n_legacy_message_id_format .unwrap_or(false), forbid_orphan_components: options.forbid_orphan_components.unwrap_or(false), + enable_template_source_locations: options + .enable_template_source_locations + .unwrap_or(false), + root_dirs: options.root_dirs, // Component metadata overrides selector: options.selector, standalone: options.standalone, From f79852e8870689a3d1803c855b958f9ac747a9a0 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 22:23:33 +0800 Subject: [PATCH 07/18] feat(vite): expose enableTemplateSourceLocations and rootDirs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Vite plugin exposes both options. `rootDirs` defaults to Vite's resolved `root`, so `ɵsetClassDebugInfo` `filePath` is now relative to it, as in an ngtsc build, instead of the absolute module id. The e2e compare runner passes the root dirs that match where ngtsc compiles each fixture, so `filePath` now matches and the global DEBUG_INFO_PATH known difference is gone (all 1174 fixtures pass). Refs #623. Co-Authored-By: Claude Opus 5.5 --- napi/angular-compiler/e2e/compare/README.md | 3 +- .../e2e/compare/fixtures/known-differences.ts | 6 +- .../e2e/compare/fixtures/runner.ts | 19 ++++-- .../e2e/compare/src/compilers/oxc.ts | 5 ++ .../test/source-locations.test.ts | 61 +++++++++++++++++++ napi/angular-compiler/vite-plugin/index.ts | 24 ++++++++ 6 files changed, 108 insertions(+), 10 deletions(-) create mode 100644 napi/angular-compiler/test/source-locations.test.ts diff --git a/napi/angular-compiler/e2e/compare/README.md b/napi/angular-compiler/e2e/compare/README.md index ba022f00d..1366dc86f 100644 --- a/napi/angular-compiler/e2e/compare/README.md +++ b/napi/angular-compiler/e2e/compare/README.md @@ -143,7 +143,8 @@ its own like Oxc does. Where that differs from a real build (for example `change class that also has a non-Angular decorator), a fixture sets `ngtscMode: 'full'`. ngtsc compiles each fixture under the project root (the compare directory), as a real build -does, so that its `ɵsetClassDebugInfo` calls carry `filePath` and `lineNumber`. +does, so that its `ɵsetClassDebugInfo` calls carry `filePath` and `lineNumber`. Oxc gets the +`rootDirs` that make its `filePath` the same project-relative path. ### Known Differences (Not Bugs) diff --git a/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts b/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts index 69513e6ec..61443d335 100644 --- a/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts +++ b/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts @@ -45,17 +45,13 @@ const ALIASED_INJECTABLE = const NG_MODULE_SCOPE = 'ɵɵsetNgModuleScope runs unguarded where Angular wraps it in `ngJitMode`: Oxc compiles one file at a time and resolves component dependencies at runtime through the scope it registers, so production builds (ngJitMode = false) still need it' -const DEBUG_INFO_PATH = - 'ɵsetClassDebugInfo filePath is the path Oxc was given (absolute here and in the Vite plugin) where Angular emits it relative to the project root' /** * Differences every fixture may have, matched against the labels `KnownDifference` uses. A * fixture whose differences are all documented here or in its own entry is reported as a known * difference with these reasons. They never go stale. */ -export const GLOBAL_KNOWN_DIFFERENCES: Array<{ label: RegExp; reason: string }> = [ - { label: /^statement ɵsetClassDebugInfo\([^()]+\)(#\d+)? filePath$/, reason: DEBUG_INFO_PATH }, -] +export const GLOBAL_KNOWN_DIFFERENCES: Array<{ label: RegExp; reason: string }> = [] export const KNOWN_DIFFERENCES: Record = { 'class-metadata/class-metadata-ngmodule': { diff --git a/napi/angular-compiler/e2e/compare/fixtures/runner.ts b/napi/angular-compiler/e2e/compare/fixtures/runner.ts index f990f68f0..8b674ae82 100644 --- a/napi/angular-compiler/e2e/compare/fixtures/runner.ts +++ b/napi/angular-compiler/e2e/compare/fixtures/runner.ts @@ -6,7 +6,7 @@ */ import { readFileSync } from 'node:fs' -import { dirname, join } from 'node:path' +import { dirname, isAbsolute, join } from 'node:path' import { fileURLToPath } from 'node:url' import { cpus } from 'os' @@ -50,6 +50,15 @@ function ngtscFilePath(filePath: string): string { return join(dirname(FIXTURE_TSCONFIG_PATH), filePath) } +/** + * The `rootDirs` Oxc compiles a fixture with, so that `filePath` relative to it is the path + * `ngtscFilePath` gives relative to the project root: `/` for `/fixtures/...`, the working + * directory for a relative path. + */ +function oxcRootDirs(filePath: string): string[] { + return [isAbsolute(filePath) ? '/' : process.cwd()] +} + /** * The ngtsc mode a fixture's reference output comes from: its `ngtscMode`, or else `full` when * the file has components and each is standalone and provably imports nothing, and `local` @@ -367,7 +376,7 @@ async function testComponentFixture(fixture: Fixture, verbose?: boolean): Promis let oxcOutput: FixtureCompilerOutput const oxcStart = performance.now() try { - const result = compileWithOxcFullFile(source, filePath) + const result = compileWithOxcFullFile(source, filePath, { rootDirs: oxcRootDirs(filePath) }) oxcOutput = { code: result.code, error: result.error, @@ -579,7 +588,9 @@ async function testFullTransformFixture( const oxcStart = performance.now() try { - const result = compileWithOxcFullFile(fixture.sourceCode, filePath) + const result = compileWithOxcFullFile(fixture.sourceCode, filePath, { + rootDirs: oxcRootDirs(filePath), + }) oxcOutput = { code: result.code, error: result.error, @@ -749,7 +760,7 @@ async function testClassMetadataFixture( let oxcOutput: FixtureCompilerOutput const oxcStart = performance.now() try { - const result = compileWithOxcFullFile(fixture.sourceCode, filePath) + const result = compileWithOxcFullFile(fixture.sourceCode, filePath, { rootDirs: [compareDir] }) oxcOutput = { code: result.code, error: result.error, diff --git a/napi/angular-compiler/e2e/compare/src/compilers/oxc.ts b/napi/angular-compiler/e2e/compare/src/compilers/oxc.ts index 42d485a82..6076f0d1b 100644 --- a/napi/angular-compiler/e2e/compare/src/compilers/oxc.ts +++ b/napi/angular-compiler/e2e/compare/src/compilers/oxc.ts @@ -57,6 +57,8 @@ export interface OxcFullFileRawOptions { resolvedResources?: ResolvedResources | null /** Path to tsconfig.json for resolving path aliases in cross-file analysis */ tsconfigPath?: string + /** Project roots that `ɵsetClassDebugInfo`'s `filePath` is made relative to, like ngtsc's `rootDirs` */ + rootDirs?: string[] } /** @@ -90,6 +92,7 @@ export function compileWithOxcFullFileRaw( tsconfigPath: options?.tsconfigPath, // Enable class metadata for TestBed support (matches Angular's output) emitClassMetadata: true, + rootDirs: options?.rootDirs, } try { @@ -247,6 +250,8 @@ export interface OxcProjectOptions { resolvedResourcesByFile?: Map /** Path to tsconfig.json for resolving path aliases in cross-file analysis */ tsconfigPath?: string + /** Project roots that `ɵsetClassDebugInfo`'s `filePath` is made relative to, like ngtsc's `rootDirs` */ + rootDirs?: string[] } /** diff --git a/napi/angular-compiler/test/source-locations.test.ts b/napi/angular-compiler/test/source-locations.test.ts new file mode 100644 index 000000000..c4c880f03 --- /dev/null +++ b/napi/angular-compiler/test/source-locations.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest' + +import { angular, type PluginOptions } from '../vite-plugin/index.js' + +const SOURCE = `import { Component } from '@angular/core'; + +@Component({ selector: 'app-root', template: '
    hi
    ' }) +export class AppComponent {} +` + +/** The plugin's output for `id`, after Vite resolved its config with `root`. */ +async function transform(options: PluginOptions, root: string, id: string): Promise { + const plugin = angular(options).find((candidate) => candidate.name === '@oxc-angular/vite') + if (!plugin?.transform || typeof plugin.transform === 'function') { + throw new Error('Expected plugin transform handler') + } + const configResolved = plugin.configResolved as unknown as + | { handler: (config: unknown) => void } + | ((config: unknown) => void) + const config = { root, build: {}, isProduction: false } + if (typeof configResolved === 'function') configResolved(config) + else configResolved?.handler(config) + + const result = await plugin.transform.handler.call( + { + error(message: string) { + throw new Error(message) + }, + warn() {}, + } as any, + SOURCE, + id, + ) + if (!result || typeof result !== 'object' || !('code' in result)) { + throw new Error('Expected transform result with code') + } + return (result.code as string).replace(/\s+/g, '') +} + +describe('@oxc-angular/vite project-relative paths', () => { + it('makes ɵsetClassDebugInfo filePath relative to the Vite root', async () => { + const code = await transform({}, '/proj', '/proj/src/app.component.ts') + expect(code).toContain('filePath:"src/app.component.ts"') + expect(code).not.toContain('attachSourceLocations') + }) + + it('uses the rootDirs option over the Vite root', async () => { + const code = await transform({ rootDirs: ['/proj/src'] }, '/proj', '/proj/src/app.component.ts') + expect(code).toContain('filePath:"app.component.ts"') + }) + + it('emits ɵɵattachSourceLocations with enableTemplateSourceLocations', async () => { + // As ngtsc 22.2.3 emits it for the same file, with the project directory as its root. + const code = await transform( + { enableTemplateSourceLocations: true }, + '/proj', + '/proj/src/app.component.ts', + ) + expect(code).toContain('ɵɵattachSourceLocations("src/app.component.ts",[[0,90,2,46]]);') + }) +}) diff --git a/napi/angular-compiler/vite-plugin/index.ts b/napi/angular-compiler/vite-plugin/index.ts index a4348f67f..ce7e3aadc 100644 --- a/napi/angular-compiler/vite-plugin/index.ts +++ b/napi/angular-compiler/vite-plugin/index.ts @@ -180,6 +180,23 @@ export interface PluginOptions { * info). Default: `false`, as in ngtsc. */ forbidOrphanComponents?: boolean + + /** + * Angular's `enableTemplateSourceLocations` compiler option: each template view + * calls `ɵɵattachSourceLocations`, which puts a `data-ng-source-location` + * attribute (`path@o:offset,l:line,c:column`) on every element. The call is not + * dev-mode guarded, so it also runs in production builds. Full compilation and + * Angular 19.1+ only. Default: `false`, as in ngtsc. + */ + enableTemplateSourceLocations?: boolean + + /** + * The `rootDirs` compiler option: the project roots that output paths are + * relative to (`ɵsetClassDebugInfo`'s `filePath`, the `ɵɵattachSourceLocations` + * path), like ngtsc's `getProjectRelativePath`. Relative entries resolve + * against `workspaceRoot`. Default: Vite's resolved `root`. + */ + rootDirs?: string[] } // Match all TypeScript files - we'll filter by @Component/@Directive decorator in the handler @@ -339,6 +356,8 @@ export function angular(options: PluginOptions = {}): Plugin[] { preserveWhitespaces: options.preserveWhitespaces, enableI18nLegacyMessageIdFormat: options.enableI18nLegacyMessageIdFormat ?? false, forbidOrphanComponents: options.forbidOrphanComponents ?? false, + enableTemplateSourceLocations: options.enableTemplateSourceLocations ?? false, + rootDirs: options.rootDirs?.map((dir) => resolve(workspaceRoot, dir)), tsconfig: options.tsconfig, } @@ -601,6 +620,11 @@ export function angular(options: PluginOptions = {}): Plugin[] { preserveWhitespaces: pluginOptions.preserveWhitespaces, enableI18nLegacyMessageIdFormat: pluginOptions.enableI18nLegacyMessageIdFormat, forbidOrphanComponents: pluginOptions.forbidOrphanComponents, + enableTemplateSourceLocations: pluginOptions.enableTemplateSourceLocations, + // ngtsc makes output paths relative to its root dirs (`rootDir`, else the + // working directory); Vite's `root` stands in for them by default. + rootDirs: + pluginOptions.rootDirs ?? (resolvedConfig?.root ? [resolvedConfig.root] : undefined), // Evaluate decorator metadata imported from other files the way // ngtsc's program-wide checker does (`inputs: INPUTS`, `@Input(OPTS)`, // `templateUrl: TEMPLATE_URL`). Read files land in From 11dfea30d83d32fed5ae216f91c122c786f105a3 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 22:34:59 +0800 Subject: [PATCH 08/18] =?UTF-8?q?test(e2e):=20list=20=C9=B5inj's=20declare?= =?UTF-8?q?d-exports=20filtering=20as=20a=20known=20difference?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compare oracle runs ngtsc in local mode, which keeps an exported declaration in ɵinj. A full ngtsc build leaves it out, as Oxc now does. Co-Authored-By: Claude Opus 5.5 --- .../e2e/compare/fixtures/known-differences.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts b/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts index 61443d335..608387495 100644 --- a/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts +++ b/napi/angular-compiler/e2e/compare/fixtures/known-differences.ts @@ -46,6 +46,9 @@ const ALIASED_INJECTABLE = const NG_MODULE_SCOPE = 'ɵɵsetNgModuleScope runs unguarded where Angular wraps it in `ngJitMode`: Oxc compiles one file at a time and resolves component dependencies at runtime through the scope it registers, so production builds (ngJitMode = false) still need it' +const INJECTOR_EXPORTS = + 'ɵinj leaves out an `exports` entry that `declarations` also lists, as a full ngtsc build does (a declaration is a component, directive or pipe, which gives no providers); ngtsc local mode keeps it' + /** * Differences every fixture may have, matched against the labels `KnownDifference` uses. A * fixture whose differences are all documented here or in its own entry is reported as a known @@ -75,9 +78,9 @@ export const KNOWN_DIFFERENCES: Record = { reasons: [NG_MODULE_SCOPE], }, 'injector/injector-with-exports': { - fields: [], + fields: ['ExportingModule.ɵinj'], statements: ['ɵɵsetNgModuleScope(ExportingModule)'], - reasons: [NG_MODULE_SCOPE], + reasons: [NG_MODULE_SCOPE, INJECTOR_EXPORTS], }, 'animations/animation-metadata-with-change-detection': { fields: ['AnimationsWithOnPushComponent.ɵcmp'], From 47f160117c5491de955e9e9eb711c4cab7171747 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 23:07:53 +0800 Subject: [PATCH 09/18] fix(metadata): count only exported namespace const enums `NS.E` reads an exported member. A const enum one `namespace NS` block keeps to itself hid an exported `class E` of another block, so the class's ctor param lost its type (ngtsc keeps `type: NS.E`). Co-Authored-By: Claude Opus 5.5 --- .../src/class_metadata/builders.rs | 26 +++++++++++++---- .../tests/const_enum_ctor_param_test.rs | 29 +++++++++++++++++++ 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/crates/oxc_angular_compiler/src/class_metadata/builders.rs b/crates/oxc_angular_compiler/src/class_metadata/builders.rs index aefcfe3dd..c397ce648 100644 --- a/crates/oxc_angular_compiler/src/class_metadata/builders.rs +++ b/crates/oxc_angular_compiler/src/class_metadata/builders.rs @@ -1455,20 +1455,36 @@ impl<'a> LocalDecls<'a> { decl: &'a oxc_ast::ast::TSNamespaceDeclaration<'a>, globals: &mut std::vec::Vec<&'a Statement<'a>>, ) { - self.namespaces - .entry(decl.id.name.as_str()) - .or_default() - .merge(Self::from_namespace_body(&decl.body, globals)); + self.namespaces.entry(decl.id.name.as_str()).or_default().merge(Self::from_namespace_body( + &decl.body, + decl.declare, + globals, + )); } + /// `ambient` is whether the namespace is `declare`d, where every member + /// is visible from outside it. fn from_namespace_body( body: &'a TSNamespaceDeclarationBody<'a>, + ambient: bool, globals: &mut std::vec::Vec<&'a Statement<'a>>, ) -> Self { match body { TSNamespaceDeclarationBody::TSModuleBlock(block) => { let mut decls = LocalDecls::default(); decls.insert_all(&block.body, globals); + // `NS.E` reads an exported member only: a const enum the block + // keeps to itself must not hide an exported `E` that another + // `namespace NS` block declares. + if !ambient { + decls.const_enums.retain(|name| { + block.body.iter().any(|stmt| { + matches!(stmt, Statement::ExportDeclaration(export) + if matches!(&export.declaration, + Declaration::TSEnumDeclaration(e) if e.id.name == *name)) + }) + }); + } decls } // `namespace Outer.Inner { ... }` nests by qualified name. @@ -1476,7 +1492,7 @@ impl<'a> LocalDecls<'a> { let mut decls = LocalDecls::default(); decls.namespaces.insert( nested.id.name.as_str(), - Self::from_namespace_body(&nested.body, globals), + Self::from_namespace_body(&nested.body, ambient || nested.declare, globals), ); decls } diff --git a/crates/oxc_angular_compiler/tests/const_enum_ctor_param_test.rs b/crates/oxc_angular_compiler/tests/const_enum_ctor_param_test.rs index 6d86633a7..3e4a00ce9 100644 --- a/crates/oxc_angular_compiler/tests/const_enum_ctor_param_test.rs +++ b/crates/oxc_angular_compiler/tests/const_enum_ctor_param_test.rs @@ -376,3 +376,32 @@ export class Svc { } } } + +#[test] +fn a_const_enum_kept_inside_one_namespace_block_does_not_hide_an_exported_class() { + // ngtsc 22.2.3: `type: NS.E`. Only the class is `NS.E` from outside. + let source = "import { Injectable } from '@angular/core'; +export namespace NS { const enum E { A = 1 } export const x = E.A; } +export namespace NS { export class E {} } +@Injectable() +export class Svc { constructor(e: NS.E) {} } +"; + for mode in MODES { + let code = compile(source, mode); + assert_eq!(ctor_params(&code), "[{type:NS.E}]", "{mode:?}"); + } +} + +#[test] +fn an_exported_namespace_const_enum_is_still_undefined() { + let source = "import { Injectable } from '@angular/core'; +export namespace NS { export const enum E { A = 1 } } +declare namespace Amb { const enum F { A = 1 } } +@Injectable() +export class Svc { constructor(e: NS.E, f: Amb.F) {} } +"; + for mode in MODES { + let code = compile(source, mode); + assert_eq!(ctor_params(&code), "[{type:undefined},{type:undefined}]", "{mode:?}"); + } +} From ee47302e828a1d840d4b9915ebc7f92ef1104fdf Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 23:07:53 +0800 Subject: [PATCH 10/18] fix(ngmodule): keep an MWP entry whose module this file declares later MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Another file's ModuleWithProviders could name a class declared after the NgModule, and the partial ɵmod then read it before it was initialized (TDZ ReferenceError). Apply the same-file check: such an entry keeps today's name. Co-Authored-By: Claude Opus 5.5 --- .../src/ng_module/module_with_providers.rs | 16 ++++++-- .../tests/partial_ng_module_mwp_test.rs | 39 +++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs b/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs index ab2b7dda9..6e95d0e48 100644 --- a/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs +++ b/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs @@ -421,12 +421,20 @@ pub fn partial_imports<'a>( } if !questions.is_empty() { for (index, answer) in asked.into_iter().zip(resolve_imported(&questions)) { - resolved[index] = answer.map(|class| match class { + resolved[index] = answer.and_then(|module| match module { + // As for a same-file entry: a class declared after this + // NgModule isn't initialized yet when its `ɵmod` is (ngtsc + // wraps the list in `() => [...]` then), and an import + // must stay. Otherwise today's name is kept. NamedClass::Local(name) => { - read_var(oxc_str::Ident::from(allocator.alloc_str(&name))) + let declared_before = top_level_class(program, &name) + .is_some_and(|module| module.span.end <= class.span.start); + let is_kept = matches!(binding(&name), Binding::Import { kept: true }); + (declared_before || is_kept) + .then(|| read_var(oxc_str::Ident::from(allocator.alloc_str(&name)))) } NamedClass::Module { specifier, name } => { - OutputExpression::ReadProp(Box::new_in( + Some(OutputExpression::ReadProp(Box::new_in( ReadPropExpr { receiver: Box::new_in( read_var(namespace_of(&specifier)), @@ -437,7 +445,7 @@ pub fn partial_imports<'a>( source_span: None, }, &allocator, - )) + ))) } }); } diff --git a/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs b/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs index 4fa44d0f9..93afcb2fb 100644 --- a/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs +++ b/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs @@ -254,6 +254,45 @@ export class AppCmp {} export class AppModule {} "; + #[test] + fn a_module_declared_later_in_this_file_keeps_its_old_output() { + // Another file's `ModuleWithProviders` names a class this file + // declares after the NgModule. Listing `Later` would read it before it + // is initialized (ngtsc wraps the list in `() => [...]`), so the + // entry stays as written. + let dir = TempDir::new().unwrap(); + write( + &dir, + "holder.ts", + r" +import { ModuleWithProviders } from '@angular/core'; +import type { Later } from './app'; +export class Holder { + static forRoot(): ModuleWithProviders { + const providers: any[] = []; + return { ngModule: null as any, providers }; + } +} +export const LATER_ROOT = Holder.forRoot(); +", + ); + let code = compile( + &dir, + "app.ts", + r" +import { NgModule } from '@angular/core'; +import { Holder, LATER_ROOT } from './holder'; +@NgModule({ imports: [Holder.forRoot(), LATER_ROOT] }) +export class M {} +@NgModule({}) +export class Later {} +", + CompilationMode::Partial, + ); + assert!(code.contains("type:M,imports:[Holder,LATER_ROOT]}"), "{code}"); + assert!(!code.contains("imports:[Later"), "{code}"); + } + #[test] fn imported_method_and_constant_name_the_module_through_an_import() { // ngtsc partial: `import * as i1 from "./dep"` and From 170d22720fdb14f628c19f08785a8cd654c88bc2 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 23:07:53 +0800 Subject: [PATCH 11/18] test(ngmodule): cover an exported component outside a readable scope Co-Authored-By: Claude Opus 5.5 --- .../tests/ng_module_injector_imports_test.rs | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs b/crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs index 69ccb86ad..95e9a5032 100644 --- a/crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs +++ b/crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs @@ -414,8 +414,8 @@ export class PkgModule {} #[test] fn unresolved_imports_stay() { - // A missing file, an `export *` that doesn't have the name, and an - // exported component that isn't in the module's scope. + // A missing file and an `export *` that doesn't have the name; the + // scope can't be read either, so the exported component stays. let dir = TempDir::new().unwrap(); dep(&dir); write(&dir, "src/barrel.ts", "export * from './dep';\n"); @@ -434,6 +434,25 @@ export class M {} ); } + #[test] + fn an_exported_component_outside_a_readable_scope_stays() { + // ngtsc rejects this module, but if the component's own imports give + // providers, leaving it out would lose them: it stays. + let dir = TempDir::new().unwrap(); + dep(&dir); + check( + &dir, + r" +import { NgModule } from '@angular/core'; +import { PlainModule, StandaloneCmp } from './dep'; +@NgModule({ imports: [PlainModule], exports: [StandaloneCmp] }) +export class M {} +", + &[("M", Some("PlainModule,StandaloneCmp"))], + &cross_file(), + ); + } + #[test] fn without_imported_values_imported_classes_stay() { let dir = TempDir::new().unwrap(); From 75b9b9d66d379cd242b550caa0c1552123089567 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 23:07:53 +0800 Subject: [PATCH 12/18] fix(vite): default rootDirs to workspaceRoot, like ngtsc's cwd fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With Vite's root, a monorepo library outside the app lost filePath and lineNumber in ɵsetClassDebugInfo. ngtsc falls back to the working directory (the workspace root). Also list the new options in the README. Co-Authored-By: Claude Opus 5.5 --- napi/angular-compiler/README.md | 13 ++++++++++ .../test/source-locations.test.ts | 24 +++++++++++++++---- napi/angular-compiler/vite-plugin/index.ts | 10 ++++---- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/napi/angular-compiler/README.md b/napi/angular-compiler/README.md index c5bf62d04..152250d0d 100644 --- a/napi/angular-compiler/README.md +++ b/napi/angular-compiler/README.md @@ -182,6 +182,12 @@ interface TransformOptions { // Default for components that do not set their own (project-wide option) preserveWhitespaces?: boolean + // Angular compiler options of the same name (default: false / unset) + forbidOrphanComponents?: boolean + enableTemplateSourceLocations?: boolean // needs rootDirs; full mode only + // Roots that ɵsetClassDebugInfo's filePath and source locations are relative to + rootDirs?: string[] + // Cross-file elision crossFileElision?: boolean // Evaluate decorator metadata imported from other files (as ngtsc does) @@ -227,6 +233,13 @@ interface PluginOptions { // Project-wide default (`angularCompilerOptions.preserveWhitespaces`); // a component's own `preserveWhitespaces` wins (default: false) preserveWhitespaces?: boolean + + // Angular compiler options of the same name (default: false) + forbidOrphanComponents?: boolean + enableTemplateSourceLocations?: boolean + // Roots for ɵsetClassDebugInfo's filePath and source locations, + // resolved against workspaceRoot (default: [workspaceRoot], like ngtsc) + rootDirs?: string[] } ``` diff --git a/napi/angular-compiler/test/source-locations.test.ts b/napi/angular-compiler/test/source-locations.test.ts index c4c880f03..d6a2f1e49 100644 --- a/napi/angular-compiler/test/source-locations.test.ts +++ b/napi/angular-compiler/test/source-locations.test.ts @@ -38,21 +38,35 @@ async function transform(options: PluginOptions, root: string, id: string): Prom } describe('@oxc-angular/vite project-relative paths', () => { - it('makes ɵsetClassDebugInfo filePath relative to the Vite root', async () => { - const code = await transform({}, '/proj', '/proj/src/app.component.ts') + it('makes ɵsetClassDebugInfo filePath relative to the workspace root', async () => { + const code = await transform({ workspaceRoot: '/proj' }, '/proj', '/proj/src/app.component.ts') expect(code).toContain('filePath:"src/app.component.ts"') expect(code).not.toContain('attachSourceLocations') }) - it('uses the rootDirs option over the Vite root', async () => { - const code = await transform({ rootDirs: ['/proj/src'] }, '/proj', '/proj/src/app.component.ts') + it('keeps filePath for a library outside the app root of a monorepo', async () => { + // ngtsc's root is the workspace (its working directory), not the app. + const code = await transform( + { workspaceRoot: '/mono' }, + '/mono/apps/app', + '/mono/libs/ui/app.component.ts', + ) + expect(code).toContain('filePath:"libs/ui/app.component.ts",lineNumber:4') + }) + + it('uses the rootDirs option over the workspace root', async () => { + const code = await transform( + { workspaceRoot: '/proj', rootDirs: ['/proj/src'] }, + '/proj', + '/proj/src/app.component.ts', + ) expect(code).toContain('filePath:"app.component.ts"') }) it('emits ɵɵattachSourceLocations with enableTemplateSourceLocations', async () => { // As ngtsc 22.2.3 emits it for the same file, with the project directory as its root. const code = await transform( - { enableTemplateSourceLocations: true }, + { enableTemplateSourceLocations: true, workspaceRoot: '/proj' }, '/proj', '/proj/src/app.component.ts', ) diff --git a/napi/angular-compiler/vite-plugin/index.ts b/napi/angular-compiler/vite-plugin/index.ts index ce7e3aadc..a1d5de090 100644 --- a/napi/angular-compiler/vite-plugin/index.ts +++ b/napi/angular-compiler/vite-plugin/index.ts @@ -194,7 +194,9 @@ export interface PluginOptions { * The `rootDirs` compiler option: the project roots that output paths are * relative to (`ɵsetClassDebugInfo`'s `filePath`, the `ɵɵattachSourceLocations` * path), like ngtsc's `getProjectRelativePath`. Relative entries resolve - * against `workspaceRoot`. Default: Vite's resolved `root`. + * against `workspaceRoot`. Default: `[workspaceRoot]`, like ngtsc, which falls + * back to the working directory (in an Angular CLI build, the workspace root), + * so library files of a monorepo outside the app's Vite `root` still get one. */ rootDirs?: string[] } @@ -622,9 +624,9 @@ export function angular(options: PluginOptions = {}): Plugin[] { forbidOrphanComponents: pluginOptions.forbidOrphanComponents, enableTemplateSourceLocations: pluginOptions.enableTemplateSourceLocations, // ngtsc makes output paths relative to its root dirs (`rootDir`, else the - // working directory); Vite's `root` stands in for them by default. - rootDirs: - pluginOptions.rootDirs ?? (resolvedConfig?.root ? [resolvedConfig.root] : undefined), + // working directory); `workspaceRoot` (the working directory by default) + // stands in for them. + rootDirs: pluginOptions.rootDirs ?? [pluginOptions.workspaceRoot], // Evaluate decorator metadata imported from other files the way // ngtsc's program-wide checker does (`inputs: INPUTS`, `@Input(OPTS)`, // `templateUrl: TEMPLATE_URL`). Read files land in From 3b4d52df79ee328dcd2184f00723a41f2a52f4ce Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 23:14:35 +0800 Subject: [PATCH 13/18] fix(compiler): resolve a templateUrl of a bare file name against the working directory A component file with no directory gave `/./x.html`, a path from the root, so no source location was emitted. Co-Authored-By: Claude Opus 5.5 --- .../src/component/source_locations.rs | 9 +++++++-- .../tests/template_source_locations_test.rs | 13 +++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/source_locations.rs b/crates/oxc_angular_compiler/src/component/source_locations.rs index 28a6df239..f064ded5f 100644 --- a/crates/oxc_angular_compiler/src/component/source_locations.rs +++ b/crates/oxc_angular_compiler/src/component/source_locations.rs @@ -110,8 +110,13 @@ pub(super) fn template_source<'a>( return None; } let file = file_path.replace('\\', "/"); - let dir = file.rfind('/').map_or("", |i| &file[..i]); - let path = project_relative_path(&format!("{dir}/{url}"), root_dirs)?; + // A bare file name has no directory: the URL is relative to the + // working directory, not to `/`. + let template_file = match file.rfind('/') { + Some(i) => format!("{}/{url}", &file[..i]), + None => url, + }; + let path = project_relative_path(&template_file, root_dirs)?; return Some((Ident::from(allocator.alloc_str(&path)), whole(template))); } diff --git a/crates/oxc_angular_compiler/tests/template_source_locations_test.rs b/crates/oxc_angular_compiler/tests/template_source_locations_test.rs index 15d30e7e8..a6d0b6790 100644 --- a/crates/oxc_angular_compiler/tests/template_source_locations_test.rs +++ b/crates/oxc_angular_compiler/tests/template_source_locations_test.rs @@ -195,6 +195,19 @@ export class Url2Component {} ); } +#[test] +fn template_url_of_a_bare_file_name_is_relative_to_the_working_directory() { + let source = "import { Component } from '@angular/core'; +@Component({ selector: 'app-bare', templateUrl: './bare.component.html' }) +export class BareComponent {} +"; + let mut resources = ResolvedResources::default(); + resources.templates.insert("./bare.component.html".to_string(), "

    x

    ".to_string()); + let options = TransformOptions { root_dirs: Some(vec![".".to_string()]), ..options() }; + let code = compile_with("bare.component.ts", source, &options, Some(&resources)); + assert_eq!(calls(&code)[0], r#"attachSourceLocations("bare.component.html",[[0,0,0,0]])"#); +} + #[test] fn a_template_not_written_as_one_literal_counts_from_its_start() { let source = r"import { Component } from '@angular/core'; From 001bfb9f21dcbee15be1e2d49c5dc40c911df8f6 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 23:19:57 +0800 Subject: [PATCH 14/18] test(vite): build the source-location test paths for the running OS On Windows the plugin resolves `rootDirs` to `D:\proj\src`, which a bare `/proj/...` module id never matches. Co-Authored-By: Claude Opus 5.5 --- .../test/source-locations.test.ts | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/napi/angular-compiler/test/source-locations.test.ts b/napi/angular-compiler/test/source-locations.test.ts index d6a2f1e49..df4a3b9d8 100644 --- a/napi/angular-compiler/test/source-locations.test.ts +++ b/napi/angular-compiler/test/source-locations.test.ts @@ -1,3 +1,5 @@ +import { resolve } from 'node:path' + import { describe, expect, it } from 'vitest' import { angular, type PluginOptions } from '../vite-plugin/index.js' @@ -8,6 +10,9 @@ const SOURCE = `import { Component } from '@angular/core'; export class AppComponent {} ` +/** `path` as an absolute path of this OS, with `/` like a Vite module id. */ +const abs = (path: string) => resolve(path).replace(/\\/g, '/') + /** The plugin's output for `id`, after Vite resolved its config with `root`. */ async function transform(options: PluginOptions, root: string, id: string): Promise { const plugin = angular(options).find((candidate) => candidate.name === '@oxc-angular/vite') @@ -39,7 +44,11 @@ async function transform(options: PluginOptions, root: string, id: string): Prom describe('@oxc-angular/vite project-relative paths', () => { it('makes ɵsetClassDebugInfo filePath relative to the workspace root', async () => { - const code = await transform({ workspaceRoot: '/proj' }, '/proj', '/proj/src/app.component.ts') + const code = await transform( + { workspaceRoot: abs('/proj') }, + abs('/proj'), + abs('/proj/src/app.component.ts'), + ) expect(code).toContain('filePath:"src/app.component.ts"') expect(code).not.toContain('attachSourceLocations') }) @@ -47,18 +56,18 @@ describe('@oxc-angular/vite project-relative paths', () => { it('keeps filePath for a library outside the app root of a monorepo', async () => { // ngtsc's root is the workspace (its working directory), not the app. const code = await transform( - { workspaceRoot: '/mono' }, - '/mono/apps/app', - '/mono/libs/ui/app.component.ts', + { workspaceRoot: abs('/mono') }, + abs('/mono/apps/app'), + abs('/mono/libs/ui/app.component.ts'), ) expect(code).toContain('filePath:"libs/ui/app.component.ts",lineNumber:4') }) it('uses the rootDirs option over the workspace root', async () => { const code = await transform( - { workspaceRoot: '/proj', rootDirs: ['/proj/src'] }, - '/proj', - '/proj/src/app.component.ts', + { workspaceRoot: abs('/proj'), rootDirs: [abs('/proj/src')] }, + abs('/proj'), + abs('/proj/src/app.component.ts'), ) expect(code).toContain('filePath:"app.component.ts"') }) @@ -66,9 +75,9 @@ describe('@oxc-angular/vite project-relative paths', () => { it('emits ɵɵattachSourceLocations with enableTemplateSourceLocations', async () => { // As ngtsc 22.2.3 emits it for the same file, with the project directory as its root. const code = await transform( - { enableTemplateSourceLocations: true, workspaceRoot: '/proj' }, - '/proj', - '/proj/src/app.component.ts', + { enableTemplateSourceLocations: true, workspaceRoot: abs('/proj') }, + abs('/proj'), + abs('/proj/src/app.component.ts'), ) expect(code).toContain('ɵɵattachSourceLocations("src/app.component.ts",[[0,90,2,46]]);') }) From a7d6bdf8f6906387d7a32e069b6cc4b527fb85e4 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 23:23:35 +0800 Subject: [PATCH 15/18] fix(compiler): only unwrap forwardRef imported from @angular/core in cross-file lists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A local function named `forwardRef` may return any class, so a `const` list built with it could be dropped from ɵinj with its providers. An aliased `forwardRef` from @angular/core is now read too, as ngtsc does. Co-Authored-By: Claude Opus 5.5 --- .../cross_file_elision/ng_module_scope.rs | 29 ++++++---- .../tests/ng_module_injector_imports_test.rs | 53 +++++++++++++++++++ 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs b/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs index 070e8d264..2c92ccf3c 100644 --- a/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs +++ b/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs @@ -584,7 +584,7 @@ impl Inner { /// `ngModule` of the object it returns. fn module_with_providers(&self, decl: &Decl, method: &str) -> Option { let parsed = self.parsed_file(&decl.file)?; - parsed.with(|_, program, _| { + parsed.with(|_, program, consts| { let class = find_class(program, &decl.name)?; let function = class.body.body.iter().find_map(|element| match element { ClassElement::MethodDefinition(m) @@ -608,7 +608,7 @@ impl Inner { _ => None, })?; let mut refs = Vec::new(); - collect_refs(returned, &decl.file, &mut refs, 0)?; + collect_refs(returned, consts, &decl.file, &mut refs, 0)?; match <[RawRef; 1]>::try_from(refs) { Ok([module]) => Some(module), Err(_) => None, @@ -1042,7 +1042,7 @@ fn read_declared<'a>( return None; } let mut refs = Vec::new(); - collect_refs(var.init.as_ref()?, &decl.file, &mut refs, 0)?; + collect_refs(var.init.as_ref()?, consts, &decl.file, &mut refs, 0)?; Some(Declared::List(refs)) }) } @@ -1182,7 +1182,7 @@ fn read_decorator<'a>( let list = |key: &str| -> Option> { let mut refs = Vec::new(); if let Some(value) = field(key) { - collect_refs(value, file, &mut refs, 0)?; + collect_refs(value, consts, file, &mut refs, 0)?; } Some(refs) }; @@ -1201,6 +1201,7 @@ fn read_decorator<'a>( /// anything else. fn collect_refs( expr: &Expression<'_>, + consts: &StringConsts<'_>, file: &str, out: &mut Vec, depth: u16, @@ -1219,10 +1220,10 @@ fn collect_refs( for element in &array.elements { match element { ArrayExpressionElement::SpreadElement(spread) => { - collect_refs(&spread.argument, file, out, depth + 1)?; + collect_refs(&spread.argument, consts, file, out, depth + 1)?; } ArrayExpressionElement::Elision(_) => return None, - _ => collect_refs(element.as_expression()?, file, out, depth + 1)?, + _ => collect_refs(element.as_expression()?, consts, file, out, depth + 1)?, } } } @@ -1234,8 +1235,16 @@ fn collect_refs( out.push(raw(Some(&object.name), &member.property.name, None)); } Expression::CallExpression(call) => match call.callee.get_inner_expression() { - // `forwardRef(() => A)` - Expression::Identifier(callee) if callee.name == "forwardRef" => { + // `forwardRef(() => A)`, with `forwardRef` imported from + // `@angular/core`: another function may return anything. + Expression::Identifier(callee) + if crate::directive::angular_core_identifier( + &callee.name, + Some(consts), + &["forwardRef"], + ) + .is_some() => + { let returned = match call.arguments.first()?.as_expression()?.get_inner_expression() { Expression::ArrowFunctionExpression(arrow) => arrow.get_expression()?, @@ -1247,7 +1256,7 @@ fn collect_refs( } _ => return None, }; - collect_refs(returned, file, out, depth + 1)?; + collect_refs(returned, consts, file, out, depth + 1)?; } // `A.forRoot()`, `ns.A.forRoot()` Expression::StaticMemberExpression(member) => { @@ -1274,7 +1283,7 @@ fn collect_refs( } _ => None, })?; - collect_refs(module, file, out, depth + 1)?; + collect_refs(module, consts, file, out, depth + 1)?; } _ => return None, } diff --git a/crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs b/crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs index 95e9a5032..a54585efa 100644 --- a/crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs +++ b/crates/oxc_angular_compiler/tests/ng_module_injector_imports_test.rs @@ -453,6 +453,59 @@ export class M {} ); } + #[test] + fn a_list_through_a_local_forward_ref_stays() { + // This `forwardRef` returns `PlainModule`, not what its callback gives. + let dir = TempDir::new().unwrap(); + dep(&dir); + write( + &dir, + "src/list.ts", + r" +import { PlainModule, StandaloneDir } from './dep'; +function forwardRef(fn: () => unknown): any { return PlainModule; } +export const IMPORTS = [forwardRef(() => StandaloneDir)]; +", + ); + check( + &dir, + r" +import { NgModule } from '@angular/core'; +import { IMPORTS } from './list'; +@NgModule({ imports: [IMPORTS] }) +export class M {} +", + &[("M", Some("IMPORTS"))], + &cross_file(), + ); + } + + #[test] + fn a_list_through_angular_forward_ref_is_left_out() { + let dir = TempDir::new().unwrap(); + dep(&dir); + write( + &dir, + "src/list.ts", + r" +import { forwardRef as ref } from '@angular/core'; +import { StandaloneDir } from './dep'; +export const IMPORTS = [ref(() => StandaloneDir)]; +", + ); + check( + &dir, + r" +import { NgModule } from '@angular/core'; +import { IMPORTS } from './list'; +@NgModule({ imports: [IMPORTS] }) +export class M {} +", + &[("M", None)], + &cross_file(), + ); + } + #[test] fn without_imported_values_imported_classes_stay() { let dir = TempDir::new().unwrap(); From ca81af7dbd5d215961bcfb5bf39810688cbcaff8 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 2 Nov 2026 23:32:50 +0800 Subject: [PATCH 16/18] fix(compiler): keep a ModuleWithProviders whose ngModule is a member of a parameter `return { ngModule: modules.X }` reads the call's argument when `modules` is a parameter, not a namespace import of the same name. Co-Authored-By: Claude Opus 5.5 --- .../src/ng_module/module_with_providers.rs | 7 +++- .../tests/partial_ng_module_mwp_test.rs | 37 ++++++++++++++++++- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs b/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs index 6e95d0e48..ae239f4ee 100644 --- a/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs +++ b/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs @@ -70,7 +70,12 @@ pub fn static_method_ng_module<'b, 'a>( && let [Statement::ReturnStatement(ret)] = body.statements.as_slice() { let value = ng_module_value(ret.argument.as_ref()?)?; - if let Expression::Identifier(id) = value + // `param` or `param.X`: the module comes from the call's argument. + let mut root = value; + while let Expression::StaticMemberExpression(member) = root { + root = member.object.get_inner_expression(); + } + if let Expression::Identifier(id) = root && function.params.items.iter().any(|param| { param.pattern.get_binding_identifiers().iter().any(|b| b.name == id.name) }) diff --git a/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs b/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs index 93afcb2fb..e64c097a7 100644 --- a/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs +++ b/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs @@ -160,17 +160,19 @@ fn a_parameter_or_type_parameter_keeps_its_old_output() { let code = partial( r" import { NgModule, ModuleWithProviders, Type } from '@angular/core'; +import * as m from './dep'; @NgModule({}) export class AModule {} export class Holder { static of(ngModule: Type) { return { ngModule, providers: [] }; } static generic(): ModuleWithProviders { const m: any = null; return m; } + static member(m: { DepModule: Type }) { return { ngModule: m.DepModule }; } } -@NgModule({ imports: [Holder.of(AModule), Holder.generic()] }) +@NgModule({ imports: [Holder.of(AModule), Holder.generic(), Holder.member({ DepModule: AModule })] }) export class AppModule {} ", ); - assert!(code.contains("type:AppModule,imports:[Holder,Holder]}"), "{code}"); + assert!(code.contains("type:AppModule,imports:[Holder,Holder,Holder]}"), "{code}"); } #[test] @@ -293,6 +295,37 @@ export class Later {} assert!(!code.contains("imports:[Later"), "{code}"); } + #[test] + fn a_member_of_a_parameter_keeps_its_old_output() { + // `modules.DepModule` reads the parameter, not the namespace import of + // the same name. + let dir = TempDir::new().unwrap(); + write(&dir, "dep.ts", DEP); + write( + &dir, + "holder.ts", + r" +import { Type } from '@angular/core'; +import * as modules from './dep'; +export class Holder { + static forRoot(modules: { DepModule: Type }) { return { ngModule: modules.DepModule }; } +} +", + ); + let code = compile( + &dir, + "app.ts", + r" +import { NgModule } from '@angular/core'; +import { Holder } from './holder'; +@NgModule({ imports: [Holder.forRoot({ DepModule: Holder })] }) +export class M {} +", + CompilationMode::Partial, + ); + assert!(code.contains("type:M,imports:[Holder]}"), "{code}"); + } + #[test] fn imported_method_and_constant_name_the_module_through_an_import() { // ngtsc partial: `import * as i1 from "./dep"` and From 85183e6821bbe6583f40c5d3e8d7e63c7aaf3048 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 3 Nov 2026 10:56:04 +0800 Subject: [PATCH 17/18] fix(compiler): read a ModuleWithProviders method in ngtsc's order for NgModule scopes The scope reader read the return type first and took a parameter or a type parameter for the module. It now uses the reader the partial imports use. With overloads, both read the first declaration, as ngtsc does. Co-Authored-By: Claude Opus 5.5 --- .../cross_file_elision/ng_module_scope.rs | 57 +++----------- .../src/ng_module/module_with_providers.rs | 39 ++++++---- .../tests/partial_ng_module_mwp_test.rs | 68 ++++++++++++++++ .../tests/partial_ng_module_scope_test.rs | 77 +++++++++++++++++++ 4 files changed, 176 insertions(+), 65 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs b/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs index 2c92ccf3c..ae64e82dd 100644 --- a/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs +++ b/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs @@ -580,53 +580,23 @@ impl Inner { } /// The NgModule the static method `method` of the class `decl` returns - /// a `ModuleWithProviders` of: its return type's argument, or the - /// `ngModule` of the object it returns. + /// a `ModuleWithProviders` of, read in ngtsc's order (see + /// `static_method_ng_module`). fn module_with_providers(&self, decl: &Decl, method: &str) -> Option { - let parsed = self.parsed_file(&decl.file)?; - parsed.with(|_, program, consts| { + let file = &decl.file; + self.parsed_file(file)?.with(|_, program, _| { let class = find_class(program, &decl.name)?; - let function = class.body.body.iter().find_map(|element| match element { - ClassElement::MethodDefinition(m) - if m.r#static && m.key.static_name().is_some_and(|key| key == method) => - { - Some(&m.value) - } - _ => None, - })?; - if let Some(return_type) = &function.return_type - && let TSType::TSTypeReference(reference) = &return_type.type_annotation - && type_name_ends_with(&reference.type_name, "ModuleWithProviders") - { - let argument = reference.type_arguments.as_ref()?.params.first()?; - let TSType::TSTypeReference(module) = argument else { return None }; - return type_name_ref(&decl.file, &module.type_name); - } - let body = function.body.as_ref()?; - let returned = body.statements.iter().rev().find_map(|stmt| match stmt { - Statement::ReturnStatement(ret) => ret.argument.as_ref(), - _ => None, - })?; - let mut refs = Vec::new(); - collect_refs(returned, consts, &decl.file, &mut refs, 0)?; - match <[RawRef; 1]>::try_from(refs) { - Ok([module]) => Some(module), - Err(_) => None, + match static_method_ng_module(class, method)? { + MwpModule::Value(value) => value_ref(value, file), + MwpModule::Type(name) => type_name_ref(file, name), } }) } /// The NgModule `holder.method(...)` returns a `ModuleWithProviders` - /// of, read in ngtsc's order (see `static_method_ng_module`). + /// of. fn mwp_method_module(&self, holder: &ClassRef, method: &str) -> Option { - let file = &holder.decl.file; - let raw = self.parsed_file(file)?.with(|_, program, _| { - let class = find_class(program, &holder.decl.name)?; - match static_method_ng_module(class, method)? { - MwpModule::Value(value) => value_ref(value, file), - MwpModule::Type(name) => type_name_ref(file, name), - } - })?; + let raw = self.module_with_providers(&holder.decl, method)?; self.resolve_ref(&raw, holder.owner.as_deref(), 1) } @@ -1133,15 +1103,6 @@ fn value_ref(value: &Expression<'_>, file: &str) -> Option { Some(RawRef { file: file.into(), namespace, name, method: None }) } -/// Whether a type name is `name` or `ns.name`. -fn type_name_ends_with(type_name: &TSTypeName<'_>, name: &str) -> bool { - match type_name { - TSTypeName::IdentifierReference(id) => id.name == name, - TSTypeName::QualifiedName(qualified) => qualified.right.name == name, - TSTypeName::ThisExpression(_) => false, - } -} - /// The scope the Angular decorator of a source `class` gives it. fn read_decorator<'a>( class: &'a Class<'a>, diff --git a/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs b/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs index ae239f4ee..b663af3fb 100644 --- a/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs +++ b/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs @@ -45,27 +45,32 @@ pub enum MwpModule<'b, 'a> { /// object literal with `ngModule` there is the answer. Any other body gives /// a dynamic value, and then `createModuleWithProvidersResolver` reads the /// `ModuleWithProviders` return type. `None` when unsure: a single -/// `return` of something else (ngtsc would evaluate it further), overloads, -/// a parameter or type parameter in place of the module. +/// `return` of something else (ngtsc would evaluate it further), a +/// parameter or type parameter in place of the module. +/// +/// With overloads, ngtsc reads only the first declaration (the symbol's +/// `valueDeclaration`), whatever the call's arguments: a signature has no +/// body, so its return type decides. pub fn static_method_ng_module<'b, 'a>( class: &'b Class<'a>, method: &str, ) -> Option> { - let mut found = class.body.body.iter().filter_map(|element| match element { - ClassElement::MethodDefinition(m) - if m.r#static - && m.kind == MethodDefinitionKind::Method - && !m.computed - && m.key.static_name().is_some_and(|key| key == method) => - { - Some(m) - } - _ => None, - }); - let function = &found.next()?.value; - if found.next().is_some() { - return None; - } + let function = &class + .body + .body + .iter() + .find_map(|element| match element { + ClassElement::MethodDefinition(m) + if m.r#static + && !m.computed + && m.key.static_name().is_some_and(|key| key == method) => + { + Some(m) + } + _ => None, + }) + .filter(|m| m.kind == MethodDefinitionKind::Method)? + .value; if let Some(body) = &function.body && let [Statement::ReturnStatement(ret)] = body.statements.as_slice() { diff --git a/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs b/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs index e64c097a7..91e9ac548 100644 --- a/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs +++ b/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs @@ -175,6 +175,26 @@ export class AppModule {} assert!(code.contains("type:AppModule,imports:[Holder,Holder,Holder]}"), "{code}"); } +#[test] +fn overloads_read_the_first_declaration() { + // ngtsc partial: `imports: [AMod]`, whatever the arguments or body. + let code = partial( + r" +import { NgModule, ModuleWithProviders } from '@angular/core'; +@NgModule({}) export class AMod {} +@NgModule({}) export class BMod {} +export class Holder { + static forRoot(): ModuleWithProviders; + static forRoot(x: number): ModuleWithProviders; + static forRoot(x?: number): any { return { ngModule: BMod, providers: [] }; } +} +@NgModule({ imports: [Holder.forRoot(1)] }) +export class M {} +", + ); + assert!(code.contains("type:M,imports:[AMod]}"), "{code}"); +} + #[test] fn a_class_in_a_function_keeps_its_old_output() { let code = partial( @@ -326,6 +346,54 @@ export class M {} assert!(code.contains("type:M,imports:[Holder]}"), "{code}"); } + #[test] + fn imported_overloads_read_the_first_declaration() { + // ngtsc partial: `imports: [i1.AMod, i1.AMod]`, for a source file + // and for a declaration file alike. + let dir = TempDir::new().unwrap(); + write( + &dir, + "lib.ts", + r" +import { NgModule, ModuleWithProviders } from '@angular/core'; +@NgModule({}) export class AMod {} +@NgModule({}) export class BMod {} +@NgModule({}) export class CMod {} +export class Holder { + static forRoot(): ModuleWithProviders; + static forRoot(x: number): ModuleWithProviders; + static forRoot(x?: number): any { return { ngModule: CMod, providers: [] }; } +} +", + ); + write( + &dir, + "types.d.ts", + r" +import * as i0 from '@angular/core'; +export declare class AMod { static ɵmod: i0.ɵɵNgModuleDeclaration; } +export declare class BMod { static ɵmod: i0.ɵɵNgModuleDeclaration; } +export declare class Typed { + static forRoot(): i0.ModuleWithProviders; + static forRoot(x: number): i0.ModuleWithProviders; +} +", + ); + let code = compile( + &dir, + "app.ts", + r" +import { NgModule } from '@angular/core'; +import { Holder } from './lib'; +import { Typed } from './types'; +@NgModule({ imports: [Holder.forRoot(1), Typed.forRoot(1)] }) +export class M {} +", + CompilationMode::Partial, + ); + assert!(code.contains("type:M,imports:[i1.AMod,i2.AMod]}"), "{code}"); + } + #[test] fn imported_method_and_constant_name_the_module_through_an_import() { // ngtsc partial: `import * as i1 from "./dep"` and diff --git a/crates/oxc_angular_compiler/tests/partial_ng_module_scope_test.rs b/crates/oxc_angular_compiler/tests/partial_ng_module_scope_test.rs index 521f24b92..63b66e5c8 100644 --- a/crates/oxc_angular_compiler/tests/partial_ng_module_scope_test.rs +++ b/crates/oxc_angular_compiler/tests/partial_ng_module_scope_test.rs @@ -765,3 +765,80 @@ export class App {} ); assert!(compact(&result.code).contains("import*asi1from'./d.mjs';"), "{}", result.code); } + +/// The scope `src/app.ts` gives `App` when its NgModule imports +/// `Holder.forRoot(...)` of this `src/holder.ts`, `None` when unread. +fn scope_through_holder(holder: &str, call: &str) -> Option { + let dir = TempDir::new().unwrap(); + dep_module(&dir); + write(&dir, "src/holder.ts", holder); + let source = format!( + r" +import {{ Component, NgModule }} from '@angular/core'; +import {{ Holder, EmptyModule }} from './holder.js'; +@Component({{ selector: 'app', template: '
    ', standalone: false }}) +export class App {{}} +@NgModule({{ declarations: [App], imports: [{call}] }}) +export class AppModule {{}} +" + ); + let code = compact(&compile(&dir, "src/app.ts", &source, &partial()).code); + let start = code.find("i0.ɵɵsetComponentScope(App,")?; + Some(code[start..].split(';').next().unwrap().to_string()) +} + +const HOLDER_HEAD: &str = r" +import { NgModule, ModuleWithProviders, Type } from '@angular/core'; +import * as modules from './dep.js'; +import { DepModule } from './dep.js'; +@NgModule({}) export class EmptyModule {} +"; + +#[test] +fn a_module_from_a_parameter_member_is_not_read_as_the_namespace() { + // `modules.DepModule` is the argument's `EmptyModule`, not the import. + let holder = format!( + "{HOLDER_HEAD}export class Holder {{ + static forRoot(modules: {{ DepModule: Type }}) {{ return {{ ngModule: modules.DepModule }}; }} +}}" + ); + let scope = scope_through_holder(&holder, "Holder.forRoot({ DepModule: EmptyModule })"); + assert_eq!(scope, None); +} + +#[test] +fn a_single_return_is_read_before_the_return_type() { + // ngtsc runs the body first: the module is `EmptyModule`. + let holder = format!( + "{HOLDER_HEAD}export class Holder {{ + static forRoot(): ModuleWithProviders {{ return {{ ngModule: EmptyModule as any }}; }} +}}" + ); + let scope = scope_through_holder(&holder, "Holder.forRoot()"); + assert_eq!(scope.as_deref(), Some("i0.ɵɵsetComponentScope(App,()=>[App],()=>[])")); +} + +#[test] +fn a_type_parameter_module_is_not_read() { + let holder = format!( + "{HOLDER_HEAD}export class Holder {{ + static forRoot(): ModuleWithProviders {{ const m: any = null; return m; }} +}}" + ); + let scope = scope_through_holder(&holder, "Holder.forRoot()"); + assert_eq!(scope, None); +} + +#[test] +fn overloads_read_the_first_declaration() { + // ngtsc reads the first signature, whatever the arguments. + let holder = format!( + "{HOLDER_HEAD}export class Holder {{ + static forRoot(): ModuleWithProviders; + static forRoot(x: number): ModuleWithProviders; + static forRoot(x?: number): any {{ return {{ ngModule: EmptyModule }}; }} +}}" + ); + let scope = scope_through_holder(&holder, "Holder.forRoot(1)"); + assert_eq!(scope.as_deref(), Some("i0.ɵɵsetComponentScope(App,()=>[i1.D,App],()=>[i1.P])")); +} From 663537d92e91e86e62fb97eec315afab9a661fc6 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 3 Nov 2026 11:23:29 +0800 Subject: [PATCH 18/18] fix(compiler): follow a returned call and read the return type when ngtsc can't evaluate a ModuleWithProviders body ngtsc's evaluator looks only through parentheses, `as` and `!`, gives up on an object literal with a getter or method, and evaluates a returned `Class.method()` call. When the body gives no value, it reads the `ModuleWithProviders` return type. Both the partial imports and the NgModule scope reader now do the same. Co-Authored-By: Claude Opus 5.5 --- .../cross_file_elision/ng_module_scope.rs | 80 ++++-- .../src/ng_module/module_with_providers.rs | 239 ++++++++++++++---- .../tests/partial_ng_module_mwp_test.rs | 73 +++++- .../tests/partial_ng_module_scope_test.rs | 50 ++++ 4 files changed, 360 insertions(+), 82 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs b/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs index ae64e82dd..007613216 100644 --- a/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs +++ b/crates/oxc_angular_compiler/src/component/cross_file_elision/ng_module_scope.rs @@ -31,7 +31,8 @@ use super::{CrossFileAnalyzer, Inner, MAX_EXPORT_CHAIN}; use crate::component::metadata::{ComponentScope, ImportedName, ScopeEntry, ScopeKind}; use crate::directive::{Evaluator, StringConsts, find_angular_class_decorator}; use crate::ng_module::module_with_providers::{ - MwpInit, MwpModule, module_with_providers_init, static_method_ng_module, top_level_const_init, + MwpInit, MwpModule, MwpRead, MwpType, module_with_providers_init, static_method_ng_module, + top_level_const_init, }; use crate::ng_module::{InjectorDrops, InjectorEntry}; @@ -572,32 +573,75 @@ impl Inner { let class = ClassRef { decl, owner }; match &raw.method { None => Some(class), - Some(method) => { - let module = self.module_with_providers(&class.decl, method)?; - self.resolve_ref(&module, class.owner.as_deref(), depth + 1) - } + Some(method) => self.mwp_method_read(&class, method, depth + 1).known(), } } - /// The NgModule the static method `method` of the class `decl` returns - /// a `ModuleWithProviders` of, read in ngtsc's order (see - /// `static_method_ng_module`). - fn module_with_providers(&self, decl: &Decl, method: &str) -> Option { - let file = &decl.file; - self.parsed_file(file)?.with(|_, program, _| { - let class = find_class(program, &decl.name)?; - match static_method_ng_module(class, method)? { - MwpModule::Value(value) => value_ref(value, file), - MwpModule::Type(name) => type_name_ref(file, name), + /// What `holder.method(...)` gives: the NgModule of the + /// `ModuleWithProviders` it returns, read in ngtsc's order (see + /// `static_method_ng_module`), following a call it returns. + fn mwp_method_read(&self, holder: &ClassRef, method: &str, depth: u16) -> MwpRead { + enum Found { + Module(RawRef), + Call { holder: RawRef, method: String, fallback: MwpRead }, + Dynamic, + } + if depth > MAX_EXPORT_CHAIN { + return MwpRead::Unknown; + } + let file = &holder.decl.file; + let found = self.parsed_file(file).and_then(|parsed| { + parsed.with(|_, program, _| { + let class = find_class(program, &holder.decl.name)?; + Some(match static_method_ng_module(class, method)? { + MwpModule::Value(value) => Found::Module(value_ref(value, file)?), + MwpModule::Type(name) => Found::Module(type_name_ref(file, name)?), + MwpModule::Dynamic => Found::Dynamic, + MwpModule::Call { namespace, class, method, fallback } => Found::Call { + holder: RawRef { + file: file.clone(), + namespace: namespace.map(Into::into), + name: class.into(), + method: None, + }, + method: method.into(), + fallback: match fallback { + Some(MwpType::Module(name)) => { + type_name_ref(file, name).map_or(MwpRead::Unknown, MwpRead::Known) + } + Some(MwpType::Dynamic) => MwpRead::Dynamic, + None => MwpRead::Unknown, + }, + }, + }) + }) + }); + let owner = holder.owner.as_deref(); + let resolve = |raw: &RawRef| { + self.resolve_ref(raw, owner, depth + 1).map_or(MwpRead::Unknown, MwpRead::Known) + }; + match found { + None => MwpRead::Unknown, + Some(Found::Module(raw)) => resolve(&raw), + Some(Found::Dynamic) => MwpRead::Dynamic, + Some(Found::Call { holder, method, fallback }) => { + let MwpRead::Known(holder) = resolve(&holder) else { return MwpRead::Unknown }; + match self.mwp_method_read(&holder, &method, depth + 1) { + MwpRead::Dynamic => match fallback { + MwpRead::Known(raw) => resolve(&raw), + MwpRead::Dynamic => MwpRead::Dynamic, + MwpRead::Unknown => MwpRead::Unknown, + }, + read => read, + } } - }) + } } /// The NgModule `holder.method(...)` returns a `ModuleWithProviders` /// of. fn mwp_method_module(&self, holder: &ClassRef, method: &str) -> Option { - let raw = self.module_with_providers(&holder.decl, method)?; - self.resolve_ref(&raw, holder.owner.as_deref(), 1) + self.mwp_method_read(holder, method, 1).known() } /// The NgModule the top-level `const` `decl` holds a diff --git a/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs b/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs index b663af3fb..90667f9e6 100644 --- a/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs +++ b/crates/oxc_angular_compiler/src/ng_module/module_with_providers.rs @@ -37,16 +37,56 @@ pub enum MwpModule<'b, 'a> { Value(&'b Expression<'a>), /// `T` of a `ModuleWithProviders` return type. Type(&'b TSTypeName<'a>), + /// `return Class.method(...)` (or `ns.Class.method(...)`): what that + /// call gives. When ngtsc can't evaluate it either, it reads this + /// method's return type instead: `fallback`, `None` when unsure. + Call { + namespace: Option<&'b str>, + class: &'b str, + method: &'b str, + fallback: Option>, + }, + /// ngtsc can't evaluate the method, and its return type names no + /// module. A method returning a call of it falls back to its own type. + Dynamic, +} + +/// What ngtsc reads from a method's return type. +#[derive(Clone, Copy)] +pub enum MwpType<'b, 'a> { + /// `T` of `ModuleWithProviders`. + Module(&'b TSTypeName<'a>), + /// No return type, or one that isn't a `ModuleWithProviders`. + Dynamic, +} + +/// What a call to a `ModuleWithProviders` method gives: a module, a value +/// ngtsc can't evaluate (`Dynamic`), or one Oxc can't tell (`Unknown`). +pub enum MwpRead { + Known(T), + Dynamic, + Unknown, +} + +impl MwpRead { + /// The module, if known. + pub fn known(self) -> Option { + match self { + Self::Known(module) => Some(module), + Self::Dynamic | Self::Unknown => None, + } + } } /// The NgModule the static method `method` of `class` returns a /// `ModuleWithProviders` of, read in ngtsc's order. Its evaluator first -/// runs a body that is a single `return` (`visitFunctionBody`), and an -/// object literal with `ngModule` there is the answer. Any other body gives -/// a dynamic value, and then `createModuleWithProvidersResolver` reads the -/// `ModuleWithProviders` return type. `None` when unsure: a single -/// `return` of something else (ngtsc would evaluate it further), a -/// parameter or type parameter in place of the module. +/// runs a body that is a single `return` (`visitFunctionBody`): an object +/// literal with `ngModule` there is the answer, and a call is evaluated the +/// same way. When that gives a dynamic value (any other body, `satisfies`, +/// a getter in the object...), `createModuleWithProvidersResolver` reads +/// the `ModuleWithProviders` return type. `None` when unsure: a +/// parameter or type parameter in place of the module, or a value ngtsc +/// would evaluate further. /// /// With overloads, ngtsc reads only the first declaration (the symbol's /// `valueDeclaration`), whatever the call's arguments: a signature has no @@ -71,33 +111,62 @@ pub fn static_method_ng_module<'b, 'a>( }) .filter(|m| m.kind == MethodDefinitionKind::Method)? .value; + let is_param = |expr: &Expression<'_>| { + // `param`, `param.X`: the value comes from the call's argument. + let mut root = expr; + while let Expression::StaticMemberExpression(member) = root { + root = evaluated(&member.object); + } + matches!(root, Expression::Identifier(id) if function.params.items.iter().any(|param| { + param.pattern.get_binding_identifiers().iter().any(|b| b.name == id.name) + })) + }; + let return_type = || return_type_module(class, function); if let Some(body) = &function.body && let [Statement::ReturnStatement(ret)] = body.statements.as_slice() { - let value = ng_module_value(ret.argument.as_ref()?)?; - // `param` or `param.X`: the module comes from the call's argument. - let mut root = value; - while let Expression::StaticMemberExpression(member) = root { - root = member.object.get_inner_expression(); - } - if let Expression::Identifier(id) = root - && function.params.items.iter().any(|param| { - param.pattern.get_binding_identifiers().iter().any(|b| b.name == id.name) - }) - { - return None; + match evaluated(ret.argument.as_ref()?) { + Expression::ObjectExpression(object) => match object_ng_module(object)? { + ObjectModule::Value(value) if is_param(value) => return None, + ObjectModule::Value(value) => return Some(MwpModule::Value(value)), + ObjectModule::Dynamic => {} + }, + Expression::CallExpression(call) => { + let (namespace, class, method) = static_call(call)?; + if is_param(evaluated(&call.callee)) { + return None; + } + return Some(MwpModule::Call { namespace, class, method, fallback: return_type() }); + } + Expression::TSSatisfiesExpression(_) + | Expression::TSTypeAssertion(_) + | Expression::TSInstantiationExpression(_) => {} + _ => return None, } - return Some(MwpModule::Value(value)); } - let return_type = &function.return_type.as_ref()?.type_annotation; - let TSType::TSTypeReference(reference) = return_type else { return None }; + match return_type()? { + MwpType::Module(name) => Some(MwpModule::Type(name)), + MwpType::Dynamic => Some(MwpModule::Dynamic), + } +} + +/// What ngtsc reads from the return type of `function`, a method of +/// `class`; `None` for a type parameter in place of the module. +fn return_type_module<'b, 'a>( + class: &'b Class<'a>, + function: &'b oxc_ast::ast::Function<'a>, +) -> Option> { + let Some(return_type) = &function.return_type else { return Some(MwpType::Dynamic) }; + let TSType::TSTypeReference(reference) = &return_type.type_annotation else { + return Some(MwpType::Dynamic); + }; let is_mwp = match &reference.type_name { TSTypeName::IdentifierReference(id) => id.name == "ModuleWithProviders", TSTypeName::QualifiedName(name) => name.right.name == "ModuleWithProviders", TSTypeName::ThisExpression(_) => false, }; if !is_mwp { - return None; + return Some(MwpType::Dynamic); } let [TSType::TSTypeReference(module)] = reference.type_arguments.as_ref()?.params.as_slice() else { @@ -112,24 +181,68 @@ pub fn static_method_ng_module<'b, 'a>( return None; } } - Some(MwpModule::Type(&module.type_name)) + Some(MwpType::Module(&module.type_name)) +} + +/// `expr` without what ngtsc's evaluator looks through: parentheses, `as` +/// and `!` (`visitExpression`). It can't evaluate `satisfies`, `x` or +/// `x`. +fn evaluated<'b, 'a>(mut expr: &'b Expression<'a>) -> &'b Expression<'a> { + loop { + expr = match expr { + Expression::ParenthesizedExpression(e) => &e.expression, + Expression::TSAsExpression(e) => &e.expression, + Expression::TSNonNullExpression(e) => &e.expression, + _ => return expr, + }; + } +} + +/// `(namespace, class, method)` of a `Class.method(...)` or +/// `ns.Class.method(...)` call. +fn static_call<'b>( + call: &'b oxc_ast::ast::CallExpression<'_>, +) -> Option<(Option<&'b str>, &'b str, &'b str)> { + let Expression::StaticMemberExpression(member) = evaluated(&call.callee) else { return None }; + let method = member.property.name.as_str(); + match evaluated(&member.object) { + Expression::Identifier(class) => Some((None, class.name.as_str(), method)), + Expression::StaticMemberExpression(class) => { + let Expression::Identifier(ns) = evaluated(&class.object) else { return None }; + Some((Some(ns.name.as_str()), class.property.name.as_str(), method)) + } + _ => None, + } } -/// The `ngModule` value of an object literal, `None` for anything else or -/// when a spread or computed key could set it. -fn ng_module_value<'b, 'a>(expr: &'b Expression<'a>) -> Option<&'b Expression<'a>> { - let Expression::ObjectExpression(object) = expr.get_inner_expression() else { return None }; +/// What ngtsc reads from a returned object literal. +enum ObjectModule<'b, 'a> { + /// Its `ngModule` value. + Value(&'b Expression<'a>), + /// Nothing: a getter or method makes it give up on the object + /// (`visitObjectLiteralExpression`). + Dynamic, +} + +/// The `ngModule` of an object literal, `None` when unsure: a spread or +/// computed key could set it, or there is none. +fn object_ng_module<'b, 'a>( + object: &'b oxc_ast::ast::ObjectExpression<'a>, +) -> Option> { let mut value = None; for property in &object.properties { let ObjectPropertyKind::ObjectProperty(property) = property else { return None }; + if property.kind != oxc_ast::ast::PropertyKind::Init || property.method { + return Some(ObjectModule::Dynamic); + } if property.computed { return None; } if property.key.static_name().is_some_and(|key| key == "ngModule") { - value = Some(property.value.get_inner_expression()); + value = Some(evaluated(&property.value)); } } - value + value.map(ObjectModule::Value) } /// How a constant's initializer gives a `ModuleWithProviders`. @@ -143,31 +256,15 @@ pub enum MwpInit<'b, 'a> { /// How `init` gives a `ModuleWithProviders`, if it is one of the shapes /// ngtsc can evaluate to one. pub fn module_with_providers_init<'b, 'a>(init: &'b Expression<'a>) -> Option> { - match init.get_inner_expression() { + match evaluated(init) { Expression::CallExpression(call) => { - let Expression::StaticMemberExpression(member) = call.callee.get_inner_expression() - else { - return None; - }; - let method = member.property.name.as_str(); - match member.object.get_inner_expression() { - Expression::Identifier(class) => { - Some(MwpInit::Call { namespace: None, class: class.name.as_str(), method }) - } - Expression::StaticMemberExpression(class) => { - let Expression::Identifier(ns) = class.object.get_inner_expression() else { - return None; - }; - Some(MwpInit::Call { - namespace: Some(ns.name.as_str()), - class: class.property.name.as_str(), - method, - }) - } - _ => None, - } + let (namespace, class, method) = static_call(call)?; + Some(MwpInit::Call { namespace, class, method }) } - expr @ Expression::ObjectExpression(_) => ng_module_value(expr).map(MwpInit::Module), + Expression::ObjectExpression(object) => match object_ng_module(object)? { + ObjectModule::Value(value) => Some(MwpInit::Module(value)), + ObjectModule::Dynamic => None, + }, _ => None, } } @@ -327,10 +424,40 @@ pub fn resolve_local_entry<'b>( /// The name `program` writes the NgModule of `class.method(...)` by, when /// `class` is declared there. fn method_module<'b>(program: &'b Program<'_>, class: &str, method: &str) -> Option<&'b str> { - match static_method_ng_module(top_level_class(program, class)?, method)? { - MwpModule::Value(Expression::Identifier(id)) - | MwpModule::Type(TSTypeName::IdentifierReference(id)) => Some(id.name.as_str()), - _ => None, + method_read(program, class, method, 0).known() +} + +/// What `class.method(...)` gives, `class` and the methods it calls +/// declared in `program`. +fn method_read<'b>( + program: &'b Program<'_>, + class: &str, + method: &str, + depth: u8, +) -> MwpRead<&'b str> { + const MAX_CALLS: u8 = 8; + let Some(class) = top_level_class(program, class).filter(|_| depth <= MAX_CALLS) else { + return MwpRead::Unknown; + }; + let name = |name: &'b TSTypeName<'_>| match name { + TSTypeName::IdentifierReference(id) => MwpRead::Known(id.name.as_str()), + _ => MwpRead::Unknown, + }; + match static_method_ng_module(class, method) { + Some(MwpModule::Value(Expression::Identifier(id))) => MwpRead::Known(id.name.as_str()), + Some(MwpModule::Type(type_name)) => name(type_name), + Some(MwpModule::Dynamic) => MwpRead::Dynamic, + Some(MwpModule::Call { namespace: None, class, method, fallback }) => { + match method_read(program, class, method, depth + 1) { + MwpRead::Dynamic => match fallback { + Some(MwpType::Module(type_name)) => name(type_name), + Some(MwpType::Dynamic) => MwpRead::Dynamic, + None => MwpRead::Unknown, + }, + read => read, + } + } + _ => MwpRead::Unknown, } } diff --git a/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs b/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs index 91e9ac548..8fca07562 100644 --- a/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs +++ b/crates/oxc_angular_compiler/tests/partial_ng_module_mwp_test.rs @@ -129,8 +129,6 @@ fn unsure_entries_keep_their_old_output() { // read it before it exists (ngtsc wraps the list in `() => [...]`). // - `typeOnly`: `DepModule` is only a type here, so its import is // removed; ngtsc names it anyway and drops the import. - // - `chained`: the body returns another call, which ngtsc evaluates - // further. let code = partial( r" import { NgModule, ModuleWithProviders } from '@angular/core'; @@ -138,18 +136,14 @@ import { DepModule } from './dep'; export class Holder { static later(): ModuleWithProviders { const p: any[] = []; return { ngModule: LaterModule, providers: p }; } static typeOnly(): ModuleWithProviders { const p: any[] = []; return { ngModule: null as any, providers: p }; } - static chained() { return Other.forRoot(); } } -export class Other { static forRoot(): ModuleWithProviders { return { ngModule: AModule, providers: [] }; } } -@NgModule({}) -export class AModule {} -@NgModule({ imports: [Holder.later(), Holder.typeOnly(), Holder.chained()] }) +@NgModule({ imports: [Holder.later(), Holder.typeOnly()] }) export class AppModule {} @NgModule({}) export class LaterModule {} ", ); - assert!(code.contains("type:AppModule,imports:[Holder,Holder,Holder]}"), "{code}"); + assert!(code.contains("type:AppModule,imports:[Holder,Holder]}"), "{code}"); assert!(!code.contains("'./dep'"), "the type-only import stays removed:\n{code}"); } @@ -195,6 +189,45 @@ export class M {} assert!(code.contains("type:M,imports:[AMod]}"), "{code}"); } +/// Holders whose single `return` ngtsc reads through, follows, or gives up +/// on (then reading the `ModuleWithProviders` return type). +const SHAPES: &str = r" +import { NgModule, ModuleWithProviders } from '@angular/core'; +@NgModule({}) export class DepModule {} +@NgModule({}) export class EmptyModule {} +@NgModule({}) export class OtherModule {} +export class Inner { static forRoot(): ModuleWithProviders { return { ngModule: OtherModule }; } } +export class NoType { static forRoot(): any { const p: any[] = []; return { ngModule: OtherModule, providers: p }; } } +export class H { + static sat(): ModuleWithProviders { return { ngModule: EmptyModule } satisfies ModuleWithProviders; } + static cast(): ModuleWithProviders { return >{ ngModule: EmptyModule }; } + static getter(): ModuleWithProviders { return { ngModule: EmptyModule as any, get providers() { return []; } }; } + static method(): ModuleWithProviders { return { ngModule: EmptyModule as any, foo() {} } as any; } + static paren(): ModuleWithProviders { return ({ ngModule: EmptyModule } as any)!; } + static wrapTyped(): ModuleWithProviders { return Inner.forRoot(); } + static wrapUntyped() { return Inner.forRoot(); } + static wrapDynamic(): ModuleWithProviders { return NoType.forRoot(); } +} +"; + +#[test] +fn a_returned_call_is_followed_and_an_unreadable_body_reads_the_type() { + // ngtsc partial, entry by entry: `satisfies`, `x`, a getter and a + // method give up on the body (`DepModule`); `()`, `as` and `!` are read + // through (`EmptyModule`); a returned call is evaluated, also over the + // return type (`OtherModule`), and falls back to it when ngtsc can't + // evaluate the call (`DepModule`). + let code = partial(&format!( + "{SHAPES}@NgModule({{ imports: [H.sat(), H.cast(), H.getter(), H.method(), H.paren(), H.wrapTyped(), H.wrapUntyped(), H.wrapDynamic()] }})\nexport class AppModule {{}}\n" + )); + assert!( + code.contains( + "type:AppModule,imports:[DepModule,DepModule,DepModule,DepModule,EmptyModule,OtherModule,OtherModule,DepModule]}" + ), + "{code}" + ); +} + #[test] fn a_class_in_a_function_keeps_its_old_output() { let code = partial( @@ -394,6 +427,30 @@ export class M {} assert!(code.contains("type:M,imports:[i1.AMod,i2.AMod]}"), "{code}"); } + #[test] + fn imported_returned_calls_and_unreadable_bodies() { + // ngtsc partial gives the same list as in one file, through `i1`. + let dir = TempDir::new().unwrap(); + write(&dir, "holder.ts", super::SHAPES); + let code = compile( + &dir, + "app.ts", + r" +import { NgModule } from '@angular/core'; +import { H } from './holder'; +@NgModule({ imports: [H.sat(), H.cast(), H.getter(), H.method(), H.paren(), H.wrapTyped(), H.wrapUntyped(), H.wrapDynamic()] }) +export class M {} +", + CompilationMode::Partial, + ); + assert!( + code.contains( + "type:M,imports:[i1.DepModule,i1.DepModule,i1.DepModule,i1.DepModule,i1.EmptyModule,i1.OtherModule,i1.OtherModule,i1.DepModule]}" + ), + "{code}" + ); + } + #[test] fn imported_method_and_constant_name_the_module_through_an_import() { // ngtsc partial: `import * as i1 from "./dep"` and diff --git a/crates/oxc_angular_compiler/tests/partial_ng_module_scope_test.rs b/crates/oxc_angular_compiler/tests/partial_ng_module_scope_test.rs index 63b66e5c8..7d8f6fc6d 100644 --- a/crates/oxc_angular_compiler/tests/partial_ng_module_scope_test.rs +++ b/crates/oxc_angular_compiler/tests/partial_ng_module_scope_test.rs @@ -842,3 +842,53 @@ fn overloads_read_the_first_declaration() { let scope = scope_through_holder(&holder, "Holder.forRoot(1)"); assert_eq!(scope.as_deref(), Some("i0.ɵɵsetComponentScope(App,()=>[i1.D,App],()=>[i1.P])")); } + +#[test] +fn a_returned_call_is_followed() { + // ngtsc evaluates `Inner.forRoot()`, also over this method's own type. + let holder = format!( + "{HOLDER_HEAD}export class Inner {{ + static forRoot() {{ return {{ ngModule: DepModule, providers: [] }}; }} +}} +export class Holder {{ + static forRoot(): ModuleWithProviders {{ return Inner.forRoot(); }} +}}" + ); + let scope = scope_through_holder(&holder, "Holder.forRoot()"); + assert_eq!(scope.as_deref(), Some("i0.ɵɵsetComponentScope(App,()=>[i1.D,App],()=>[i1.P])")); +} + +#[test] +fn a_call_ngtsc_cannot_evaluate_reads_the_return_type() { + let holder = format!( + "{HOLDER_HEAD}export class Inner {{ + static forRoot(): any {{ const p: any[] = []; return {{ ngModule: EmptyModule, providers: p }}; }} +}} +export class Holder {{ + static forRoot(): ModuleWithProviders {{ return Inner.forRoot(); }} +}}" + ); + let scope = scope_through_holder(&holder, "Holder.forRoot()"); + assert_eq!(scope.as_deref(), Some("i0.ɵɵsetComponentScope(App,()=>[i1.D,App],()=>[i1.P])")); +} + +#[test] +fn a_body_ngtsc_cannot_evaluate_reads_the_return_type() { + // `satisfies` and a getter make ngtsc give up on the returned object. + for body in [ + "return { ngModule: EmptyModule } satisfies ModuleWithProviders;", + "return { ngModule: EmptyModule as any, get providers() { return []; } };", + ] { + let holder = format!( + "{HOLDER_HEAD}export class Holder {{ + static forRoot(): ModuleWithProviders {{ {body} }} +}}" + ); + let scope = scope_through_holder(&holder, "Holder.forRoot()"); + assert_eq!( + scope.as_deref(), + Some("i0.ɵɵsetComponentScope(App,()=>[i1.D,App],()=>[i1.P])"), + "{body}" + ); + } +}