Skip to content

Commit 95ef483

Browse files
Brooooooklynclaude
andauthored
fix(transform): strip non-Angular class decorators from compiled output (#48)
When a class had both Angular decorators (@component, @directive, etc.) and non-Angular class decorators (e.g. @UntilDestroy()), only the Angular decorator was removed during compilation. The non-Angular decorator remained in the output, producing invalid JS since it decorated a compiled class with static ɵcmp/ɵfac fields rather than a plain class declaration. Fix by collecting ALL class-level decorator spans for removal when any Angular decorator is present, instead of only the single Angular one. - Close #44 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e4c6212 commit 95ef483

3 files changed

Lines changed: 165 additions & 40 deletions

File tree

crates/oxc_angular_compiler/src/component/decorator.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,6 +1023,19 @@ fn extract_param_token<'a>(param: &'a oxc_ast::ast::FormalParameter<'a>) -> Opti
10231023
// Decorator Span Collection for Removal
10241024
// =============================================================================
10251025

1026+
/// Collect all class-level decorator spans.
1027+
///
1028+
/// When an Angular class decorator is found (e.g. `@Component`, `@Directive`), ALL class-level
1029+
/// decorators must be removed — not just the Angular one. Non-Angular decorators like
1030+
/// `@UntilDestroy()` would otherwise remain in front of the compiled class output, producing
1031+
/// invalid JavaScript (a decorator on a non-class construct).
1032+
///
1033+
/// These spans are used by `transform.rs` to remove the decorators from the
1034+
/// source text during transformation.
1035+
pub fn collect_all_class_decorator_spans(class: &Class<'_>, spans: &mut std::vec::Vec<Span>) {
1036+
spans.extend(class.decorators.iter().map(|d| d.span));
1037+
}
1038+
10261039
/// Collect all decorator spans from constructor parameters.
10271040
///
10281041
/// Parameter decorators like `@Optional()`, `@Inject()`, `@Host()`, `@Self()`,

crates/oxc_angular_compiler/src/component/transform.rs

Lines changed: 33 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ use rustc_hash::FxHashMap;
1818
#[cfg(feature = "cross_file_elision")]
1919
use super::cross_file_elision::CrossFileAnalyzer;
2020
use super::decorator::{
21-
collect_constructor_decorator_spans, collect_member_decorator_spans,
22-
extract_component_metadata, find_component_decorator, find_component_decorator_span,
21+
collect_all_class_decorator_spans, collect_constructor_decorator_spans,
22+
collect_member_decorator_spans, extract_component_metadata, find_component_decorator,
23+
find_component_decorator_span,
2324
};
2425
use super::definition::{const_value_to_expression, generate_component_definitions};
2526
use super::import_elision::{ImportElisionAnalyzer, filter_imports};
@@ -790,10 +791,13 @@ pub fn transform_angular_file(
790791
);
791792
}
792793

793-
// Track the decorator span to remove
794-
if let Some(span) = find_component_decorator_span(class) {
795-
decorator_spans_to_remove.push(span);
796-
}
794+
// Track ALL class decorator spans for removal (Angular + non-Angular).
795+
// Non-Angular decorators (e.g. @UntilDestroy()) must also be removed,
796+
// otherwise they end up decorating the compiled output which is invalid JS.
797+
collect_all_class_decorator_spans(
798+
class,
799+
&mut decorator_spans_to_remove,
800+
);
797801
// Collect constructor parameter decorators (@Optional, @Inject, etc.)
798802
collect_constructor_decorator_spans(
799803
class,
@@ -927,10 +931,8 @@ pub fn transform_angular_file(
927931
if let Some(mut directive_metadata) =
928932
extract_directive_metadata(allocator, class, implicit_standalone)
929933
{
930-
// Track decorator span for removal
931-
if let Some(span) = find_directive_decorator_span(class) {
932-
decorator_spans_to_remove.push(span);
933-
}
934+
// Track ALL class decorator spans for removal (Angular + non-Angular)
935+
collect_all_class_decorator_spans(class, &mut decorator_spans_to_remove);
934936
// Collect constructor parameter decorators (@Optional, @Inject, etc.)
935937
collect_constructor_decorator_spans(class, &mut decorator_spans_to_remove);
936938
// Collect member decorators (@Input, @Output, @HostBinding, etc.)
@@ -981,10 +983,8 @@ pub fn transform_angular_file(
981983
// - ɵprov: Provider metadata for Angular's DI system
982984
// - ɵfac: Factory function to instantiate the class
983985

984-
// Track decorator span for removal
985-
if let Some(span) = find_injectable_decorator_span(class) {
986-
decorator_spans_to_remove.push(span);
987-
}
986+
// Track ALL class decorator spans for removal (Angular + non-Angular)
987+
collect_all_class_decorator_spans(class, &mut decorator_spans_to_remove);
988988
// Collect constructor parameter decorators (@Optional, @Inject, etc.)
989989
collect_constructor_decorator_spans(class, &mut decorator_spans_to_remove);
990990

@@ -1035,10 +1035,8 @@ pub fn transform_angular_file(
10351035
// - ɵpipe: Pipe definition for Angular's pipe system
10361036
// - ɵfac: Factory function for dependency injection (when pipe has constructor deps)
10371037

1038-
// Track decorator span for removal
1039-
if let Some(span) = find_pipe_decorator_span(class) {
1040-
decorator_spans_to_remove.push(span);
1041-
}
1038+
// Track ALL class decorator spans for removal (Angular + non-Angular)
1039+
collect_all_class_decorator_spans(class, &mut decorator_spans_to_remove);
10421040
// Collect constructor parameter decorators (@Optional, @Inject, etc.)
10431041
collect_constructor_decorator_spans(class, &mut decorator_spans_to_remove);
10441042

@@ -1083,10 +1081,8 @@ pub fn transform_angular_file(
10831081
// - ɵfac: Factory function for instantiation (with constructor dependencies)
10841082
// - ɵinj: Injector definition with providers and imports
10851083

1086-
// Track decorator span for removal
1087-
if let Some(span) = find_ng_module_decorator_span(class) {
1088-
decorator_spans_to_remove.push(span);
1089-
}
1084+
// Track ALL class decorator spans for removal (Angular + non-Angular)
1085+
collect_all_class_decorator_spans(class, &mut decorator_spans_to_remove);
10901086
// Collect constructor parameter decorators (@Optional, @Inject, etc.)
10911087
collect_constructor_decorator_spans(class, &mut decorator_spans_to_remove);
10921088

@@ -1169,25 +1165,22 @@ pub fn transform_angular_file(
11691165
_ => None,
11701166
};
11711167
if let Some(class) = class {
1172-
// Check for component, directive, injectable, pipe, or ngmodule decorators
1173-
// and collect associated parameter/member decorator spans
1174-
if let Some(span) = find_component_decorator_span(class) {
1175-
new_decorator_spans.push(span);
1176-
collect_constructor_decorator_spans(class, &mut new_decorator_spans);
1177-
collect_member_decorator_spans(class, &mut new_decorator_spans);
1178-
} else if let Some(span) = find_directive_decorator_span(class) {
1179-
new_decorator_spans.push(span);
1180-
collect_constructor_decorator_spans(class, &mut new_decorator_spans);
1181-
collect_member_decorator_spans(class, &mut new_decorator_spans);
1182-
} else if let Some(span) = find_injectable_decorator_span(class) {
1183-
new_decorator_spans.push(span);
1184-
collect_constructor_decorator_spans(class, &mut new_decorator_spans);
1185-
} else if let Some(span) = find_pipe_decorator_span(class) {
1186-
new_decorator_spans.push(span);
1187-
collect_constructor_decorator_spans(class, &mut new_decorator_spans);
1188-
} else if let Some(span) = find_ng_module_decorator_span(class) {
1189-
new_decorator_spans.push(span);
1168+
// Check if this is an Angular-decorated class (component, directive, etc.)
1169+
// and collect ALL decorator spans for removal (Angular + non-Angular)
1170+
let is_component = find_component_decorator_span(class).is_some();
1171+
let is_directive = !is_component && find_directive_decorator_span(class).is_some();
1172+
let is_angular_class = is_component
1173+
|| is_directive
1174+
|| find_injectable_decorator_span(class).is_some()
1175+
|| find_pipe_decorator_span(class).is_some()
1176+
|| find_ng_module_decorator_span(class).is_some();
1177+
1178+
if is_angular_class {
1179+
collect_all_class_decorator_spans(class, &mut new_decorator_spans);
11901180
collect_constructor_decorator_spans(class, &mut new_decorator_spans);
1181+
if is_component || is_directive {
1182+
collect_member_decorator_spans(class, &mut new_decorator_spans);
1183+
}
11911184
}
11921185
}
11931186
}

crates/oxc_angular_compiler/tests/integration_test.rs

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5376,3 +5376,122 @@ fn test_if_block_no_expression_skips_main_branch() {
53765376
}
53775377
assert!(!errors.is_empty(), "Should report a parse error for @if without expression");
53785378
}
5379+
5380+
/// Regression test for issue #44: Non-Angular class decorators break OXC compiled output.
5381+
///
5382+
/// When a class has both Angular decorators (@Component, @Directive, etc.) and non-Angular
5383+
/// class decorators (like @UntilDestroy()), the non-Angular decorator must be removed from
5384+
/// the output. Otherwise it ends up decorating the compiled class with static ɵcmp/ɵfac fields,
5385+
/// which can produce invalid JS (decorators on non-class constructs) or cause esbuild errors.
5386+
#[test]
5387+
fn test_non_angular_class_decorator_removed_for_component() {
5388+
let allocator = Allocator::default();
5389+
let source = r#"
5390+
import { Component } from '@angular/core';
5391+
5392+
function UntilDestroy() {
5393+
return function(target: any) { return target; };
5394+
}
5395+
5396+
@UntilDestroy()
5397+
@Component({
5398+
selector: 'test-comp',
5399+
template: '<div>hello</div>',
5400+
standalone: true,
5401+
})
5402+
export class TestComponent {}
5403+
"#;
5404+
5405+
let result = transform_angular_file(
5406+
&allocator,
5407+
"test.component.ts",
5408+
source,
5409+
&ComponentTransformOptions::default(),
5410+
None,
5411+
);
5412+
5413+
assert_eq!(result.component_count, 1);
5414+
assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics);
5415+
5416+
// The @UntilDestroy() decorator must NOT appear in the output
5417+
assert!(
5418+
!result.code.contains("@UntilDestroy"),
5419+
"Non-Angular class decorator @UntilDestroy() should be removed from compiled output.\nGot:\n{}",
5420+
result.code
5421+
);
5422+
// The class should still be present
5423+
assert!(
5424+
result.code.contains("class TestComponent"),
5425+
"Class declaration should still be present in output.\nGot:\n{}",
5426+
result.code
5427+
);
5428+
}
5429+
5430+
/// Same as above but for @Directive classes.
5431+
#[test]
5432+
fn test_non_angular_class_decorator_removed_for_directive() {
5433+
let allocator = Allocator::default();
5434+
let source = r#"
5435+
import { Directive } from '@angular/core';
5436+
5437+
function UntilDestroy() {
5438+
return function(target: any) { return target; };
5439+
}
5440+
5441+
@UntilDestroy()
5442+
@Directive({
5443+
selector: '[testDir]',
5444+
standalone: true,
5445+
})
5446+
export class TestDirective {}
5447+
"#;
5448+
5449+
let result = transform_angular_file(
5450+
&allocator,
5451+
"test.directive.ts",
5452+
source,
5453+
&ComponentTransformOptions::default(),
5454+
None,
5455+
);
5456+
5457+
assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics);
5458+
5459+
assert!(
5460+
!result.code.contains("@UntilDestroy"),
5461+
"Non-Angular class decorator @UntilDestroy() should be removed from directive output.\nGot:\n{}",
5462+
result.code
5463+
);
5464+
}
5465+
5466+
/// Same as above but for @Injectable classes.
5467+
#[test]
5468+
fn test_non_angular_class_decorator_removed_for_injectable() {
5469+
let allocator = Allocator::default();
5470+
let source = r#"
5471+
import { Injectable } from '@angular/core';
5472+
5473+
function MyCustomDecorator() {
5474+
return function(target: any) { return target; };
5475+
}
5476+
5477+
@MyCustomDecorator()
5478+
@Injectable({ providedIn: 'root' })
5479+
export class TestService {}
5480+
"#;
5481+
5482+
let result = transform_angular_file(
5483+
&allocator,
5484+
"test.service.ts",
5485+
source,
5486+
&ComponentTransformOptions::default(),
5487+
None,
5488+
);
5489+
5490+
assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics);
5491+
5492+
assert!(
5493+
!result.code.contains("@MyCustomDecorator"),
5494+
"Non-Angular class decorator @MyCustomDecorator() should be removed from injectable output.\nGot:\n{}",
5495+
result.code
5496+
);
5497+
}

0 commit comments

Comments
 (0)