diff --git a/crates/oxc_angular_compiler/src/component/decorator.rs b/crates/oxc_angular_compiler/src/component/decorator.rs index 6faf86107..0ac348a0a 100644 --- a/crates/oxc_angular_compiler/src/component/decorator.rs +++ b/crates/oxc_angular_compiler/src/component/decorator.rs @@ -131,7 +131,7 @@ pub fn extract_component_metadata<'a>( metadata.encapsulation = extract_encapsulation(&prop.value); } "changeDetection" => { - metadata.change_detection = extract_change_detection(&prop.value); + metadata.change_detection = Some(extract_change_detection(&prop.value)); } "host" => { metadata.host = extract_host_metadata(allocator, &prop.value, consts); @@ -469,13 +469,18 @@ fn extract_change_detection(expr: &Expression<'_>) -> ChangeDetectionStrategy { match expr { Expression::StaticMemberExpression(member) => match member.property.name.as_str() { "OnPush" => ChangeDetectionStrategy::OnPush, + "Eager" => ChangeDetectionStrategy::Eager, + // `Default` (value 1) is the pre-v22 spelling of `Eager`. Keep it + // distinct so partial emit can preserve the author's exact member + // for older Angular targets that lack `Eager`. "Default" => ChangeDetectionStrategy::Default, _ => ChangeDetectionStrategy::default(), }, Expression::NumericLiteral(num) => { - // Angular's numeric values: Default = 0, OnPush = 1 + // Angular v22 numeric values: OnPush = 0, Eager = 1 (Default = 1). match num.value as i32 { - 1 => ChangeDetectionStrategy::OnPush, + 0 => ChangeDetectionStrategy::OnPush, + 1 => ChangeDetectionStrategy::Eager, _ => ChangeDetectionStrategy::default(), } } @@ -1502,12 +1507,14 @@ mod tests { class TestComponent {} "#; assert_metadata(code, |meta| { - assert_eq!(meta.change_detection, ChangeDetectionStrategy::OnPush); + assert_eq!(meta.change_detection, Some(ChangeDetectionStrategy::OnPush)); }); } #[test] - fn test_extract_change_detection_default() { + fn test_extract_change_detection_default_is_distinct() { + // `Default` (value 1) is the pre-v22 spelling of `Eager`. It is kept as + // a distinct variant so partial emit can preserve the author's member. let code = r#" @Component({ selector: 'app-test', @@ -1517,13 +1524,13 @@ mod tests { class TestComponent {} "#; assert_metadata(code, |meta| { - assert_eq!(meta.change_detection, ChangeDetectionStrategy::Default); + assert_eq!(meta.change_detection, Some(ChangeDetectionStrategy::Default)); }); } #[test] - fn test_extract_change_detection_numeric_on_push() { - // Angular uses: Default=0, OnPush=1 + fn test_extract_change_detection_numeric_eager() { + // Angular v22 numeric values: OnPush = 0, Eager = 1. let code = r#" @Component({ selector: 'app-test', @@ -1533,12 +1540,15 @@ mod tests { class TestComponent {} "#; assert_metadata(code, |meta| { - assert_eq!(meta.change_detection, ChangeDetectionStrategy::OnPush); + assert_eq!(meta.change_detection, Some(ChangeDetectionStrategy::Eager)); }); } #[test] - fn test_change_detection_defaults_to_default() { + fn test_change_detection_unspecified_is_none() { + // When the decorator omits `changeDetection`, the metadata leaves it + // `None` so the emitter applies the target version's default (OnPush in + // v22+, Default/Eager before). let code = r#" @Component({ selector: 'app-test', @@ -1547,7 +1557,7 @@ mod tests { class TestComponent {} "#; assert_metadata(code, |meta| { - assert_eq!(meta.change_detection, ChangeDetectionStrategy::Default); + assert_eq!(meta.change_detection, None); }); } @@ -1938,7 +1948,7 @@ mod tests { assert_eq!(meta.styles.len(), 1); assert!(meta.standalone); assert_eq!(meta.encapsulation, ViewEncapsulation::None); - assert_eq!(meta.change_detection, ChangeDetectionStrategy::OnPush); + assert_eq!(meta.change_detection, Some(ChangeDetectionStrategy::OnPush)); assert!(meta.host.is_some()); let host = meta.host.as_ref().unwrap(); assert_eq!(host.class_attr.as_ref().unwrap().as_str(), "app-complete"); diff --git a/crates/oxc_angular_compiler/src/component/definition.rs b/crates/oxc_angular_compiler/src/component/definition.rs index 63d816c23..5defdf772 100644 --- a/crates/oxc_angular_compiler/src/component/definition.rs +++ b/crates/oxc_angular_compiler/src/component/definition.rs @@ -485,39 +485,36 @@ fn generate_cmp_definition<'a>( )); } - // 24. changeDetection: ChangeDetectionStrategy.OnPush - only emit if not Default - // (to match TypeScript compiler behavior) - // Per Angular compiler.ts lines 334-346 - // Angular enum values: OnPush = 0, Default = 1 - // NOTE: Angular emits without namespace prefix (ChangeDetectionStrategy.OnPush, not i0.ChangeDetectionStrategy.OnPush) - if metadata.change_detection != ChangeDetectionStrategy::Default { - let strategy_name = match metadata.change_detection { - ChangeDetectionStrategy::Default => "Default", - ChangeDetectionStrategy::OnPush => "OnPush", + // 24. changeDetection — emit the resolved numeric value only when the + // explicit strategy differs from the value the runtime assumes when the + // field is omitted. That default flipped in v22: + // - v22+: default is OnPush (0); runtime `onPush = cd !== Eager`. + // - < v22: default is Default/Eager (1); runtime `onPush = cd === OnPush`. + // So pre-v22 emits `0` for an explicit OnPush while v22 emits `1` for Eager; + // each version omits its own default. Enum values: OnPush = 0, Eager = 1. + if let Some(strategy) = metadata.change_detection { + let value = match strategy { + ChangeDetectionStrategy::OnPush => 0, + // `Eager` and `Default` are the same numeric value (1); the AOT + // output is numeric, so the spelling distinction does not matter here. + ChangeDetectionStrategy::Eager | ChangeDetectionStrategy::Default => 1, }; - // Build: ChangeDetectionStrategy.OnPush (no i0 prefix) - // ReadPropExpr { receiver: ReadVarExpr("ChangeDetectionStrategy"), name: "OnPush" } - let change_detection_strategy_expr = OutputExpression::ReadVar(Box::new_in( - ReadVarExpr { - name: Ident::from(Identifiers::CHANGE_DETECTION_STRATEGY), - source_span: None, - }, - allocator, - )); - let strategy_value_expr = OutputExpression::ReadProp(Box::new_in( - ReadPropExpr { - receiver: Box::new_in(change_detection_strategy_expr, allocator), - name: Ident::from(strategy_name), - optional: false, - source_span: None, - }, - allocator, - )); - entries.push(LiteralMapEntry::new( - Ident::from("changeDetection"), - strategy_value_expr, - false, - )); + let omitted_default = + if job.angular_version.map_or(true, |v| v.change_detection_default_is_on_push()) { + 0 // v22+: OnPush is the default + } else { + 1 // < v22: Default/Eager is the default + }; + if value != omitted_default { + entries.push(LiteralMapEntry::new( + Ident::from("changeDetection"), + OutputExpression::Literal(Box::new_in( + LiteralExpr { value: LiteralValue::Number(value as f64), source_span: None }, + allocator, + )), + false, + )); + } } // Create the config object diff --git a/crates/oxc_angular_compiler/src/component/hoist.rs b/crates/oxc_angular_compiler/src/component/hoist.rs index e070ac1eb..3de1682d8 100644 --- a/crates/oxc_angular_compiler/src/component/hoist.rs +++ b/crates/oxc_angular_compiler/src/component/hoist.rs @@ -46,6 +46,8 @@ use oxc_syntax::symbol::SymbolId; use crate::optimizer::Edit; +use super::transform::{ImportMap, is_angular_core_export, is_angular_core_namespace}; + /// Per-statement record collected during the initial scan. Multi-declarator /// statements (`const A = 1, B = 2;`) get a single entry shared by every /// symbol they bind; `init_symbols` is the union of identifier references @@ -95,6 +97,7 @@ pub fn collect_hoist_edits<'a>( program: &Program<'a>, source: &str, semantic: &Semantic<'a>, + import_map: &ImportMap<'a>, ) -> Vec { // Step 1: index top-level bindings (keyed by SymbolId). // - `symbol_to_stmt`: binding SymbolId → containing statement's `start`. @@ -159,7 +162,7 @@ pub fn collect_hoist_edits<'a>( let mut classes: Vec<(&Class<'a>, u32, HashSet, HashSet)> = Vec::new(); for stmt in &program.body { let Some((class, stmt_start_pos)) = class_of(stmt) else { continue }; - if !has_angular_decorator(class) { + if !has_hoistable_angular_decorator(class, import_map) { continue; } let mut direct: HashSet = HashSet::new(); @@ -887,6 +890,11 @@ fn class_of<'a, 'src>(stmt: &'src Statement<'a>) -> Option<(&'src Class<'a>, u32 /// Does this class carry any decorator that Angular's compiler emits eager /// definitions for? We don't try to be precise here — any of the well-known /// Angular decorators makes the class a candidate. +/// +/// Name-only and import-agnostic: used by the cheap pre-check +/// [`program_has_angular_decorated_class`], where a false positive only costs +/// a wasted scan. The actual hoist filter uses the import-aware +/// [`has_hoistable_angular_decorator`]. fn has_angular_decorator(class: &Class<'_>) -> bool { class.decorators.iter().any(|d| { let callee = match &d.expression { @@ -898,7 +906,48 @@ fn has_angular_decorator(class: &Class<'_>) -> bool { Expression::StaticMemberExpression(member) => member.property.name.as_str(), _ => return false, }; - matches!(name, "Component" | "Directive" | "Pipe" | "NgModule" | "Injectable") + matches!(name, "Component" | "Directive" | "Pipe" | "NgModule" | "Injectable" | "Service") + }) +} + +/// Whether this class carries an Angular decorator that warrants hoisting its +/// referenced declarations. +/// +/// Like [`has_angular_decorator`], but verifies `@Service` resolves to +/// `@angular/core` before treating it as Angular. `Service` is a common name +/// in non-Angular code, and hoisting a third-party `@Service` class's +/// referenced declarations would reorder statements and change that class's +/// runtime evaluation semantics. The other decorator names predate this check +/// and stay name-only — over-triggering there is the long-standing behavior and +/// only ever hoists a TDZ-safe declaration earlier. +fn has_hoistable_angular_decorator<'a>(class: &Class<'a>, import_map: &ImportMap<'a>) -> bool { + class.decorators.iter().any(|d| { + let callee = match &d.expression { + Expression::CallExpression(call) => &call.callee, + expr => expr, + }; + match callee { + Expression::Identifier(id) => { + let name = id.name.as_str(); + if name == "Service" { + is_angular_core_export(import_map, name, "Service") + } else { + matches!(name, "Component" | "Directive" | "Pipe" | "NgModule" | "Injectable") + } + } + Expression::StaticMemberExpression(member) => { + let name = member.property.name.as_str(); + if name == "Service" { + // `@ns.Service()` — accept only when `ns` is a namespace + // import from `@angular/core`. + matches!(&member.object, Expression::Identifier(ns) + if is_angular_core_namespace(import_map, ns.name.as_str())) + } else { + matches!(name, "Component" | "Directive" | "Pipe" | "NgModule" | "Injectable") + } + } + _ => false, + } }) } @@ -907,7 +956,7 @@ fn has_angular_decorator(class: &Class<'_>) -> bool { /// /// Used by the AOT transform pipeline to skip the `Semantic` build and the /// full hoist scan for files with no decorated classes (plain TS helpers, -/// type-only modules, services without `@Injectable`, …). This walks +/// type-only modules, classes with no Angular decorator, …). This walks /// `program.body` only and never descends into class bodies or expressions, /// so it's O(top-level statements) with a tiny per-statement cost. pub(crate) fn program_has_angular_decorated_class(program: &Program<'_>) -> bool { diff --git a/crates/oxc_angular_compiler/src/component/metadata.rs b/crates/oxc_angular_compiler/src/component/metadata.rs index 67c956960..c2a2ec272 100644 --- a/crates/oxc_angular_compiler/src/component/metadata.rs +++ b/crates/oxc_angular_compiler/src/component/metadata.rs @@ -72,6 +72,19 @@ impl AngularVersion { self.major >= 22 } + /// Whether the default component change-detection strategy is `OnPush` + /// (v22.0.0+). + /// + /// Angular v22 made `OnPush` (0) the default strategy and added `Eager` (1). + /// The runtime computes `onPush = changeDetection !== Eager`, so the full + /// definition omits `changeDetection` for `OnPush` and emits `1` for `Eager`. + /// Before v22 the default was `Default`/`Eager` (1) and the runtime computed + /// `onPush = changeDetection === OnPush`, so the definition instead omits the + /// default and emits `0` for an explicit `OnPush`. + pub fn change_detection_default_is_on_push(&self) -> bool { + self.major >= 22 + } + /// Check if this version emits control instructions for the extended set of /// control properties (v22.0.0+). /// @@ -163,11 +176,21 @@ pub enum ViewEncapsulation { /// Matches Angular's `ChangeDetectionStrategy` enum. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum ChangeDetectionStrategy { - /// Check the component on every change detection cycle. + /// Only check when inputs change or events are triggered. Angular v22's + /// default strategy (`OnPush = 0`). #[default] - Default, - /// Only check when inputs change or events are triggered. OnPush, + /// Check the component on every change detection cycle (`Eager = 1`). The + /// member introduced in Angular v22 to replace `Default`. + Eager, + /// Check the component on every change detection cycle. The pre-v22 spelling + /// (`Default = 1`), kept distinct from [`Eager`] so partial declarations + /// preserve the exact `ChangeDetectionStrategy` member the author wrote — + /// emitting `Eager` for an older Angular target would reference a member + /// that does not exist there. + /// + /// [`Eager`]: ChangeDetectionStrategy::Eager + Default, } /// Metadata extracted from an `@Component` decorator. @@ -203,8 +226,9 @@ pub struct ComponentMetadata<'a> { /// View encapsulation mode. pub encapsulation: ViewEncapsulation, - /// Change detection strategy. - pub change_detection: ChangeDetectionStrategy, + /// Change detection strategy, or `None` when the decorator does not specify + /// one (so the runtime's version-dependent default applies). + pub change_detection: Option, /// Host bindings and listeners. pub host: Option>, @@ -614,7 +638,7 @@ impl<'a> ComponentMetadata<'a> { style_urls: Vec::new_in(allocator), standalone: implicit_standalone, encapsulation: ViewEncapsulation::default(), - change_detection: ChangeDetectionStrategy::default(), + change_detection: None, host: None, imports: Vec::new_in(allocator), export_as: Vec::new_in(allocator), diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 0198f24b5..039b1d9d6 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -960,7 +960,7 @@ fn find_angular_service_decorator<'a>( /// the local binding is `Service`. Bare `import { Service }` (no alias) /// passes because `imported_name` is `None`, meaning the local name and the /// exported name agree. -fn is_angular_core_export( +pub(crate) fn is_angular_core_export( import_map: &ImportMap<'_>, local_name: &str, exported_name: &str, @@ -979,7 +979,7 @@ fn is_angular_core_export( /// from `@angular/core`. Used to validate namespace-style decorator calls /// like `@ns.Service()` — without this check, any third-party namespace /// `Service` decorator would classify as the Angular v22 decorator. -fn is_angular_core_namespace(import_map: &ImportMap<'_>, local_name: &str) -> bool { +pub(crate) fn is_angular_core_namespace(import_map: &ImportMap<'_>, local_name: &str) -> bool { import_map .get(&Ident::from(local_name)) .map(|info| info.source_module.as_str() == "@angular/core" && !info.is_named_import) @@ -3511,7 +3511,12 @@ pub fn transform_angular_file( // The JIT path (see ~line 1380) follows the same convention. let hoist_semantic = oxc_semantic::SemanticBuilder::new().build(&parser_ret.program).semantic; - edits.extend(collect_hoist_edits(&parser_ret.program, source, &hoist_semantic)); + edits.extend(collect_hoist_edits( + &parser_ret.program, + source, + &hoist_semantic, + &import_map, + )); } // Apply all edits in one pass diff --git a/crates/oxc_angular_compiler/src/linker/mod.rs b/crates/oxc_angular_compiler/src/linker/mod.rs index ae2f045a6..a7a3771c6 100644 --- a/crates/oxc_angular_compiler/src/linker/mod.rs +++ b/crates/oxc_angular_compiler/src/linker/mod.rs @@ -2059,13 +2059,16 @@ fn link_component( parts.push(format!("data: {{ animation: {animations} }}")); } - // 22. changeDetection - if let Some(cd) = get_property_source(meta, "changeDetection", source) - && cd.contains("OnPush") - { - parts.push("changeDetection: 0".to_string()); + // 22. changeDetection — resolve the enum member to its numeric value. + // Angular v22: OnPush = 0, Eager = 1 (Default is a deprecated alias for + // Eager). The runtime computes `onPush = changeDetection !== Eager`. + if let Some(cd) = get_property_source(meta, "changeDetection", source) { + if cd.contains("Eager") || cd.contains("Default") { + parts.push("changeDetection: 1".to_string()); + } else if cd.contains("OnPush") { + parts.push("changeDetection: 0".to_string()); + } } - // Default (1) is the default, no need to emit let define_component = format!("{ns}.\u{0275}\u{0275}defineComponent({{ {} }})", parts.join(", ")); @@ -2747,6 +2750,26 @@ MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: " ); } + #[test] + fn test_link_component_with_eager_change_detection() { + // Angular v22's `Eager` strategy must resolve to the numeric `1` so the + // runtime computes `onPush = (1 !== Eager) = false`. + let allocator = Allocator::default(); + let code = r#" +import * as i0 from "@angular/core"; +class MyComponent { +} +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "22.0.0", ngImport: i0, type: MyComponent, selector: "my-comp", template: "
", changeDetection: i0.ChangeDetectionStrategy.Eager }); +"#; + let result = link(&allocator, code, "test.mjs"); + assert!(result.linked); + assert!( + result.code.contains("changeDetection: 1"), + "ChangeDetectionStrategy.Eager should resolve to 1, got:\n{}", + result.code + ); + } + #[test] fn test_link_component_with_host_attrs() { let allocator = Allocator::default(); diff --git a/crates/oxc_angular_compiler/src/partial/component.rs b/crates/oxc_angular_compiler/src/partial/component.rs index 9f6931990..41d281acb 100644 --- a/crates/oxc_angular_compiler/src/partial/component.rs +++ b/crates/oxc_angular_compiler/src/partial/component.rs @@ -267,12 +267,20 @@ pub fn compile_declare_component_from_metadata<'a>( )); } - // changeDetection: emit only when != Default (Default is the runtime - // default and matches upstream's "null" handling). - if meta.change_detection != ChangeDetectionStrategy::default() { - let variant = match meta.change_detection { - ChangeDetectionStrategy::Default => "Default", + // changeDetection: emit the symbolic enum member whenever the decorator + // specifies one (matching Angular's partial compiler, which emits it when + // `changeDetection !== null`). The version-dependent "omit the default" rule + // is applied later, when the linker fully compiles the declaration against + // the consuming app's Angular version. + if let Some(strategy) = meta.change_detection { + // Preserve the exact member the author wrote: `Eager` and `Default` + // share a numeric value but are different symbols, and a partial + // declaration targeting a pre-v22 Angular must not reference `Eager` + // (which does not exist there). + let variant = match strategy { ChangeDetectionStrategy::OnPush => "OnPush", + ChangeDetectionStrategy::Eager => "Eager", + ChangeDetectionStrategy::Default => "Default", }; entries.push(LiteralMapEntry::new( Ident::from("changeDetection"), diff --git a/crates/oxc_angular_compiler/tests/change_detection_eager_test.rs b/crates/oxc_angular_compiler/tests/change_detection_eager_test.rs new file mode 100644 index 000000000..6e8befe25 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/change_detection_eager_test.rs @@ -0,0 +1,131 @@ +//! Version-gated `changeDetection` emit. +//! +//! Angular reworked `ChangeDetectionStrategy` in v22: `OnPush = 0` became the +//! default and `Eager = 1` was added (`Default = 1` is a deprecated alias). The +//! full definition emits the numeric value only when it differs from the +//! value the runtime assumes for an omitted field, and that default flipped: +//! +//! - v22+: default is `OnPush` (0); emit `1` for `Eager`, omit `OnPush`. +//! Runtime: `onPush = changeDetection !== Eager`. +//! - < v22: default is `Default`/`Eager` (1); emit `0` for `OnPush`, omit the +//! default. Runtime: `onPush = changeDetection === OnPush`. +//! +//! So the emit must be gated on the target Angular version, or one regime +//! silently breaks the other. + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{AngularVersion, TransformOptions, transform_angular_file}; + +fn compile(source: &str, version: Option) -> String { + let allocator = Allocator::default(); + let options = TransformOptions { angular_version: version, ..Default::default() }; + let result = + transform_angular_file(&allocator, "test.component.ts", source, Some(&options), None); + assert!(!result.has_errors(), "unexpected errors: {:?}", result.diagnostics); + result.code +} + +/// The `ɵɵdefineComponent` config region, excluding the trailing +/// `ɵsetClassMetadata` (which keeps the symbolic strategy regardless). +fn define_region(code: &str) -> String { + let start = code.find("\u{0275}\u{0275}defineComponent").expect("defineComponent"); + let end = code.find("setClassMetadata").unwrap_or(code.len()); + code[start..end].to_string() +} + +fn component(field: &str) -> String { + format!( + "import {{ Component, ChangeDetectionStrategy }} from '@angular/core';\n\ + @Component({{ selector: 'app-x', template: ''{field} }})\n\ + export class X {{}}\n" + ) +} + +fn has_change_detection(region: &str, value: u32) -> bool { + region.contains(&format!("changeDetection:{value}")) + || region.contains(&format!("changeDetection: {value}")) +} + +const V22: Option = Some(AngularVersion { major: 22, minor: 0, patch: 0 }); +const V21: Option = Some(AngularVersion { major: 21, minor: 0, patch: 0 }); + +// --- v22+ (and unknown version, which assumes latest) ----------------------- + +#[test] +fn v22_eager_emits_one() { + let region = define_region(&compile( + &component(", changeDetection: ChangeDetectionStrategy.Eager"), + V22, + )); + assert!(has_change_detection(®ion, 1), "v22 Eager should emit 1: {region}"); +} + +#[test] +fn v22_default_alias_emits_one() { + let region = define_region(&compile( + &component(", changeDetection: ChangeDetectionStrategy.Default"), + V22, + )); + assert!(has_change_detection(®ion, 1), "v22 Default (==Eager) should emit 1: {region}"); +} + +#[test] +fn v22_on_push_is_omitted() { + let region = define_region(&compile( + &component(", changeDetection: ChangeDetectionStrategy.OnPush"), + V22, + )); + assert!( + !region.contains("changeDetection"), + "v22 OnPush (default) should be omitted: {region}" + ); +} + +#[test] +fn unknown_version_defaults_to_v22_behavior() { + let region = define_region(&compile( + &component(", changeDetection: ChangeDetectionStrategy.Eager"), + None, + )); + assert!( + has_change_detection(®ion, 1), + "unknown version should assume latest (emit 1): {region}" + ); +} + +// --- pre-v22 (Angular 21) — the backward-compatibility cases ---------------- + +#[test] +fn v21_on_push_emits_zero() { + // The regression guard: a pre-v22 explicit OnPush must emit `0`, otherwise + // the 17-21 runtime (`onPush = cd === OnPush`) treats the omitted field as + // eager and the component silently loses OnPush. + let region = define_region(&compile( + &component(", changeDetection: ChangeDetectionStrategy.OnPush"), + V21, + )); + assert!(has_change_detection(®ion, 0), "pre-v22 OnPush should emit 0: {region}"); +} + +#[test] +fn v21_default_is_omitted() { + let region = define_region(&compile( + &component(", changeDetection: ChangeDetectionStrategy.Default"), + V21, + )); + assert!( + !region.contains("changeDetection"), + "pre-v22 Default (default) should be omitted: {region}" + ); +} + +#[test] +fn unspecified_is_omitted_on_both_versions() { + for version in [V22, V21, None] { + let region = define_region(&compile(&component(""), version)); + assert!( + !region.contains("changeDetection"), + "an unspecified strategy must be omitted (version {version:?}): {region}" + ); + } +} diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 9d61ce174..1578824ad 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -11328,6 +11328,59 @@ export class TestComponent {} assert!(token_pos < class_pos, "Order should be preserved.\nCode:\n{}", result.code); } +/// A third-party `@Service` decorator (NOT imported from `@angular/core`) must +/// not trigger hoisting. `Service` is a common name in non-Angular DI +/// frameworks, and reordering such a class's referenced declarations would +/// change that class's runtime evaluation order. The hoist filter verifies the +/// `@Service` import resolves to `@angular/core` before acting. (PR #360 review) +#[test] +fn third_party_service_decorator_does_not_hoist_referenced_const() { + let allocator = Allocator::default(); + let source = r#" +import { Service } from './di'; +@Service(CONFIG) +export class MyService {} +const CONFIG = { name: 'svc' }; +"#; + let result = transform_angular_file(&allocator, "my.service.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + // The const must stay where the author wrote it — after the class. + let config_pos = result.code.find("const CONFIG").expect("CONFIG missing"); + let class_pos = result.code.find("class MyService").expect("class missing"); + assert!( + class_pos < config_pos, + "Third-party @Service must NOT hoist `const CONFIG` above the class. \ + class@{class_pos} config@{config_pos}\nCode:\n{}", + result.code + ); +} + +/// The companion to the above: a genuine `@angular/core` `@Service` whose +/// metadata references a later-declared const still hoists it, so the +/// import-aware gate does not regress real Angular services. +#[test] +fn angular_core_service_decorator_hoists_referenced_const() { + let allocator = Allocator::default(); + let source = r#" +import { Service } from '@angular/core'; +@Service({ autoProvided: FLAG }) +export class MyService {} +const FLAG = false; +"#; + let result = transform_angular_file(&allocator, "my.service.ts", source, None, None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let flag_pos = result.code.find("const FLAG").expect("FLAG missing"); + let class_pos = result.code.find("class MyService").expect("class missing"); + assert!( + flag_pos < class_pos, + "@angular/core @Service must hoist `const FLAG` above the class. \ + flag@{flag_pos} class@{class_pos}\nCode:\n{}", + result.code + ); +} + /// When two bindings from the *same* /// multi-declarator statement (`const A = 1, B = 2;`) are referenced by /// different decorated classes, the hoist plan keys entries by binding name, diff --git a/crates/oxc_angular_compiler/tests/partial_service_test.rs b/crates/oxc_angular_compiler/tests/partial_service_test.rs new file mode 100644 index 000000000..deff741d1 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/partial_service_test.rs @@ -0,0 +1,65 @@ +//! Tests for the partial-declaration Service linker (`ɵɵngDeclareService`), +//! the partial form of Angular v22's `@Service` decorator. +//! +//! Each case pins the linked output against Angular's `compileService` +//! (packages/compiler/src/service_compiler.ts) goldens: the factory delegates +//! to the class `ɵfac` unless an explicit `factory` is declared, and +//! `autoProvided` is emitted only when explicitly `false`. + +use oxc_allocator::Allocator; +use oxc_angular_compiler::linker::link; + +/// Build a `ɵɵngDeclareService` partial declaration with extra metadata props +/// appended after `type` (e.g. `, factory: ...` or `, autoProvided: false`). +fn declare_service(extra: &str) -> String { + format!( + r#"import * as i0 from "@angular/core"; +export class MyService {{}} +MyService.ɵfac = function MyService_Factory(__ngFactoryType__) {{ return new (__ngFactoryType__ || MyService)(); }}; +MyService.ɵprov = i0.ɵɵngDeclareService({{ minVersion: "22.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyService{extra} }});"# + ) +} + +#[test] +fn links_default_service_to_define_service() { + let allocator = Allocator::default(); + let result = link(&allocator, &declare_service(""), "test.mjs"); + assert!( + result.code.contains( + "i0.\u{0275}\u{0275}defineService({ token: MyService, factory: MyService.\u{0275}fac })" + ), + "expected default defineService delegating to ɵfac, got: {}", + result.code + ); + // The partial declaration must be linked away (no JIT fallback at runtime). + assert!( + !result.code.contains("ngDeclareService"), + "ɵɵngDeclareService must be replaced, got: {}", + result.code + ); +} + +#[test] +fn links_service_with_explicit_factory() { + let allocator = Allocator::default(); + let result = link(&allocator, &declare_service(", factory: () => new Alternate()"), "test.mjs"); + assert!( + result.code.contains("factory: () => (() => new Alternate())()"), + "expected the declared factory wrapped in an invoking arrow, got: {}", + result.code + ); + assert!(!result.code.contains("ngDeclareService"), "got: {}", result.code); +} + +#[test] +fn links_service_with_auto_provided_false() { + let allocator = Allocator::default(); + let result = link(&allocator, &declare_service(", autoProvided: false"), "test.mjs"); + assert!( + result.code.contains( + "i0.\u{0275}\u{0275}defineService({ token: MyService, factory: MyService.\u{0275}fac, autoProvided: false })" + ), + "expected autoProvided: false to be preserved, got: {}", + result.code + ); +} diff --git a/crates/oxc_angular_compiler/tests/partial_upstream_parity_test.rs b/crates/oxc_angular_compiler/tests/partial_upstream_parity_test.rs index d2b5b385c..dca81b0df 100644 --- a/crates/oxc_angular_compiler/tests/partial_upstream_parity_test.rs +++ b/crates/oxc_angular_compiler/tests/partial_upstream_parity_test.rs @@ -272,3 +272,35 @@ export class MyService {} "ClassMetadata decorators array should reference the decorator class. Got:\n{code}" ); } + +// ============================================================================ +// ChangeDetection parity (Angular v22 `Eager` / `OnPush`) +// ============================================================================ + +#[test] +fn change_detection_partial_emit_matches_upstream() { + // Angular's partial compiler emits the symbolic enum member whenever + // `changeDetection` is specified (partial/component.ts:109-118). The + // version-dependent "omit the default" rule is applied later — at link / + // full-compile time, against the consuming app's Angular version — so the + // partial declaration must preserve the author's strategy verbatim. + let allocator = Allocator::default(); + for (strategy, expected) in [ + ("Eager", "changeDetection:i0.ChangeDetectionStrategy.Eager"), + ("OnPush", "changeDetection:i0.ChangeDetectionStrategy.OnPush"), + // `Default` must be preserved as-is, not normalized to `Eager`: a + // partial lib built for a pre-v22 Angular has no `Eager` member. + ("Default", "changeDetection:i0.ChangeDetectionStrategy.Default"), + ] { + let source = format!( + "import {{ Component, ChangeDetectionStrategy }} from '@angular/core';\n\ + @Component({{ selector: 'app-x', template: '
', standalone: false, changeDetection: ChangeDetectionStrategy.{strategy} }})\n\ + export class X {{}}\n" + ); + let code = compile_partial(&allocator, "x.component.ts", &source); + assert!( + contains_collapsed(&code, expected), + "partial changeDetection for {strategy} should match upstream's symbolic emit. Got:\n{code}" + ); + } +} diff --git a/crates/oxc_angular_compiler/tests/service_decorator_aot_test.rs b/crates/oxc_angular_compiler/tests/service_decorator_aot_test.rs new file mode 100644 index 000000000..d314c6642 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/service_decorator_aot_test.rs @@ -0,0 +1,49 @@ +//! Regression: a class whose only Angular decorator is `@Service` (Angular v22) +//! must be AOT-compiled (emitting `ɵfac` + `ɵɵdefineService`), not left for the +//! JIT compiler. +//! +//! The cheap pre-check `has_angular_decorator` previously omitted `Service`, so +//! a `@Service`-only file skipped Angular compilation entirely and the decorator +//! was downleveled to a runtime `_decorate([...])` call — which triggers +//! `ɵɵngDeclareService` → JIT fallback at runtime +//! ("service needs the JIT compiler, '@angular/compiler' is not available"). + +use oxc_allocator::Allocator; +use oxc_angular_compiler::transform_angular_file; + +#[test] +fn compiles_service_only_file_to_aot_definitions() { + let allocator = Allocator::default(); + let source = r#" +import { ErrorHandler, Service, inject } from '@angular/core'; +import { AnalyticsService } from './analytics'; + +@Service({ autoProvided: false }) +export class AnalyticsErrorReportHandler extends ErrorHandler { + private _analytics = inject(AnalyticsService); +} +"#; + + let result = transform_angular_file(&allocator, "error-report-handler.ts", source, None, None); + assert!(!result.has_errors(), "unexpected errors: {:?}", result.diagnostics); + let code = &result.code; + + // AOT definitions must be emitted. + assert!( + code.contains("\u{0275}\u{0275}defineService"), + "expected ɵɵdefineService (AOT), got:\n{code}" + ); + assert!(code.contains("\u{0275}fac"), "expected ɵfac factory, got:\n{code}"); + // Emitted JS is compact (no space after the colon). + assert!( + code.contains("autoProvided:false"), + "expected autoProvided:false to be preserved, got:\n{code}" + ); + + // The `@Service` decorator must be compiled away — not downleveled to a + // runtime decorator call (which would fall back to JIT at runtime). + assert!( + !code.contains("_decorate("), + "@Service must be compiled away, not left as a runtime decorator, got:\n{code}" + ); +} diff --git a/napi/angular-compiler/src/lib.rs b/napi/angular-compiler/src/lib.rs index e658a9e2c..d692cbeca 100644 --- a/napi/angular-compiler/src/lib.rs +++ b/napi/angular-compiler/src/lib.rs @@ -295,11 +295,13 @@ fn parse_view_encapsulation(s: &str) -> Option { /// Parse a ChangeDetectionStrategy string to the Rust enum. /// -/// Valid values: "Default", "OnPush" +/// Valid values: "OnPush", "Eager", and "Default" (the pre-v22 spelling of +/// "Eager", kept distinct so partial emit preserves the author's member). fn parse_change_detection_strategy(s: &str) -> Option { match s { - "Default" => Some(RustChangeDetectionStrategy::Default), "OnPush" => Some(RustChangeDetectionStrategy::OnPush), + "Eager" => Some(RustChangeDetectionStrategy::Eager), + "Default" => Some(RustChangeDetectionStrategy::Default), _ => None, } } @@ -1592,10 +1594,14 @@ pub fn extract_component_metadata_sync( } .to_string(); - // Convert change detection to string + // Convert change detection to string. `None` (unspecified) maps + // to "" so it round-trips back to `None` via + // `parse_change_detection_strategy` on the HMR re-compile. let change_detection = match metadata.change_detection { - RustChangeDetection::Default => "Default", - RustChangeDetection::OnPush => "OnPush", + Some(RustChangeDetection::OnPush) => "OnPush", + Some(RustChangeDetection::Eager) => "Eager", + Some(RustChangeDetection::Default) => "Default", + None => "", } .to_string(); diff --git a/napi/angular-compiler/vite-plugin/index.ts b/napi/angular-compiler/vite-plugin/index.ts index 758aba31b..31df1f320 100644 --- a/napi/angular-compiler/vite-plugin/index.ts +++ b/napi/angular-compiler/vite-plugin/index.ts @@ -623,13 +623,14 @@ export function angular(options: PluginOptions = {}): Plugin[] { } // Quick check for Angular decorators - avoids parsing files without them - // OXC handles @Component, @Directive, @NgModule, @Injectable, and @Pipe + // OXC handles @Component, @Directive, @NgModule, @Injectable, @Pipe, @Service const hasAngularDecorator = code.includes('@Component') || code.includes('@Directive') || code.includes('@NgModule') || code.includes('@Injectable') || - code.includes('@Pipe') + code.includes('@Pipe') || + code.includes('@Service') if (!hasAngularDecorator) { return }