Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions crates/oxc_angular_compiler/src/component/decorator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Span>) {
spans.extend(class.decorators.iter().map(|d| d.span));
}

/// Collect all decorator spans from constructor parameters.
///
/// Parameter decorators like `@Optional()`, `@Inject()`, `@Host()`, `@Self()`,
Expand Down
73 changes: 33 additions & 40 deletions crates/oxc_angular_compiler/src/component/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.)
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
}
}
}
Expand Down
119 changes: 119 additions & 0 deletions crates/oxc_angular_compiler/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: '<div>hello</div>',
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
);
}
Loading