diff --git a/crates/oxc_angular_compiler/src/component/decorator.rs b/crates/oxc_angular_compiler/src/component/decorator.rs index 9d810baed..11766888b 100644 --- a/crates/oxc_angular_compiler/src/component/decorator.rs +++ b/crates/oxc_angular_compiler/src/component/decorator.rs @@ -1023,6 +1023,19 @@ fn extract_param_token<'a>(param: &'a oxc_ast::ast::FormalParameter<'a>) -> Opti // Decorator Span Collection for Removal // ============================================================================= +/// Collect all class-level decorator spans. +/// +/// When an Angular class decorator is found (e.g. `@Component`, `@Directive`), ALL class-level +/// decorators must be removed — not just the Angular one. Non-Angular decorators like +/// `@UntilDestroy()` would otherwise remain in front of the compiled class output, producing +/// invalid JavaScript (a decorator on a non-class construct). +/// +/// These spans are used by `transform.rs` to remove the decorators from the +/// source text during transformation. +pub fn collect_all_class_decorator_spans(class: &Class<'_>, spans: &mut std::vec::Vec) { + spans.extend(class.decorators.iter().map(|d| d.span)); +} + /// Collect all decorator spans from constructor parameters. /// /// Parameter decorators like `@Optional()`, `@Inject()`, `@Host()`, `@Self()`, diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 2df3f3e9d..e32633143 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -18,8 +18,9 @@ use rustc_hash::FxHashMap; #[cfg(feature = "cross_file_elision")] use super::cross_file_elision::CrossFileAnalyzer; use super::decorator::{ - collect_constructor_decorator_spans, collect_member_decorator_spans, - extract_component_metadata, find_component_decorator, find_component_decorator_span, + collect_all_class_decorator_spans, collect_constructor_decorator_spans, + collect_member_decorator_spans, extract_component_metadata, find_component_decorator, + find_component_decorator_span, }; use super::definition::{const_value_to_expression, generate_component_definitions}; use super::import_elision::{ImportElisionAnalyzer, filter_imports}; @@ -790,10 +791,13 @@ pub fn transform_angular_file( ); } - // Track the decorator span to remove - if let Some(span) = find_component_decorator_span(class) { - decorator_spans_to_remove.push(span); - } + // Track ALL class decorator spans for removal (Angular + non-Angular). + // Non-Angular decorators (e.g. @UntilDestroy()) must also be removed, + // otherwise they end up decorating the compiled output which is invalid JS. + collect_all_class_decorator_spans( + class, + &mut decorator_spans_to_remove, + ); // Collect constructor parameter decorators (@Optional, @Inject, etc.) collect_constructor_decorator_spans( class, @@ -927,10 +931,8 @@ pub fn transform_angular_file( if let Some(mut directive_metadata) = extract_directive_metadata(allocator, class, implicit_standalone) { - // Track decorator span for removal - if let Some(span) = find_directive_decorator_span(class) { - decorator_spans_to_remove.push(span); - } + // Track ALL class decorator spans for removal (Angular + non-Angular) + collect_all_class_decorator_spans(class, &mut decorator_spans_to_remove); // Collect constructor parameter decorators (@Optional, @Inject, etc.) collect_constructor_decorator_spans(class, &mut decorator_spans_to_remove); // Collect member decorators (@Input, @Output, @HostBinding, etc.) @@ -981,10 +983,8 @@ pub fn transform_angular_file( // - ɵprov: Provider metadata for Angular's DI system // - ɵfac: Factory function to instantiate the class - // Track decorator span for removal - if let Some(span) = find_injectable_decorator_span(class) { - decorator_spans_to_remove.push(span); - } + // Track ALL class decorator spans for removal (Angular + non-Angular) + collect_all_class_decorator_spans(class, &mut decorator_spans_to_remove); // Collect constructor parameter decorators (@Optional, @Inject, etc.) collect_constructor_decorator_spans(class, &mut decorator_spans_to_remove); @@ -1035,10 +1035,8 @@ pub fn transform_angular_file( // - ɵpipe: Pipe definition for Angular's pipe system // - ɵfac: Factory function for dependency injection (when pipe has constructor deps) - // Track decorator span for removal - if let Some(span) = find_pipe_decorator_span(class) { - decorator_spans_to_remove.push(span); - } + // Track ALL class decorator spans for removal (Angular + non-Angular) + collect_all_class_decorator_spans(class, &mut decorator_spans_to_remove); // Collect constructor parameter decorators (@Optional, @Inject, etc.) collect_constructor_decorator_spans(class, &mut decorator_spans_to_remove); @@ -1083,10 +1081,8 @@ pub fn transform_angular_file( // - ɵfac: Factory function for instantiation (with constructor dependencies) // - ɵinj: Injector definition with providers and imports - // Track decorator span for removal - if let Some(span) = find_ng_module_decorator_span(class) { - decorator_spans_to_remove.push(span); - } + // Track ALL class decorator spans for removal (Angular + non-Angular) + collect_all_class_decorator_spans(class, &mut decorator_spans_to_remove); // Collect constructor parameter decorators (@Optional, @Inject, etc.) collect_constructor_decorator_spans(class, &mut decorator_spans_to_remove); @@ -1169,25 +1165,22 @@ pub fn transform_angular_file( _ => None, }; if let Some(class) = class { - // Check for component, directive, injectable, pipe, or ngmodule decorators - // and collect associated parameter/member decorator spans - if let Some(span) = find_component_decorator_span(class) { - new_decorator_spans.push(span); - collect_constructor_decorator_spans(class, &mut new_decorator_spans); - collect_member_decorator_spans(class, &mut new_decorator_spans); - } else if let Some(span) = find_directive_decorator_span(class) { - new_decorator_spans.push(span); - collect_constructor_decorator_spans(class, &mut new_decorator_spans); - collect_member_decorator_spans(class, &mut new_decorator_spans); - } else if let Some(span) = find_injectable_decorator_span(class) { - new_decorator_spans.push(span); - collect_constructor_decorator_spans(class, &mut new_decorator_spans); - } else if let Some(span) = find_pipe_decorator_span(class) { - new_decorator_spans.push(span); - collect_constructor_decorator_spans(class, &mut new_decorator_spans); - } else if let Some(span) = find_ng_module_decorator_span(class) { - new_decorator_spans.push(span); + // Check if this is an Angular-decorated class (component, directive, etc.) + // and collect ALL decorator spans for removal (Angular + non-Angular) + let is_component = find_component_decorator_span(class).is_some(); + let is_directive = !is_component && find_directive_decorator_span(class).is_some(); + let is_angular_class = is_component + || is_directive + || find_injectable_decorator_span(class).is_some() + || find_pipe_decorator_span(class).is_some() + || find_ng_module_decorator_span(class).is_some(); + + if is_angular_class { + collect_all_class_decorator_spans(class, &mut new_decorator_spans); collect_constructor_decorator_spans(class, &mut new_decorator_spans); + if is_component || is_directive { + collect_member_decorator_spans(class, &mut new_decorator_spans); + } } } } diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 1131aec37..ee7be8d6a 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -5376,3 +5376,122 @@ fn test_if_block_no_expression_skips_main_branch() { } assert!(!errors.is_empty(), "Should report a parse error for @if without expression"); } + +/// Regression test for issue #44: Non-Angular class decorators break OXC compiled output. +/// +/// When a class has both Angular decorators (@Component, @Directive, etc.) and non-Angular +/// class decorators (like @UntilDestroy()), the non-Angular decorator must be removed from +/// the output. Otherwise it ends up decorating the compiled class with static ɵcmp/ɵfac fields, +/// which can produce invalid JS (decorators on non-class constructs) or cause esbuild errors. +#[test] +fn test_non_angular_class_decorator_removed_for_component() { + let allocator = Allocator::default(); + let source = r#" +import { Component } from '@angular/core'; + +function UntilDestroy() { + return function(target: any) { return target; }; +} + +@UntilDestroy() +@Component({ + selector: 'test-comp', + template: '
hello
', + standalone: true, +}) +export class TestComponent {} +"#; + + let result = transform_angular_file( + &allocator, + "test.component.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert_eq!(result.component_count, 1); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + // The @UntilDestroy() decorator must NOT appear in the output + assert!( + !result.code.contains("@UntilDestroy"), + "Non-Angular class decorator @UntilDestroy() should be removed from compiled output.\nGot:\n{}", + result.code + ); + // The class should still be present + assert!( + result.code.contains("class TestComponent"), + "Class declaration should still be present in output.\nGot:\n{}", + result.code + ); +} + +/// Same as above but for @Directive classes. +#[test] +fn test_non_angular_class_decorator_removed_for_directive() { + let allocator = Allocator::default(); + let source = r#" +import { Directive } from '@angular/core'; + +function UntilDestroy() { + return function(target: any) { return target; }; +} + +@UntilDestroy() +@Directive({ + selector: '[testDir]', + standalone: true, +}) +export class TestDirective {} +"#; + + let result = transform_angular_file( + &allocator, + "test.directive.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + assert!( + !result.code.contains("@UntilDestroy"), + "Non-Angular class decorator @UntilDestroy() should be removed from directive output.\nGot:\n{}", + result.code + ); +} + +/// Same as above but for @Injectable classes. +#[test] +fn test_non_angular_class_decorator_removed_for_injectable() { + let allocator = Allocator::default(); + let source = r#" +import { Injectable } from '@angular/core'; + +function MyCustomDecorator() { + return function(target: any) { return target; }; +} + +@MyCustomDecorator() +@Injectable({ providedIn: 'root' }) +export class TestService {} +"#; + + let result = transform_angular_file( + &allocator, + "test.service.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + assert!( + !result.code.contains("@MyCustomDecorator"), + "Non-Angular class decorator @MyCustomDecorator() should be removed from injectable output.\nGot:\n{}", + result.code + ); +}