From 9f251c31015811c7fc048c7efd1a16a60f95e76a Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 28 Oct 2026 15:32:35 +0800 Subject: [PATCH] fix(compiler): align five small emit shapes with ngtsc (#623) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Invalid ctor deps: a param without a usable token makes the whole factory `ɵɵinvalidFactory()` (full) / `deps: "invalid"` (partial), like ngtsc's `unwrapConstructorDependencies` (annotations/common/src/ di.ts). The linker's `ɵɵinvalidFactoryDep` is unchanged. - NgModule schemas: `ɵmod` no longer lists them. The NgModule handler passes `schemas: []` (ng_module/src/handler.ts, "FW-1004" TODO); setClassMetadata still keeps them. - Directive queries: predicates are pooled (`const _c0 = [...]`), content queries first, like `baseDirectiveFields` in render3/view/compiler.ts. Host binding constants continue the index. - Default-imported tokens stay `Foo`, not `i1.default` (typeToValue's LOCAL reference with `defaultImportStatement`). - `@Inject(TOKEN)` tokens stay as written (`getConstructorDependencies` wraps the decorator argument), not `i1.TOKEN` / `i0.DOCUMENT`. Import elision keeps these imports, also without class metadata. A default import used only as a ctor param type is kept when the factory or metadata names it; without metadata, `@Inject` / `@Attribute` replace the type as the token, so it is dropped. Co-Authored-By: Claude Opus 5.5 --- .../src/component/decorator.rs | 9 +- .../src/component/definition.rs | 10 +- .../src/component/dependency.rs | 16 + .../src/component/import_elision.rs | 131 ++++---- .../src/component/transform.rs | 72 +++- .../src/directive/compiler.rs | 44 ++- .../src/factory/compiler.rs | 7 +- .../src/factory/metadata.rs | 11 + .../src/ng_module/decorator.rs | 12 +- .../src/partial/factory.rs | 10 +- .../oxc_angular_compiler/src/pipeline/emit.rs | 2 +- .../import_elision_output_references_test.rs | 19 +- .../tests/integration_test.rs | 20 +- .../tests/namespaced_ctor_token_test.rs | 4 +- .../tests/ngtsc_emit_parity_test.rs | 307 ++++++++++++++++++ .../set_class_metadata_import_elision_test.rs | 7 +- ...lass_metadata_angular_core_decorators.snap | 3 +- napi/angular-compiler/test/factory.spec.ts | 6 +- 18 files changed, 542 insertions(+), 148 deletions(-) create mode 100644 crates/oxc_angular_compiler/tests/ngtsc_emit_parity_test.rs diff --git a/crates/oxc_angular_compiler/src/component/decorator.rs b/crates/oxc_angular_compiler/src/component/decorator.rs index e6a2367cc..cafe312ca 100644 --- a/crates/oxc_angular_compiler/src/component/decorator.rs +++ b/crates/oxc_angular_compiler/src/component/decorator.rs @@ -784,6 +784,7 @@ fn extract_param_dependency<'a>( // Determine the token: // 1. If @Inject(TOKEN) is present, use TOKEN // 2. Otherwise, use the type annotation + let from_inject = inject_token.is_some(); let (token, member_path) = inject_token .or_else(|| extract_param_token(allocator, param)) .map_or((None, &[][..]), |(t, m)| (Some(t), m)); @@ -823,10 +824,10 @@ fn extract_param_dependency<'a>( // Carry the exported name so namespaced refs (`i1.X`) use the export // name, not the local alias, for aliased imports. d.token_imported_name = import_info.imported_name.clone(); - // Always use namespace imports for DI tokens (has_named_import = false). - // Import elision removes @Inject(TOKEN) argument imports since they're - // only used in decorator positions that get compiled away. - // Using bare TOKEN would fail at runtime because the import is gone. + // Type-annotation tokens use namespace imports (`i1.X`). Like ngtsc, + // an `@Inject(TOKEN)` token keeps the expression as written and a + // default import keeps its local name; import elision keeps both. + d.has_named_import = from_inject || import_info.is_default_import; d } } else { diff --git a/crates/oxc_angular_compiler/src/component/definition.rs b/crates/oxc_angular_compiler/src/component/definition.rs index ff0c56ea4..f7a9a3129 100644 --- a/crates/oxc_angular_compiler/src/component/definition.rs +++ b/crates/oxc_angular_compiler/src/component/definition.rs @@ -632,11 +632,11 @@ fn generate_constructor_factory<'a>( &allocator, )); - // If any constructor parameter resolved to a type-only import, the whole - // factory cannot be formed at runtime. Emit `i0.ɵɵinvalidFactory()` instead - // of a `new ClassCtor(...)` expression — matching Angular's - // `ValueUnavailableKind.TYPE_ONLY_IMPORT` behaviour. See issue #288. - if deps.iter().any(|d| d.type_only_invalid) { + // If any constructor parameter has no usable token (or resolved to a + // type-only import), the whole factory cannot be formed at runtime. Emit + // `i0.ɵɵinvalidFactory()` instead of a `new ClassCtor(...)` expression — + // matching ngtsc's `'invalid'` constructor deps. See issue #288. + if deps.iter().any(R3DependencyMetadata::is_invalid) { statements.push(OutputStatement::Expression(Box::new_in( crate::output::ast::ExpressionStatement { expr: create_invalid_factory_call( diff --git a/crates/oxc_angular_compiler/src/component/dependency.rs b/crates/oxc_angular_compiler/src/component/dependency.rs index 5976390b8..e3575ab87 100644 --- a/crates/oxc_angular_compiler/src/component/dependency.rs +++ b/crates/oxc_angular_compiler/src/component/dependency.rs @@ -149,6 +149,17 @@ impl<'a> R3DependencyMetadata<'a> { } } + /// Whether this dependency makes the whole factory invalid. + /// + /// ngtsc never emits `ɵɵinvalidFactoryDep` in AOT: a parameter without a + /// usable token turns the constructor deps into `'invalid'`, which becomes + /// `ɵɵinvalidFactory()`. + /// See: `packages/compiler-cli/src/ngtsc/annotations/common/src/di.ts` + /// (`unwrapConstructorDependencies`). + pub fn is_invalid(&self) -> bool { + self.type_only_invalid || (self.token.is_none() && self.attribute_name.is_none()) + } + /// Create a dependency with `@Optional()` decorator. pub fn with_optional(mut self) -> Self { self.optional = true; @@ -693,6 +704,11 @@ mod tests { let dep = R3DependencyMetadata::invalid(); let mut registry = NamespaceRegistry::new(&allocator); + // A dep without a token makes the whole factory `ɵɵinvalidFactory()`, + // so factory compilation never reaches the per-dep call below. + assert!(dep.is_invalid()); + assert!(!R3DependencyMetadata::attribute(Ident::from("title")).is_invalid()); + let result = compile_inject_dependency(&allocator, &dep, FactoryTarget::Component, 2, &mut registry); diff --git a/crates/oxc_angular_compiler/src/component/import_elision.rs b/crates/oxc_angular_compiler/src/component/import_elision.rs index 2418f01a4..eceabcd5a 100644 --- a/crates/oxc_angular_compiler/src/component/import_elision.rs +++ b/crates/oxc_angular_compiler/src/component/import_elision.rs @@ -17,14 +17,15 @@ //! - `import { type X }` - explicit type-only specifiers //! - Types only used in type annotations (e.g., `userId?: UserId`) //! - Interfaces only used in `implements` clause -//! - Constructor parameter types (DI tokens are provided via namespace imports) +//! - Constructor parameter types (DI tokens are provided via namespace imports), +//! except default imports, which the factory names by their local name //! - Constructor parameter decorators (`@Inject`, `@Optional`, etc.) - Angular removes these -//! - DI tokens used only in `@Inject(TOKEN)` arguments //! -//! The last two apply only when no `ɵsetClassMetadata` is emitted: the -//! metadata's `ctorParameters` callback names the param decorators and -//! `@Inject` tokens as bare identifiers, so their imports must survive -//! (matching ngtsc, which keeps them). +//! The last one applies only when no `ɵsetClassMetadata` is emitted: the +//! metadata's `ctorParameters` callback names the param decorators as bare +//! identifiers, so their imports must survive (matching ngtsc, which keeps +//! them). `@Inject(TOKEN)` tokens are always kept: the factory names them as +//! written, like ngtsc. //! //! ## What gets preserved //! @@ -78,11 +79,11 @@ impl<'a> ImportElisionAnalyzer<'a> { /// `emit_metadata` must mirror whether the transform will emit /// `ɵsetClassMetadata` for this file (`emit_class_metadata && /// !advanced_optimizations`). The metadata callback names constructor - /// parameter decorators (`{type: Optional}`), `@Inject(TOKEN)` argument - /// tokens (`args: [TOKEN]`), and member decorators (`{type: Input}`) by - /// their local (imported) name, so those imports must survive elision — - /// like ngtsc, which keeps them. When no metadata is emitted, the - /// decorators are stripped outright and the imports can be dropped. + /// parameter decorators (`{type: Optional}`) and member decorators + /// (`{type: Input}`) by their local (imported) name, so those imports must + /// survive elision — like ngtsc, which keeps them. When no metadata is + /// emitted, the decorators are stripped outright and the imports can be + /// dropped. pub fn analyze(program: &'a Program<'a>, emit_metadata: bool) -> Self { let semantic_ret = SemanticBuilder::new().build(program); Self::analyze_with_semantic(program, &semantic_ret.semantic, emit_metadata) @@ -197,19 +198,18 @@ impl<'a> ImportElisionAnalyzer<'a> { } let mut kept_by_output = computed_key_idents; - // Post-pass: when class metadata is emitted, a default import used as a - // bare ctor-param type reference is emitted into the metadata's - // `ctorParameters` `type:` by its local name — ngtsc keeps the import - // via `defaultImportStatement` (`type_to_value.ts` + + // Post-pass: a default import used as a bare ctor-param type reference + // is emitted by its local name, in the factory (`ɵɵinject(Foo)`) and in + // the metadata's `ctorParameters` `type:` — ngtsc keeps the import via + // `defaultImportStatement` (`type_to_value.ts` + // `attachDefaultImportDeclaration`). Keep it here too, or the emitted - // `type: Foo` would dangle. - if emit_metadata { - let ctor_type_names = Self::collect_ctor_param_type_leftmost_names(program, &consts); - for name in &ctor_type_names { - type_only_specifiers.remove(name); - } - kept_by_output.extend(ctor_type_names); + // `Foo` would dangle. + let ctor_type_names = + Self::collect_ctor_param_type_leftmost_names(program, &consts, emit_metadata); + for name in &ctor_type_names { + type_only_specifiers.remove(name); } + kept_by_output.extend(ctor_type_names); Self { type_only_specifiers, @@ -228,9 +228,13 @@ impl<'a> ImportElisionAnalyzer<'a> { /// Only default imports need this: named/namespace imports emit /// `i{reg}.Export`, which re-anchors the module import, while a default /// import emits its local name (upstream's LOCAL + defaultImportStatement). + /// + /// Without metadata, only the factory names the type, and only for a + /// param whose token is not overridden by `@Inject` / `@Attribute`. fn collect_ctor_param_type_leftmost_names( program: &'a Program<'a>, consts: &crate::directive::StringConsts<'a>, + emit_metadata: bool, ) -> FxHashSet> { let mut default_imports: FxHashSet<&'a str> = FxHashSet::default(); for stmt in &program.body { @@ -261,6 +265,7 @@ impl<'a> ImportElisionAnalyzer<'a> { class, &default_imports, consts, + emit_metadata, &mut names, ); } @@ -272,6 +277,7 @@ impl<'a> ImportElisionAnalyzer<'a> { class, &default_imports, consts, + emit_metadata, &mut names, ); } @@ -283,6 +289,7 @@ impl<'a> ImportElisionAnalyzer<'a> { class, &default_imports, consts, + emit_metadata, &mut names, ); } @@ -301,6 +308,7 @@ impl<'a> ImportElisionAnalyzer<'a> { class: &'a oxc_ast::ast::Class<'a>, default_imports: &FxHashSet<&'a str>, consts: &crate::directive::StringConsts<'a>, + emit_metadata: bool, names: &mut FxHashSet>, ) { let has_angular_decorator = class @@ -313,6 +321,20 @@ impl<'a> ImportElisionAnalyzer<'a> { let type_params = crate::class_metadata::type_param_names_of(class); if let Some(method) = crate::util::find_constructor(class) { for param in &method.value.params.items { + // `@Inject(TOKEN)` / `@Attribute('x')` replace the type as the + // factory's token (`getConstructorDependencies` in + // `annotations/common/src/di.ts`), so only the metadata's + // `type:` still names it. + if !emit_metadata + && param.decorators.iter().any(|d| { + matches!( + crate::directive::angular_param_decorator(d, Some(consts)), + Some("Inject" | "Attribute") + ) + }) + { + continue; + } let Some(annotation) = ¶m.type_annotation else { continue; }; @@ -550,9 +572,8 @@ impl<'a> ImportElisionAnalyzer<'a> { /// /// - an Angular constructor parameter decorator of the constructor implementation (the one /// `collect_constructor_decorator_spans` strips and the factory reads, see - /// [`crate::util::find_constructor`]), with the root of `@Inject(X)` / - /// `@Inject(X.sub)`: the factory names `X` through a namespace import (`i1.X.sub`), while any other - /// argument (`TOKEN['sub']`, `forwardRef(() => TOKEN)`) is copied as written + /// [`crate::util::find_constructor`]), but not its arguments: the factory names an `@Inject(X)` + /// token as written, like ngtsc /// - the Angular member decorator itself on a `declare` property, not its arguments: a query predicate /// (`viewQuery(Foo)`) or an input `transform` is compiled into the definition by its local name /// @@ -581,23 +602,6 @@ impl<'a> ImportElisionAnalyzer<'a> { } } } - /// The name a token resolves through: `TOKEN` in `TOKEN` or `TOKEN.a?.b`, like - /// `resolve_factory_dep_namespaces` does. `(TOKEN).a` or `TOKEN['a']` is copied as written. - fn member_chain_root<'b, 'a>( - expression: &'b Expression<'a>, - ) -> Option<&'b oxc_ast::ast::IdentifierReference<'a>> { - match expression { - Expression::Identifier(id) => Some(id), - Expression::StaticMemberExpression(member) => member_chain_root(&member.object), - Expression::ChainExpression(chain) => match &chain.expression { - oxc_ast::ast::ChainElement::StaticMemberExpression(member) => { - member_chain_root(&member.object) - } - _ => None, - }, - _ => None, - } - } let mut references = References::default(); for stmt in &program.body { let class = match stmt { @@ -621,22 +625,11 @@ impl<'a> ImportElisionAnalyzer<'a> { for param in constructor.into_iter().flat_map(|ctor| &ctor.value.params.items) { for decorator in ¶m.decorators { // by what it resolves to: `@I(TOKEN)` for `import { Inject as I }` is removed too - let Some(name) = - crate::directive::angular_param_decorator(decorator, Some(consts)) - else { - continue; - }; - references.visit_callee(decorator); - if name == "Inject" - && let Expression::CallExpression(call) = &decorator.expression - && let Some(token) = call - .arguments - .first() - .and_then(oxc_ast::ast::Argument::as_expression) - .and_then(member_chain_root) + if crate::directive::angular_param_decorator(decorator, Some(consts)).is_none() { - references.visit_identifier_reference(token); + continue; } + references.visit_callee(decorator); } } for element in &class.body.body { @@ -1616,8 +1609,8 @@ export class BitLabelComponent { #[test] fn test_inject_token_used_elsewhere_in_the_file_is_kept() { - // `@Inject(TOKEN)` is removed, but TOKEN is also a value elsewhere in the file: an exported const, a - // function, a nested scope. Each needs the import. + // `@Inject(TOKEN)` is removed and TOKEN is also a value elsewhere in the file: an exported const, a + // function, a nested scope. The import is kept (the factory names TOKEN too). for elsewhere in [ "export const providers = [{ provide: TOKEN, useValue: 1 }];", "export function token() { return TOKEN; }", @@ -1649,7 +1642,8 @@ export class TestService {{ #[test] fn test_aliased_inject_counts_as_removed() { - // `@I(TOKEN)` is Angular's Inject under an alias, which compilation removes like `@Inject(TOKEN)` + // `@I(TOKEN)` is Angular's Inject under an alias, which compilation removes like `@Inject(TOKEN)`. + // TOKEN stays: the factory names it as written. let source = r#" import { Injectable, Inject } from "@angular/core"; import { Inject as I } from "@angular/core"; @@ -1665,12 +1659,15 @@ export class B { constructor(@Inject(TOKEN) private value: unknown) {} } "#; - assert!(analyze_source(source).contains("TOKEN")); + let type_only = analyze_source(source); + assert!(type_only.contains("I"), "the aliased Inject should be elided"); + assert!(!type_only.contains("TOKEN"), "TOKEN should be kept"); } #[test] - fn test_inject_decorator_and_token_elided() { - // @Inject(TOKEN) - both the decorator and the token should be elided + fn test_inject_decorator_elided_and_token_kept() { + // @Inject(TOKEN) - the decorator is elided, but the factory names the + // token as written (`ɵɵdirectiveInject(DOCUMENT)`), like ngtsc let source = r#" import { Component, Inject } from "@angular/core"; import { DOCUMENT } from "@angular/common"; @@ -1687,10 +1684,10 @@ export class TestComponent { // Inject should be elided (ctor param decorator) assert!(type_only.contains("Inject"), "Inject decorator should be elided"); - // DOCUMENT token should be elided (only used in @Inject argument) + // DOCUMENT token should be kept (the factory uses it) assert!( - type_only.contains("DOCUMENT"), - "DOCUMENT token should be elided (only used in @Inject)" + !type_only.contains("DOCUMENT"), + "DOCUMENT token should be kept (the factory uses it)" ); // Component should be preserved @@ -1721,8 +1718,8 @@ export class TestComponent { assert!(type_only.contains(decorator), "{} decorator should be elided", decorator); } - // LOCALE_ID token should be elided - assert!(type_only.contains("LOCALE_ID"), "LOCALE_ID token should be elided"); + // LOCALE_ID token should be kept (the factory uses it) + assert!(!type_only.contains("LOCALE_ID"), "LOCALE_ID token should be kept"); // Component should be preserved assert!(!type_only.contains("Component"), "Component should be preserved"); diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index ea6984a72..85757ab9d 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -724,14 +724,34 @@ fn module_export_name_to_str<'a>(name: &ModuleExportName<'a>) -> Option<&'a str> /// qualified type `ns.one.One` reads off the namespace import (`i1.one.One`), and /// `mod.Foo` off the named import (`i1.mod.Foo`). /// This ensures the factory works correctly even after import elision. +/// +/// Only type-annotation tokens are rewritten. Like ngtsc's `getConstructorDependencies` +/// (`annotations/common/src/di.ts`), an `@Inject(TOKEN)` token is the expression as +/// written, and a default import (`import Foo from './foo'`) stays `Foo` +/// (`typeToValue`'s LOCAL reference with a `defaultImportStatement`). Import elision +/// keeps both imports. fn resolve_factory_dep_namespaces<'a>( allocator: &'a Allocator, deps: &mut oxc_allocator::Vec<'a, crate::factory::R3DependencyMetadata<'a>>, + class: &oxc_ast::ast::Class<'a>, + string_consts: &crate::directive::StringConsts<'a>, import_map: &ImportMap<'a>, namespace_registry: &mut NamespaceRegistry<'a>, ) { - for dep in deps.iter_mut() { + // One dep per constructor parameter, in order. + let params = crate::util::find_constructor(class).map(|ctor| &ctor.value.params.items); + let is_inject_with_token = |decorator: &oxc_ast::ast::Decorator<'a>| { + crate::directive::angular_param_decorator(decorator, Some(string_consts)) == Some("Inject") + && matches!( + &decorator.expression, + Expression::CallExpression(call) if !call.arguments.is_empty() + ) + }; + for (index, dep) in deps.iter_mut().enumerate() { let Some(token) = dep.token.as_mut() else { continue }; + let from_inject = params + .and_then(|params| params.get(index)) + .is_some_and(|param| param.decorators.iter().any(is_inject_with_token)); let is_member_chain = matches!(token, OutputExpression::ReadProp(_)); let root = member_chain_root(token); // Only process bare variable references (ReadVar) @@ -749,6 +769,9 @@ fn resolve_factory_dep_namespaces<'a>( dep.type_only_invalid = true; continue; } + if from_inject || import_info.is_default_import { + continue; + } // Replace with namespace-prefixed reference: i1.Store instead of Store let namespace = OutputExpression::ReadVar(oxc_allocator::Box::new_in( ReadVarExpr { @@ -3423,6 +3446,8 @@ pub fn transform_angular_file( resolve_factory_dep_namespaces( &allocator, deps, + class, + &string_consts, &import_map, &mut file_namespace_registry, ); @@ -4003,6 +4028,8 @@ pub fn transform_angular_file( resolve_factory_dep_namespaces( &allocator, deps, + class, + &string_consts, &import_map, &mut file_namespace_registry, ); @@ -4152,6 +4179,8 @@ pub fn transform_angular_file( resolve_factory_dep_namespaces( &allocator, deps, + class, + &string_consts, &import_map, &mut file_namespace_registry, ); @@ -4267,6 +4296,8 @@ pub fn transform_angular_file( resolve_factory_dep_namespaces( &allocator, deps, + class, + &string_consts, &import_map, &mut file_namespace_registry, ); @@ -4497,6 +4528,8 @@ pub fn transform_angular_file( resolve_factory_dep_namespaces( &allocator, deps, + class, + &string_consts, &import_map, &mut file_namespace_registry, ); @@ -8604,13 +8637,15 @@ export class CustomElementsModule {} let result = transform_angular_file(&allocator, "custom-elements.module.ts", source, None, None); - // ɵmod should be generated with schemas + // ɵmod should be generated without schemas: like ngtsc, they are only + // kept in the class metadata. + let mod_line = result.code.lines().find(|l| l.contains("static ɵmod = ")).unwrap(); + assert!(!mod_line.contains("schemas"), "ɵmod should not list schemas: {mod_line}"); assert!( - result.code.contains("static ɵmod = "), - "Code should contain .ɵmod = , but got:\n{}", + result.code.contains("schemas:[CUSTOM_ELEMENTS_SCHEMA]"), + "Class metadata should keep schemas, but got:\n{}", result.code ); - assert!(result.code.contains("schemas"), "Code should contain schemas property"); } #[test] @@ -9024,29 +9059,34 @@ export class TestComponent { let result = transform_angular_file(&allocator, "test.component.ts", source, Some(&options), None); - // @Inject(DARK_THEME) now uses namespace imports (i1 for @app/theme), - // so rxjs gets i2 for Observable. + // Like ngtsc, the factory names @Inject(DARK_THEME) as written, so only + // rxjs gets a namespace import (i1 for Observable). assert!( - result.code.contains("import * as i1 from '@app/theme'"), - "Should generate namespace import for @app/theme, but got:\n{}", + result.code.contains("import * as i1 from 'rxjs'"), + "Should generate namespace import for rxjs, but got:\n{}", result.code ); assert!( - result.code.contains("import * as i2 from 'rxjs'"), - "Should generate namespace import for rxjs, but got:\n{}", + !result.code.contains("import * as i2"), + "@app/theme should not get a namespace import, but got:\n{}", + result.code + ); + assert!( + result.code.contains("import { DARK_THEME } from '@app/theme'"), + "The DARK_THEME import should be kept, but got:\n{}", result.code ); - // The factory should use namespace-prefixed DARK_THEME + // The factory should use DARK_THEME as written assert!( - result.code.contains("i1.DARK_THEME"), - "Factory should use namespace-prefixed DARK_THEME, but got:\n{}", + result.code.contains("ɵɵdirectiveInject(DARK_THEME)"), + "Factory should use DARK_THEME as written, but got:\n{}", result.code ); - // The setClassMetadata ctor params should reference i2.Observable + // The setClassMetadata ctor params should reference i1.Observable assert!( - result.code.contains("i2.Observable"), + result.code.contains("i1.Observable"), "setClassMetadata should use namespace-prefixed Observable, but got:\n{}", result.code ); diff --git a/crates/oxc_angular_compiler/src/directive/compiler.rs b/crates/oxc_angular_compiler/src/directive/compiler.rs index 70cc79e19..254a591b5 100644 --- a/crates/oxc_angular_compiler/src/directive/compiler.rs +++ b/crates/oxc_angular_compiler/src/directive/compiler.rs @@ -24,11 +24,15 @@ use super::metadata::{ use crate::ast::expression::{BindingType, ParsedEventType}; use crate::ast::r3::{R3BoundAttribute, R3BoundEvent, SecurityContext}; use crate::output::ast::{ - FunctionExpr, InvokeFunctionExpr, LiteralArrayExpr, LiteralExpr, LiteralMapEntry, - LiteralMapExpr, LiteralValue, OutputExpression, OutputStatement, ReadPropExpr, ReadVarExpr, + DeclareVarStmt, FunctionExpr, InvokeFunctionExpr, LiteralArrayExpr, LiteralExpr, + LiteralMapEntry, LiteralMapExpr, LiteralValue, OutputExpression, OutputStatement, ReadPropExpr, + ReadVarExpr, StmtModifier, }; use crate::parser::expression::BindingParser; -use crate::pipeline::emit::{HostBindingCompilationResult, compile_host_bindings}; +use crate::pipeline::constant_pool::ConstantPool; +use crate::pipeline::emit::{ + HostBindingCompilationResult, compile_host_bindings, emit_pooled_constant_value, +}; use crate::pipeline::ingest::{HostAttributes, HostBindingInput, ingest_host_binding_with_version}; use crate::pipeline::selector::{ parse_selector_to_r3_selector as parse_css_to_r3, r3_selector_to_output_expr, @@ -120,7 +124,6 @@ fn build_base_directive_fields<'a>( angular_version: Option, ) -> (Vec<'a, LiteralMapEntry<'a>>, u32, oxc_allocator::Vec<'a, OutputStatement<'a>>) { let mut entries = Vec::new_in(&allocator); - let mut next_pool_index = pool_starting_index; let mut host_declarations = oxc_allocator::Vec::new_in(&allocator); // type: MyDirective @@ -137,16 +140,18 @@ fn build_base_directive_fields<'a>( } } + // Query predicates go to the constant pool, content queries first, like + // `baseDirectiveFields` in Angular's `render3/view/compiler.ts`. + let mut query_pool = ConstantPool::with_starting_index(allocator, pool_starting_index); + // contentQueries: (rf, ctx, dirIndex) => { ... } if !metadata.queries.is_empty() { - // Note: Directive compiler doesn't have access to constant pool, so predicates - // are not pooled. For components, pool is passed from component compilation. let content_queries_fn = super::query::create_content_queries_function( &allocator, core_namespace, &metadata.queries, Some(metadata.name.as_str()), - None, + Some(&mut query_pool), angular_version, ); entries.push(LiteralMapEntry::new( @@ -158,19 +163,34 @@ fn build_base_directive_fields<'a>( // viewQuery: (rf, ctx) => { ... } if !metadata.view_queries.is_empty() { - // Note: Directive compiler doesn't have access to constant pool, so predicates - // are not pooled. For components, pool is passed from component compilation. let view_queries_fn = super::query::create_view_queries_function( &allocator, core_namespace, &metadata.view_queries, Some(metadata.name.as_str()), - None, + Some(&mut query_pool), angular_version, ); entries.push(LiteralMapEntry::new(Ident::from("viewQuery"), view_queries_fn, false)); } + // Emit the pooled predicates before the host binding constants, which continue + // from the query pool's next index so `_cN` names don't clash. + let mut next_pool_index = query_pool.next_name_index(); + for constant in query_pool.constants_mut() { + let value = emit_pooled_constant_value(allocator, &mut constant.kind); + host_declarations.push(OutputStatement::DeclareVar(Box::new_in( + DeclareVarStmt { + name: constant.name, + value: Some(value), + modifiers: StmtModifier::FINAL, + leading_comment: None, + source_span: None, + }, + &allocator, + ))); + } + // hostBindings: (rf, ctx) => { ... } // Uses the IR pipeline for proper host binding compilation // Per Angular's compiler.ts lines 525-532, createHostBindingsFunction sets: @@ -182,7 +202,7 @@ fn build_base_directive_fields<'a>( allocator, core_namespace, metadata, - pool_starting_index, + next_pool_index, angular_version, ) { next_pool_index = new_pool_index; @@ -219,7 +239,7 @@ fn build_base_directive_fields<'a>( } // Collect host binding pool declarations (pure functions, etc.) - host_declarations = result.declarations; + host_declarations.extend(result.declarations); } } diff --git a/crates/oxc_angular_compiler/src/factory/compiler.rs b/crates/oxc_angular_compiler/src/factory/compiler.rs index 33215725d..a84cfce0a 100644 --- a/crates/oxc_angular_compiler/src/factory/compiler.rs +++ b/crates/oxc_angular_compiler/src/factory/compiler.rs @@ -141,9 +141,10 @@ pub fn compile_factory_function<'a>( // Build the constructor expression based on deps // See: r3_factory.ts:119-129 let ctor_expr: Option> = match &base.deps { - R3FactoryDeps::Valid(deps) if deps.iter().any(|d| d.type_only_invalid) => { - // A constructor parameter resolves to a type-only import; the factory - // as a whole cannot be formed. Force `ɵɵinvalidFactory()`. See #288. + R3FactoryDeps::Valid(deps) if deps.iter().any(R3DependencyMetadata::is_invalid) => { + // A constructor parameter has no usable token (or resolves to a type-only + // import), so the factory as a whole cannot be formed. Like ngtsc, which + // marks such deps `'invalid'`, force `ɵɵinvalidFactory()`. See #288. None } R3FactoryDeps::Valid(deps) => { diff --git a/crates/oxc_angular_compiler/src/factory/metadata.rs b/crates/oxc_angular_compiler/src/factory/metadata.rs index c83f87a04..fee94f3fe 100644 --- a/crates/oxc_angular_compiler/src/factory/metadata.rs +++ b/crates/oxc_angular_compiler/src/factory/metadata.rs @@ -95,6 +95,17 @@ impl<'a> R3DependencyMetadata<'a> { type_only_invalid: false, } } + + /// Whether this dependency makes the whole factory invalid. + /// + /// ngtsc never emits `ɵɵinvalidFactoryDep` in AOT: a parameter without a + /// usable token turns the constructor deps into `'invalid'`, which becomes + /// `ɵɵinvalidFactory()` (full) or `deps: "invalid"` (partial). + /// See: `packages/compiler-cli/src/ngtsc/annotations/common/src/di.ts` + /// (`unwrapConstructorDependencies`). + pub fn is_invalid(&self) -> bool { + self.type_only_invalid || (self.token.is_none() && self.attribute_name_type.is_none()) + } } /// Result type for dependencies: valid deps, invalid (error), or none (inherited). diff --git a/crates/oxc_angular_compiler/src/ng_module/decorator.rs b/crates/oxc_angular_compiler/src/ng_module/decorator.rs index dfa7d14da..9ac2067e2 100644 --- a/crates/oxc_angular_compiler/src/ng_module/decorator.rs +++ b/crates/oxc_angular_compiler/src/ng_module/decorator.rs @@ -167,14 +167,10 @@ impl<'a> NgModuleMetadata<'a> { builder = builder.add_bootstrap(R3Reference::value_only(bootstrap_expr)); } - // Add schemas - for schema in &self.schemas { - let schema_expr = OutputExpression::ReadVar(Box::new_in( - ReadVarExpr { name: schema.clone(), source_span: None }, - &allocator, - )); - builder = builder.add_schema(R3Reference::value_only(schema_expr)); - } + // Schemas are left out: ngtsc always passes `schemas: []` here (the + // "TODO: to be implemented as a part of FW-1004" in the NgModule + // handler), so `ɵmod` never lists them. They stay in the class metadata. + // See: packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts // Set module ID if present if let Some(id) = &self.id { diff --git a/crates/oxc_angular_compiler/src/partial/factory.rs b/crates/oxc_angular_compiler/src/partial/factory.rs index 7adb24630..4bcce26fb 100644 --- a/crates/oxc_angular_compiler/src/partial/factory.rs +++ b/crates/oxc_angular_compiler/src/partial/factory.rs @@ -13,7 +13,7 @@ use oxc_allocator::{Allocator, Box, Vec}; use oxc_str::Ident; use super::{MIN_VERSION_FACTORY, PLACEHOLDER_VERSION}; -use crate::factory::{FactoryTarget, R3FactoryDeps, R3FactoryMetadata}; +use crate::factory::{FactoryTarget, R3DependencyMetadata, R3FactoryDeps, R3FactoryMetadata}; use crate::output::ast::{ InvokeFunctionExpr, LiteralArrayExpr, LiteralExpr, LiteralMapEntry, LiteralMapExpr, LiteralValue, OutputExpression, ReadPropExpr, ReadVarExpr, @@ -94,9 +94,9 @@ pub fn compile_declare_factory_function<'a>( /// `ɵɵinvalidFactory()`). /// - `R3FactoryDeps::None` → `null` (linker emits the inherited-factory /// pattern). -/// - `R3FactoryDeps::Valid(deps)` where any dep has `type_only_invalid` → -/// coerced to `"invalid"`, matching full-mode behavior (see #288 and -/// `factory/compiler.rs:143`). +/// - `R3FactoryDeps::Valid(deps)` where any dep is invalid (no token, or a +/// type-only import) → coerced to `"invalid"`, matching full-mode behavior +/// (see #288 and `factory/compiler.rs:143`). /// - Otherwise → array literal of per-dep maps. fn compile_dependencies<'a>( allocator: &'a Allocator, @@ -108,7 +108,7 @@ fn compile_dependencies<'a>( LiteralExpr { value: LiteralValue::Null, source_span: None }, &allocator, )), - R3FactoryDeps::Valid(deps) if deps.iter().any(|d| d.type_only_invalid) => { + R3FactoryDeps::Valid(deps) if deps.iter().any(R3DependencyMetadata::is_invalid) => { string_literal(allocator, "invalid") } R3FactoryDeps::Valid(deps) => { diff --git a/crates/oxc_angular_compiler/src/pipeline/emit.rs b/crates/oxc_angular_compiler/src/pipeline/emit.rs index 51c28ab68..f3b08a2c4 100644 --- a/crates/oxc_angular_compiler/src/pipeline/emit.rs +++ b/crates/oxc_angular_compiler/src/pipeline/emit.rs @@ -1465,7 +1465,7 @@ fn convert_ast_for_pure_function_body<'a>( /// Emit a pooled constant as an OutputExpression. /// Takes mutable reference to allow taking ownership of expressions that can't be cloned. -fn emit_pooled_constant_value<'a>( +pub fn emit_pooled_constant_value<'a>( allocator: &'a Allocator, kind: &mut crate::pipeline::constant_pool::PooledConstantKind<'a>, ) -> OutputExpression<'a> { diff --git a/crates/oxc_angular_compiler/tests/import_elision_output_references_test.rs b/crates/oxc_angular_compiler/tests/import_elision_output_references_test.rs index e3ebe610b..a60a388bd 100644 --- a/crates/oxc_angular_compiler/tests/import_elision_output_references_test.rs +++ b/crates/oxc_angular_compiler/tests/import_elision_output_references_test.rs @@ -52,7 +52,8 @@ fn assert_kept(source: &str, names: &[&str]) { } #[test] -fn plain_inject_token_and_decorator_are_still_elided() { +fn plain_inject_token_is_kept_and_decorators_are_elided() { + // Like ngtsc, the factory names the token as written (`ɵɵinject(TOKEN, 8)`). let code = compile( r#" import { Injectable, Inject, Optional } from "@angular/core"; @@ -61,10 +62,11 @@ import { TOKEN } from "./tokens"; export class B { constructor(@Optional() @Inject(TOKEN) v: unknown) {} } "#, ); - assert!(!imports(&code, "TOKEN"), "{code}"); + assert!(imports(&code, "TOKEN"), "{code}"); assert!(!imports(&code, "Inject"), "{code}"); assert!(!imports(&code, "Optional"), "{code}"); - assert!(code.contains("i1.TOKEN"), "{code}"); + assert!(code.contains("ɵɵinject(TOKEN,8)"), "{code}"); + assert!(!code.contains("i1.TOKEN"), "{code}"); } #[test] @@ -84,9 +86,9 @@ export class B { constructor(@Inject(TOKEN) v: unknown) {} } } #[test] -fn inject_member_expression_reads_off_namespace_import() { - // The factory reads `TOKEN.sub` off the namespace import (`i1.TOKEN.sub`). - for (arg, member) in [("TOKEN.sub", "i1.TOKEN.sub"), ("TOKEN?.a.b", "i1.TOKEN?.a.b")] { +fn inject_member_expression_keeps_token() { + // The factory copies `TOKEN.sub` as written, like ngtsc. + for arg in ["TOKEN.sub", "TOKEN?.a.b"] { let code = compile(&format!( r#" import {{ Injectable, Inject }} from "@angular/core"; @@ -94,8 +96,9 @@ import {{ TOKEN }} from "./tokens"; @Injectable() export class S {{ constructor(@Inject({arg}) private v: unknown) {{}} }} "# )); - assert!(!imports(&code, "TOKEN"), "{code}"); - assert!(code.contains(member), "{code}"); + assert!(imports(&code, "TOKEN"), "{code}"); + assert!(code.contains(&format!("ɵɵinject({arg})")), "{code}"); + assert!(!code.contains("i1."), "{code}"); } } diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index e52261f76..1fb7499fe 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -4076,7 +4076,8 @@ export class BitLabelComponent { "DOCUMENT should be kept in imports (named by setClassMetadata). Import line: {common_line}" ); - // With class metadata emission disabled, the same imports are elided again. + // With class metadata emission disabled, the param decorators are elided again, + // but DOCUMENT stays: the factory names it as written (`ɵɵdirectiveInject(DOCUMENT)`). let no_metadata = ComponentTransformOptions { emit_class_metadata: false, ..ComponentTransformOptions::default() @@ -4097,14 +4098,15 @@ export class BitLabelComponent { "{decorator} should be elided when no setClassMetadata is emitted. Import line: {import_line}" ); } - if let Some(common_line) = - code.lines().find(|l| l.starts_with("import") && l.contains("@angular/common")) - { - assert!( - !common_line.contains("DOCUMENT"), - "DOCUMENT should be elided when no setClassMetadata is emitted. Import line: {common_line}" - ); - } + let common_line = code + .lines() + .find(|l| l.starts_with("import") && l.contains("@angular/common")) + .unwrap_or_else(|| panic!("DOCUMENT import missing. Code: {code}")); + assert!( + common_line.contains("DOCUMENT"), + "DOCUMENT should be kept (named by the factory). Import line: {common_line}" + ); + assert!(code.contains("ɵɵdirectiveInject(DOCUMENT)"), "Code: {code}"); } /// Test that @HostBinding and @HostListener decorators are elided from imports. diff --git a/crates/oxc_angular_compiler/tests/namespaced_ctor_token_test.rs b/crates/oxc_angular_compiler/tests/namespaced_ctor_token_test.rs index 1808ffd07..b20d8675a 100644 --- a/crates/oxc_angular_compiler/tests/namespaced_ctor_token_test.rs +++ b/crates/oxc_angular_compiler/tests/namespaced_ctor_token_test.rs @@ -153,7 +153,7 @@ export class ACmp {{ } } -/// A component's `@Inject(TOKEN.sub)` is injected, not the parameter type. +/// A component's `@Inject(TOKEN.sub)` is injected as written, not the parameter type. #[test] fn component_inject_member_chain_wins_over_the_type() { let code = compile( @@ -167,5 +167,5 @@ export class ACmp { } ", ); - assert_contains(&code, "(i0.ɵɵdirectiveInject(i1.TOKEN.a.b),i0.ɵɵdirectiveInject(i2.TOKEN))"); + assert_contains(&code, "(i0.ɵɵdirectiveInject(TOKEN.a.b),i0.ɵɵdirectiveInject(ns.TOKEN))"); } diff --git a/crates/oxc_angular_compiler/tests/ngtsc_emit_parity_test.rs b/crates/oxc_angular_compiler/tests/ngtsc_emit_parity_test.rs new file mode 100644 index 000000000..0ff06d6ab --- /dev/null +++ b/crates/oxc_angular_compiler/tests/ngtsc_emit_parity_test.rs @@ -0,0 +1,307 @@ +//! Small emit-shape differences from ngtsc (issue #623), each checked against +//! `@angular/compiler-cli` 22.2.3 output: +//! +//! - a constructor parameter without a usable token makes the whole factory +//! `ɵɵinvalidFactory()` (full) / `deps: "invalid"` (partial), never +//! `ɵɵinvalidFactoryDep(n)` +//! - `ɵmod` never lists `schemas` (the NgModule handler passes `schemas: []`) +//! - a directive's query predicates are pooled (`const _c0 = [...]`) +//! - a default-imported DI token stays `Foo`, not `i1.default`; its import is +//! dropped when `@Inject` / `@Attribute` override it and no metadata is emitted +//! - an `@Inject(TOKEN)` token stays as written, not `i1.TOKEN` + +// The expected output has object literals like `{token:Foo}`. +#![expect(clippy::literal_string_with_formatting_args)] + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{CompilationMode, TransformOptions, transform_angular_file}; + +/// The compiled `source`, without whitespace. +fn compile_with(source: &str, options: &TransformOptions) -> String { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, "test.ts", 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) -> String { + compile_with(source, &TransformOptions::default()) +} + +fn compile_partial(source: &str) -> String { + compile_with( + source, + &TransformOptions { compilation_mode: CompilationMode::Partial, ..Default::default() }, + ) +} + +fn compile_without_metadata(source: &str, mode: CompilationMode) -> String { + compile_with( + source, + &TransformOptions { + compilation_mode: mode, + emit_class_metadata: false, + ..Default::default() + }, + ) +} + +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}"); +} + +const INVALID_DEP_CASES: &[(&str, &str)] = &[ + ( + "string param", + "import { Injectable } from '@angular/core'; +@Injectable() +export class S { constructor(s: string) {} }", + ), + ( + "untyped param", + "import { Injectable } from '@angular/core'; +@Injectable() +export class S { constructor(s) {} }", + ), + ( + "second param invalid", + "import { Injectable } from '@angular/core'; +class Ok {} +@Injectable() +export class S { constructor(o: Ok, s: string) {} }", + ), + ( + "directive without selector", + "import { Directive } from '@angular/core'; +@Directive() +export class D { constructor(s: string) {} }", + ), + ( + "component", + "import { Component } from '@angular/core'; +@Component({ selector: 'c', template: '' }) +export class C { constructor(s: string) {} }", + ), +]; + +#[test] +fn invalid_ctor_dep_makes_the_full_factory_invalid() { + for (name, source) in INVALID_DEP_CASES { + let code = compile(source); + assert!( + code.contains("_Factory(__ngFactoryType__){i0.ɵɵinvalidFactory();}"), + "{name}:\n{code}" + ); + assert!(!code.contains("ɵɵinvalidFactoryDep"), "{name}:\n{code}"); + } +} + +#[test] +fn invalid_ctor_dep_makes_the_partial_deps_invalid() { + for (name, source) in INVALID_DEP_CASES { + let code = compile_partial(source); + assert!(code.contains("deps:\"invalid\""), "{name}:\n{code}"); + assert!(!code.contains("token:null"), "{name}:\n{code}"); + } +} + +const SCHEMAS_MODULE: &str = " +import { NgModule, NO_ERRORS_SCHEMA, CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; +@NgModule({ schemas: [NO_ERRORS_SCHEMA, CUSTOM_ELEMENTS_SCHEMA] }) +export class M {} +"; + +#[test] +fn ng_module_schemas_are_left_out_of_the_full_definition() { + let code = compile(SCHEMAS_MODULE); + assert_contains(&code, "i0.ɵɵdefineNgModule({type:M})"); + // The class metadata still lists them. + assert_contains(&code, "args:[{schemas:[NO_ERRORS_SCHEMA,CUSTOM_ELEMENTS_SCHEMA]}]"); +} + +#[test] +fn ng_module_schemas_are_left_out_of_the_partial_declaration() { + let code = compile_partial(SCHEMAS_MODULE); + assert_contains( + &code, + "ɵɵngDeclareNgModule({minVersion:\"14.0.0\",version:\"0.0.0-PLACEHOLDER\",ngImport:i0,type:M})", + ); + assert_contains(&code, "args:[{schemas:[NO_ERRORS_SCHEMA,CUSTOM_ELEMENTS_SCHEMA]}]"); +} + +#[test] +fn directive_query_predicates_are_pooled_before_host_binding_constants() { + let code = compile( + " +import { Directive, ViewChild, ContentChild } from '@angular/core'; +@Directive({ selector: '[d]', host: { '[style]': '{color: c, width: w}', '[class]': '[c, w]' } }) +export class D { + c = 'red'; w = 1; + @ViewChild('test1') a: any; + @ContentChild('c1') c1: any; +} +", + ); + // Content queries first, then view queries, then the host pure functions. + assert_contains( + &code, + "const_c0=[\"c1\"];const_c1=[\"test1\"];const_c2=(a0,a1)=>({color:a0,width:a1});const_c3=(a0,a1)=>[a0,a1];", + ); + assert_contains(&code, "i0.ɵɵcontentQuery(dirIndex,_c0,5)"); + assert_contains(&code, "i0.ɵɵviewQuery(_c1,5)"); + assert_contains(&code, "i0.ɵɵpureFunction2(4,_c2,ctx.c,ctx.w)"); + assert_contains(&code, "i0.ɵɵpureFunction2(7,_c3,ctx.c,ctx.w)"); +} + +#[test] +fn directive_query_predicates_continue_the_file_numbering() { + let code = compile( + " +import { Directive, ViewChild, ViewChildren, ContentChildren } from '@angular/core'; +@Directive({ selector: '[d1]', host: { '[style]': '{color: c}' } }) +export class D1 { + c = 'red'; + @ViewChild('v1') a: any; + @ContentChildren('k1,k2') b: any; +} +@Directive({ selector: '[d2]' }) +export class D2 { + @ViewChildren('v2') a: any; +} +", + ); + assert_contains( + &code, + "const_c0=[\"k1\",\"k2\"];const_c1=[\"v1\"];const_c2=(a0)=>({color:a0});", + ); + assert_contains(&code, "const_c3=[\"v2\"];"); + assert_contains(&code, "i0.ɵɵcontentQuery(dirIndex,_c0,4)"); + assert_contains(&code, "i0.ɵɵviewQuery(_c1,5)"); + assert_contains(&code, "i0.ɵɵviewQuery(_c3,5)"); +} + +const DEFAULT_IMPORT: &str = " +import { Injectable, Component, Inject } from '@angular/core'; +import Foo from './foo'; +@Injectable() +export class S { constructor(f: Foo, @Inject(Foo) g: Foo) {} } +@Component({ selector: 'c', template: '' }) +export class C { constructor(f: Foo) {} } +"; + +#[test] +fn default_imported_token_keeps_its_local_name() { + for mode in [CompilationMode::Full, CompilationMode::Partial] { + for code in [ + compile_with( + DEFAULT_IMPORT, + &TransformOptions { compilation_mode: mode, ..Default::default() }, + ), + compile_without_metadata(DEFAULT_IMPORT, mode), + ] { + assert_contains(&code, "importFoofrom'./foo';"); + assert_not_contains(&code, "i1.default"); + assert_not_contains(&code, "import*asi1"); + if mode == CompilationMode::Full { + assert_contains(&code, "(i0.ɵɵinject(Foo),i0.ɵɵinject(Foo))"); + assert_contains(&code, "(i0.ɵɵdirectiveInject(Foo))"); + } else { + assert_contains(&code, "type:S,deps:[{token:Foo},{token:Foo}]"); + assert_contains(&code, "type:C,deps:[{token:Foo}]"); + } + } + } +} + +const DEFAULT_IMPORT_OVERRIDDEN: &str = " +import { Injectable, Directive, Component, Inject, Attribute, Optional } from '@angular/core'; +import Foo from './foo'; +import Bar from './bar'; +import { TOKEN } from './tok'; +@Injectable() +export class S { constructor(@Inject(TOKEN) f: Foo, @Optional() b: Bar) {} } +@Directive({ selector: '[x]' }) +export class D { constructor(@Attribute('x') f: Foo) {} } +@Component({ selector: 'c', template: '' }) +export class C { constructor(@Optional() @Inject(TOKEN) f: Foo) {} } +"; + +#[test] +fn default_import_overridden_by_inject_or_attribute_is_elided_without_metadata() { + // `@Inject` / `@Attribute` replace the type as the factory's token, so + // without class metadata nothing names `Foo` and ngtsc drops its import. + // `@Optional()` keeps the type as the token. + for mode in [CompilationMode::Full, CompilationMode::Partial] { + let code = compile_without_metadata(DEFAULT_IMPORT_OVERRIDDEN, mode); + assert_not_contains(&code, "importFoofrom"); + assert_contains(&code, "importBarfrom'./bar';"); + assert_contains(&code, "import{TOKEN}from'./tok';"); + if mode == CompilationMode::Full { + assert_contains(&code, "(i0.ɵɵinject(TOKEN),i0.ɵɵinject(Bar,8))"); + assert_contains(&code, "(i0.ɵɵinjectAttribute(\"x\"))"); + } else { + assert_contains(&code, "type:S,deps:[{token:TOKEN},{token:Bar,optional:true}]"); + } + + // The metadata's `ctorParameters` still names it (`type: Foo`). + let code = compile_with( + DEFAULT_IMPORT_OVERRIDDEN, + &TransformOptions { compilation_mode: mode, ..Default::default() }, + ); + assert_contains(&code, "importFoofrom'./foo';"); + } +} + +const INJECT_TOKENS: &str = " +import { Injectable, Component, Inject, Optional, DOCUMENT } from '@angular/core'; +import { TOKEN, Svc } from './tok'; +import { TOKEN as TK } from './tok'; +import * as t from './tok'; +@Injectable() +export class S { + constructor(@Inject(TOKEN) a: string, @Inject(DOCUMENT) b: Document, c: Svc, @Optional() @Inject(TK) d: any, @Inject(t.TOKEN) e: any) {} +} +@Component({ selector: 'c', template: '' }) +export class C { constructor(@Inject(TOKEN) a: string, @Inject(DOCUMENT) b: any) {} } +"; + +#[test] +fn inject_token_is_kept_as_written() { + for mode in [CompilationMode::Full, CompilationMode::Partial] { + for code in [ + compile_with( + INJECT_TOKENS, + &TransformOptions { compilation_mode: mode, ..Default::default() }, + ), + compile_without_metadata(INJECT_TOKENS, mode), + ] { + // The imports the factory names stay; only the type token is namespaced. + assert_contains(&code, "import{TOKEN}from"); + assert_contains(&code, "import{TOKENasTK}from'./tok';"); + assert_contains(&code, "import*astfrom'./tok';"); + assert_contains(&code, "DOCUMENT}from"); + assert_not_contains(&code, "i1.TOKEN"); + assert_not_contains(&code, "i0.DOCUMENT"); + if mode == CompilationMode::Full { + assert_contains( + &code, + "(i0.ɵɵinject(TOKEN),i0.ɵɵinject(DOCUMENT),i0.ɵɵinject(i1.Svc),i0.ɵɵinject(TK,8),i0.ɵɵinject(t.TOKEN))", + ); + assert_contains( + &code, + "(i0.ɵɵdirectiveInject(TOKEN),i0.ɵɵdirectiveInject(DOCUMENT))", + ); + } else { + assert_contains( + &code, + "deps:[{token:TOKEN},{token:DOCUMENT},{token:i1.Svc},{token:TK,optional:true},{token:t.TOKEN}]", + ); + assert_contains(&code, "type:C,deps:[{token:TOKEN},{token:DOCUMENT}]"); + } + } + } +} diff --git a/crates/oxc_angular_compiler/tests/set_class_metadata_import_elision_test.rs b/crates/oxc_angular_compiler/tests/set_class_metadata_import_elision_test.rs index 2365a38e7..d18474c5b 100644 --- a/crates/oxc_angular_compiler/tests/set_class_metadata_import_elision_test.rs +++ b/crates/oxc_angular_compiler/tests/set_class_metadata_import_elision_test.rs @@ -56,7 +56,8 @@ export class C { } /// Same repro with `emit_class_metadata` off: the decorators are stripped and -/// their imports are elided, as before. +/// their imports are elided, as before. TOKEN stays: like ngtsc, the factory +/// names it as written (`ɵɵdirectiveInject(TOKEN, 8)`). #[test] fn ctor_param_decorators_elided_without_class_metadata() { let source = r#" @@ -76,7 +77,7 @@ export class C { .find(|l| l.starts_with("import") && l.contains("@angular/core") && !l.contains("* as")) .unwrap(); assert_eq!(core_import, "import { Component } from \"@angular/core\";"); - assert!(!code.contains("import {TOKEN}"), "TOKEN should be elided: {code}"); + assert!(code.contains("import {TOKEN} from './tokens'"), "TOKEN import must survive: {code}"); } /// `@Inject(TOKEN)` must also keep the decorator imports for @Directive, @@ -235,5 +236,5 @@ export class C { .unwrap(); assert!(!core_import.contains("Inject"), "Inject should be elided: {code}"); assert!(!core_import.contains("Optional"), "Optional should be elided: {code}"); - assert!(!code.contains("import {TOKEN}"), "TOKEN should be elided: {code}"); + assert!(code.contains("import {TOKEN} from './tokens'"), "TOKEN import must survive: {code}"); } diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__class_metadata_angular_core_decorators.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__class_metadata_angular_core_decorators.snap index 44c772764..98e062da1 100644 --- a/crates/oxc_angular_compiler/tests/snapshots/integration_test__class_metadata_angular_core_decorators.snap +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__class_metadata_angular_core_decorators.snap @@ -26,8 +26,7 @@ export class TestComponent { ) {} static ɵfac = function TestComponent_Factory(__ngFactoryType__) { - return new (__ngFactoryType__ || TestComponent)(i0.ɵɵdirectiveInject(TOKEN),i0.ɵɵinvalidFactoryDep(1), - i0.ɵɵinvalidFactoryDep(2)); + i0.ɵɵinvalidFactory(); }; static ɵcmp = /*@__PURE__*/ i0.ɵɵdefineComponent({type:TestComponent,selectors:[["app-test"]],decls:0, vars:0,template:function TestComponent_Template(rf,ctx) { diff --git a/napi/angular-compiler/test/factory.spec.ts b/napi/angular-compiler/test/factory.spec.ts index 9ca33fe62..dd96494e4 100644 --- a/napi/angular-compiler/test/factory.spec.ts +++ b/napi/angular-compiler/test/factory.spec.ts @@ -207,9 +207,9 @@ describe('compileFactory', () => { const result = await compileFactory(input) expect(result.errors).toHaveLength(0) expect(result.code).toContain('MyService_Factory') - // Invalid dependency calls invalidFactoryDep(index) - expect(result.code).toContain('invalidFactoryDep') - expect(result.code).toContain('0') // index 0 + // Like ngtsc, an invalid dependency makes the whole factory invalid + expect(result.code).toContain('ɵɵinvalidFactory()') + expect(result.code).not.toContain('invalidFactoryDep') }) test('returns error for invalid target', async () => {