Skip to content

Commit b296ec2

Browse files
committed
Revert "fix(transform): strip non-Angular class decorators from compiled output (#48)"
This reverts commit 95ef483.
1 parent 95ef483 commit b296ec2

3 files changed

Lines changed: 40 additions & 165 deletions

File tree

crates/oxc_angular_compiler/src/component/decorator.rs

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,19 +1023,6 @@ 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-
10391026
/// Collect all decorator spans from constructor parameters.
10401027
///
10411028
/// Parameter decorators like `@Optional()`, `@Inject()`, `@Host()`, `@Self()`,

crates/oxc_angular_compiler/src/component/transform.rs

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,8 @@ use rustc_hash::FxHashMap;
1818
#[cfg(feature = "cross_file_elision")]
1919
use super::cross_file_elision::CrossFileAnalyzer;
2020
use super::decorator::{
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,
21+
collect_constructor_decorator_spans, collect_member_decorator_spans,
22+
extract_component_metadata, find_component_decorator, find_component_decorator_span,
2423
};
2524
use super::definition::{const_value_to_expression, generate_component_definitions};
2625
use super::import_elision::{ImportElisionAnalyzer, filter_imports};
@@ -791,13 +790,10 @@ pub fn transform_angular_file(
791790
);
792791
}
793792

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-
);
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+
}
801797
// Collect constructor parameter decorators (@Optional, @Inject, etc.)
802798
collect_constructor_decorator_spans(
803799
class,
@@ -931,8 +927,10 @@ pub fn transform_angular_file(
931927
if let Some(mut directive_metadata) =
932928
extract_directive_metadata(allocator, class, implicit_standalone)
933929
{
934-
// Track ALL class decorator spans for removal (Angular + non-Angular)
935-
collect_all_class_decorator_spans(class, &mut decorator_spans_to_remove);
930+
// Track decorator span for removal
931+
if let Some(span) = find_directive_decorator_span(class) {
932+
decorator_spans_to_remove.push(span);
933+
}
936934
// Collect constructor parameter decorators (@Optional, @Inject, etc.)
937935
collect_constructor_decorator_spans(class, &mut decorator_spans_to_remove);
938936
// Collect member decorators (@Input, @Output, @HostBinding, etc.)
@@ -983,8 +981,10 @@ pub fn transform_angular_file(
983981
// - ɵprov: Provider metadata for Angular's DI system
984982
// - ɵfac: Factory function to instantiate the class
985983

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

@@ -1035,8 +1035,10 @@ 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 ALL class decorator spans for removal (Angular + non-Angular)
1039-
collect_all_class_decorator_spans(class, &mut decorator_spans_to_remove);
1038+
// Track decorator span for removal
1039+
if let Some(span) = find_pipe_decorator_span(class) {
1040+
decorator_spans_to_remove.push(span);
1041+
}
10401042
// Collect constructor parameter decorators (@Optional, @Inject, etc.)
10411043
collect_constructor_decorator_spans(class, &mut decorator_spans_to_remove);
10421044

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

1084-
// Track ALL class decorator spans for removal (Angular + non-Angular)
1085-
collect_all_class_decorator_spans(class, &mut decorator_spans_to_remove);
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+
}
10861090
// Collect constructor parameter decorators (@Optional, @Inject, etc.)
10871091
collect_constructor_decorator_spans(class, &mut decorator_spans_to_remove);
10881092

@@ -1165,22 +1169,25 @@ pub fn transform_angular_file(
11651169
_ => None,
11661170
};
11671171
if let Some(class) = class {
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);
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);
11801190
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-
}
11841191
}
11851192
}
11861193
}

crates/oxc_angular_compiler/tests/integration_test.rs

Lines changed: 0 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -5376,122 +5376,3 @@ 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)