From d00873ca6289ddd2f14cc91cedf24226caffbb61 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 4 Mar 2026 16:04:49 +0800 Subject: [PATCH] fix(angular): resolve e2e comparison mismatches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix multiple compiler output divergences from Angular TS: - fix(variable_optimization): handle Oxc's split handler_expression architecture in optimizeSaveRestoreView, removing unnecessary restoreView/resetView wrapping - fix(variable_optimization): reorder optimization steps to match Angular TS (arrow functions and listener handlers before create/update ops) - fix(variable_optimization): include Animation handlers in save/restore optimization - fix(emitter): emit non-ASCII characters as raw UTF-8 instead of \uNNNN escapes - fix(ordering): add OpKind::Control to update op ordering phase (priority 8, last) - fix(reify): add missing name string literal as second argument to ɵɵcontrol() - fix(emit): emit host binding pool constants (pure functions) alongside template declarations, matching Angular TS's shared ConstantPool behavior - fix(entities): use greedy &-to-; matching in decode_entities_in_string to replicate Angular TS's /&([^;]+);/g regex behavior Co-Authored-By: Claude Opus 4.6 --- .../oxc_angular_compiler/src/component/mod.rs | 9 +- .../src/component/transform.rs | 49 +- .../src/directive/compiler.rs | 23 +- crates/oxc_angular_compiler/src/linker/mod.rs | 19 +- .../src/output/emitter.rs | 88 ++-- .../src/parser/html/entities.rs | 107 +++- .../oxc_angular_compiler/src/pipeline/emit.rs | 31 +- .../src/pipeline/phases/ordering.rs | 6 +- .../src/pipeline/phases/reify/mod.rs | 2 +- .../pipeline/phases/reify/statements/misc.rs | 10 +- .../pipeline/phases/variable_optimization.rs | 149 ++++-- .../tests/integration_test.rs | 476 ++++++++++++++++-- .../tests/playground_repro.rs | 13 +- ...on_enter_string_literal_embedded_view.snap | 3 +- ...ter_string_literal_only_embedded_view.snap | 8 +- ...ation_test__unused_template_reference.snap | 4 +- 16 files changed, 809 insertions(+), 188 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/mod.rs b/crates/oxc_angular_compiler/src/component/mod.rs index 3a8f6f81b..37b5d2816 100644 --- a/crates/oxc_angular_compiler/src/component/mod.rs +++ b/crates/oxc_angular_compiler/src/component/mod.rs @@ -35,8 +35,9 @@ pub use metadata::{ pub use namespace_registry::NamespaceRegistry; pub use transform::{ CompiledComponent, HmrTemplateCompileOutput, HostMetadataInput, ImportInfo, ImportMap, - LinkerTemplateOutput, ResolvedResources, TemplateCompileOutput, TransformOptions, - TransformResult, build_import_map, compile_component_template, compile_for_hmr, - compile_host_bindings_for_linker, compile_template_for_hmr, compile_template_for_linker, - compile_template_to_js, compile_template_to_js_with_options, transform_angular_file, + LinkerHostBindingOutput, LinkerTemplateOutput, ResolvedResources, TemplateCompileOutput, + TransformOptions, TransformResult, build_import_map, compile_component_template, + compile_for_hmr, compile_host_bindings_for_linker, compile_template_for_hmr, + compile_template_for_linker, compile_template_to_js, compile_template_to_js_with_options, + transform_angular_file, }; diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 2df3f3e9d..0facc40b0 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -1569,10 +1569,20 @@ fn compile_component_full<'a>( compile_component_host_bindings(allocator, metadata, template_pool_index); // Extract the result and update pool index if host bindings were compiled - let (host_binding_result, host_binding_next_pool_index) = match host_binding_output { - Some(output) => (Some(output.result), Some(output.next_pool_index)), - None => (None, None), - }; + let (host_binding_result, host_binding_next_pool_index, host_binding_declarations) = + match host_binding_output { + Some(output) => { + let declarations = output.result.declarations; + let result = HostBindingCompilationResult { + host_binding_fn: output.result.host_binding_fn, + host_attrs: output.result.host_attrs, + host_vars: output.result.host_vars, + declarations: OxcVec::new_in(allocator), + }; + (Some(result), Some(output.next_pool_index), declarations) + } + None => (None, None, OxcVec::new_in(allocator)), + }; // Stage 7: Generate ɵcmp/ɵfac definitions // The namespace registry is shared across all components in the file to ensure @@ -1599,6 +1609,11 @@ fn compile_component_full<'a>( declarations_js.push_str(&emitter.emit_statement(decl)); declarations_js.push('\n'); } + // Emit host binding declarations (pooled constants like pure functions) + for decl in host_binding_declarations.iter() { + declarations_js.push_str(&emitter.emit_statement(decl)); + declarations_js.push('\n'); + } // For HMR, we emit the template separately using compile_template_to_js // The ɵcmp already contains the template function inline @@ -1972,6 +1987,11 @@ pub fn compile_template_to_js_with_options<'a>( component_name, options.selector.as_deref(), ) { + // Add host binding pool declarations (pure functions, etc.) + for decl in host_result.declarations { + all_statements.push(decl); + } + // Add the host bindings function as a declaration if present if let Some(host_fn) = host_result.host_binding_fn { if let Some(fn_name) = host_fn.name.clone() { @@ -2556,6 +2576,16 @@ fn compile_host_bindings_from_input<'a>( Some(result) } +/// Result of compiling host bindings for the linker. +pub struct LinkerHostBindingOutput { + /// The host binding function as JS. + pub fn_js: String, + /// Number of host variables. + pub host_vars: u32, + /// Pool constant declarations (pure functions, etc.) as JS. + pub declarations_js: String, +} + /// Compile host bindings for the linker, returning the emitted JS function + hostVars count. /// /// This takes host property/listener data extracted from a partial declaration and compiles @@ -2566,7 +2596,7 @@ pub fn compile_host_bindings_for_linker( host_input: &HostMetadataInput, component_name: &str, selector: Option<&str>, -) -> Option<(String, u32)> { +) -> Option { let allocator = Allocator::default(); let result = compile_host_bindings_from_input(&allocator, host_input, component_name, selector)?; @@ -2575,12 +2605,19 @@ pub fn compile_host_bindings_for_linker( let host_vars = result.host_vars.unwrap_or(0); + // Emit host binding pool declarations (pure functions, etc.) + let mut declarations_js = String::new(); + for decl in result.declarations.iter() { + declarations_js.push_str(&emitter.emit_statement(decl)); + declarations_js.push('\n'); + } + let fn_js = result.host_binding_fn.map(|f| { let expr = OutputExpression::Function(oxc_allocator::Box::new_in(f, &allocator)); emitter.emit_expression(&expr) })?; - Some((fn_js, host_vars)) + Some(LinkerHostBindingOutput { fn_js, host_vars, declarations_js }) } /// Output from compiling a template for the linker. diff --git a/crates/oxc_angular_compiler/src/directive/compiler.rs b/crates/oxc_angular_compiler/src/directive/compiler.rs index 3a0c04a9d..4fc032c1f 100644 --- a/crates/oxc_angular_compiler/src/directive/compiler.rs +++ b/crates/oxc_angular_compiler/src/directive/compiler.rs @@ -71,7 +71,7 @@ pub fn compile_directive_from_metadata<'a>( pool_starting_index: u32, ) -> DirectiveCompileResult<'a> { // Build the base directive fields, passing pool_starting_index for host bindings - let (definition_map, next_pool_index) = + let (definition_map, next_pool_index, host_declarations) = build_base_directive_fields(allocator, metadata, pool_starting_index); // Add features @@ -81,22 +81,30 @@ pub fn compile_directive_from_metadata<'a>( // Create the expression: ɵɵdefineDirective(definitionMap) let expression = create_define_directive_call(allocator, definition_map); - DirectiveCompileResult { expression, statements: Vec::new_in(allocator), next_pool_index } + // Convert host binding declarations to statements + let mut statements = Vec::new_in(allocator); + for decl in host_declarations { + statements.push(decl); + } + + DirectiveCompileResult { expression, statements, next_pool_index } } /// Builds the base directive definition map. /// /// Corresponds to `baseDirectiveFields()` in Angular's compiler. /// -/// Returns a tuple of (entries, next_pool_index) where next_pool_index is the -/// next available constant pool index after host binding compilation. +/// Returns a tuple of (entries, next_pool_index, host_declarations) where next_pool_index is the +/// next available constant pool index after host binding compilation, and host_declarations +/// contains any pooled constants (pure functions) from host binding compilation. fn build_base_directive_fields<'a>( allocator: &'a Allocator, metadata: &R3DirectiveMetadata<'a>, pool_starting_index: u32, -) -> (Vec<'a, LiteralMapEntry<'a>>, u32) { +) -> (Vec<'a, LiteralMapEntry<'a>>, u32, oxc_allocator::Vec<'a, OutputStatement<'a>>) { let mut entries = Vec::new_in(allocator); let mut next_pool_index = pool_starting_index; + let mut host_declarations = oxc_allocator::Vec::new_in(allocator); // type: MyDirective entries.push(LiteralMapEntry { @@ -196,6 +204,9 @@ fn build_base_directive_fields<'a>( quoted: false, }); } + + // Collect host binding pool declarations (pure functions, etc.) + host_declarations = result.declarations; } } @@ -264,7 +275,7 @@ fn build_base_directive_fields<'a>( }); } - (entries, next_pool_index) + (entries, next_pool_index, host_declarations) } /// Adds features to the definition map. diff --git a/crates/oxc_angular_compiler/src/linker/mod.rs b/crates/oxc_angular_compiler/src/linker/mod.rs index f8e0baee9..7b03dab0e 100644 --- a/crates/oxc_angular_compiler/src/linker/mod.rs +++ b/crates/oxc_angular_compiler/src/linker/mod.rs @@ -1362,6 +1362,7 @@ fn link_component( // Build the defineComponent properties let mut parts: Vec = Vec::new(); + let mut host_binding_declarations_js = String::new(); // 1. type parts.push(format!("type: {type_name}")); @@ -1398,13 +1399,16 @@ fn link_component( // through the full Angular expression parser for correct output. let host_input = extract_host_metadata_input(host_obj); let selector = get_string_property(meta, "selector"); - if let Some((host_fn, host_vars)) = + if let Some(host_output) = crate::component::compile_host_bindings_for_linker(&host_input, type_name, selector) { - if host_vars > 0 { - parts.push(format!("hostVars: {host_vars}")); + if host_output.host_vars > 0 { + parts.push(format!("hostVars: {}", host_output.host_vars)); + } + parts.push(format!("hostBindings: {}", host_output.fn_js)); + if !host_output.declarations_js.is_empty() { + host_binding_declarations_js = host_output.declarations_js; } - parts.push(format!("hostBindings: {host_fn}")); } } @@ -1533,8 +1537,11 @@ fn link_component( let define_component = format!("{ns}.\u{0275}\u{0275}defineComponent({{ {} }})", parts.join(", ")); - // Wrap in IIFE with template declarations - let declarations = &template_output.declarations_js; + // Wrap in IIFE with template and host binding declarations + let mut declarations = template_output.declarations_js; + if !host_binding_declarations_js.is_empty() { + declarations.push_str(&host_binding_declarations_js); + } if declarations.trim().is_empty() { Some(define_component) } else { diff --git a/crates/oxc_angular_compiler/src/output/emitter.rs b/crates/oxc_angular_compiler/src/output/emitter.rs index c3d3ee0be..9eb735d4e 100644 --- a/crates/oxc_angular_compiler/src/output/emitter.rs +++ b/crates/oxc_angular_compiler/src/output/emitter.rs @@ -1234,11 +1234,10 @@ fn is_nullish_coalesce(expr: &OutputExpression<'_>) -> bool { /// Escape a string for JavaScript output. /// /// Uses double quotes to match Angular's output style. -/// Escapes `"`, `\`, `\n`, `\r`, `$` (when requested), ASCII control characters, -/// and all non-ASCII characters (code point > 0x7E) as `\uNNNN` sequences. -/// Characters above the BMP (U+10000+) are encoded as UTF-16 surrogate pairs -/// (`\uXXXX\uXXXX`). This matches TypeScript's emitter behavior, which escapes -/// non-ASCII characters in string literals. +/// Escapes `"`, `\`, `\n`, `\r`, `$` (when requested), and ASCII control characters +/// as `\uNNNN` sequences. Non-ASCII characters (code point > 0x7E) are emitted as +/// raw UTF-8 to match Angular's TypeScript emitter behavior (see `escapeIdentifier` +/// in `abstract_emitter.ts`), which only escapes `'`, `\`, `\n`, `\r`, and `$`. pub(crate) fn escape_string(input: &str, escape_dollar: bool) -> String { let mut result = String::with_capacity(input.len() + 2); result.push('"'); @@ -1251,18 +1250,14 @@ pub(crate) fn escape_string(input: &str, escape_dollar: bool) -> String { '$' if escape_dollar => result.push_str("\\$"), // ASCII printable characters (0x20-0x7E) are emitted literally c if (' '..='\x7E').contains(&c) => result.push(c), - // Everything else (ASCII control chars, non-ASCII) is escaped as \uNNNN. - // Characters above the BMP are encoded as UTF-16 surrogate pairs. + // DEL (0x7F) is an ASCII control character and must be escaped + '\x7F' => push_unicode_escape(&mut result, 0x7F), + // Non-ASCII characters (> 0x7F) are emitted as raw UTF-8 to match + // Angular's TypeScript emitter, which does not escape them. + c if (c as u32) > 0x7F => result.push(c), + // ASCII control characters (0x00-0x1F) are escaped as \uNNNN. c => { - let code = c as u32; - if code <= 0xFFFF { - push_unicode_escape(&mut result, code); - } else { - let hi = 0xD800 + ((code - 0x10000) >> 10); - let lo = 0xDC00 + ((code - 0x10000) & 0x3FF); - push_unicode_escape(&mut result, hi); - push_unicode_escape(&mut result, lo); - } + push_unicode_escape(&mut result, c as u32); } } } @@ -1514,35 +1509,35 @@ mod tests { #[test] fn test_escape_string_unicode_literals() { - // Non-ASCII characters should be escaped as \uNNNN to match - // TypeScript's emitter behavior. + // Non-ASCII characters should be emitted as raw UTF-8 to match + // Angular's TypeScript emitter behavior (escapeIdentifier in abstract_emitter.ts). - // × (multiplication sign U+00D7) -> \u00D7 - assert_eq!(escape_string("\u{00D7}", false), "\"\\u00D7\""); + // × (multiplication sign U+00D7) -> raw UTF-8 + assert_eq!(escape_string("\u{00D7}", false), "\"\u{00D7}\""); - //   (non-breaking space U+00A0) -> \u00A0 - assert_eq!(escape_string("\u{00A0}", false), "\"\\u00A0\""); + //   (non-breaking space U+00A0) -> raw UTF-8 + assert_eq!(escape_string("\u{00A0}", false), "\"\u{00A0}\""); // Mixed ASCII and non-ASCII - assert_eq!(escape_string("a\u{00D7}b", false), "\"a\\u00D7b\""); + assert_eq!(escape_string("a\u{00D7}b", false), "\"a\u{00D7}b\""); // Multiple non-ASCII characters - assert_eq!(escape_string("\u{00D7}\u{00A0}", false), "\"\\u00D7\\u00A0\""); + assert_eq!(escape_string("\u{00D7}\u{00A0}", false), "\"\u{00D7}\u{00A0}\""); - // Characters outside BMP (emoji) -> surrogate pair - assert_eq!(escape_string("\u{1F600}", false), "\"\\uD83D\\uDE00\""); + // Characters outside BMP (emoji) -> raw UTF-8 + assert_eq!(escape_string("\u{1F600}", false), "\"\u{1F600}\""); - // Common HTML entities -> all escaped as \uNNNN - assert_eq!(escape_string("\u{00A9}", false), "\"\\u00A9\""); // © © - assert_eq!(escape_string("\u{00AE}", false), "\"\\u00AE\""); // ® ® - assert_eq!(escape_string("\u{2014}", false), "\"\\u2014\""); // — — - assert_eq!(escape_string("\u{2013}", false), "\"\\u2013\""); // – – + // Common HTML entities -> all emitted as raw UTF-8 + assert_eq!(escape_string("\u{00A9}", false), "\"\u{00A9}\""); // © © + assert_eq!(escape_string("\u{00AE}", false), "\"\u{00AE}\""); // ® ® + assert_eq!(escape_string("\u{2014}", false), "\"\u{2014}\""); // — — + assert_eq!(escape_string("\u{2013}", false), "\"\u{2013}\""); // – – // Greek letter alpha - assert_eq!(escape_string("\u{03B1}", false), "\"\\u03B1\""); // α + assert_eq!(escape_string("\u{03B1}", false), "\"\u{03B1}\""); // α // Accented Latin letter - assert_eq!(escape_string("\u{00E9}", false), "\"\\u00E9\""); // é + assert_eq!(escape_string("\u{00E9}", false), "\"\u{00E9}\""); // é } #[test] @@ -1561,34 +1556,33 @@ mod tests { } #[test] - fn test_escape_string_non_ascii_as_unicode_escapes() { - // Non-ASCII characters should be escaped as \uNNNN to match - // TypeScript's emitter behavior (which escapes non-ASCII in string literals). + fn test_escape_string_non_ascii_as_raw_utf8() { + // Non-ASCII characters should be emitted as raw UTF-8 to match + // Angular's TypeScript emitter behavior (escapeIdentifier in abstract_emitter.ts). // Non-breaking space U+00A0 - assert_eq!(escape_string("\u{00A0}", false), "\"\\u00A0\""); + assert_eq!(escape_string("\u{00A0}", false), "\"\u{00A0}\""); // En dash U+2013 - assert_eq!(escape_string("\u{2013}", false), "\"\\u2013\""); + assert_eq!(escape_string("\u{2013}", false), "\"\u{2013}\""); // Trademark U+2122 - assert_eq!(escape_string("\u{2122}", false), "\"\\u2122\""); + assert_eq!(escape_string("\u{2122}", false), "\"\u{2122}\""); // Infinity U+221E - assert_eq!(escape_string("\u{221E}", false), "\"\\u221E\""); + assert_eq!(escape_string("\u{221E}", false), "\"\u{221E}\""); // Mixed ASCII and non-ASCII - assert_eq!(escape_string("a\u{00D7}b", false), "\"a\\u00D7b\""); + assert_eq!(escape_string("a\u{00D7}b", false), "\"a\u{00D7}b\""); // Multiple non-ASCII characters - assert_eq!(escape_string("\u{00D7}\u{00A0}", false), "\"\\u00D7\\u00A0\""); + assert_eq!(escape_string("\u{00D7}\u{00A0}", false), "\"\u{00D7}\u{00A0}\""); - // Characters above BMP should use surrogate pairs - // U+1F600 (grinning face) = surrogate pair D83D DE00 - assert_eq!(escape_string("\u{1F600}", false), "\"\\uD83D\\uDE00\""); + // Characters above BMP (emoji) -> raw UTF-8 + assert_eq!(escape_string("\u{1F600}", false), "\"\u{1F600}\""); - // U+10000 (first supplementary char) = surrogate pair D800 DC00 - assert_eq!(escape_string("\u{10000}", false), "\"\\uD800\\uDC00\""); + // U+10000 (first supplementary char) -> raw UTF-8 + assert_eq!(escape_string("\u{10000}", false), "\"\u{10000}\""); // ASCII printable chars (0x20-0x7E) should remain literal assert_eq!(escape_string(" ~", false), "\" ~\""); diff --git a/crates/oxc_angular_compiler/src/parser/html/entities.rs b/crates/oxc_angular_compiler/src/parser/html/entities.rs index bde1923f4..032c68baa 100644 --- a/crates/oxc_angular_compiler/src/parser/html/entities.rs +++ b/crates/oxc_angular_compiler/src/parser/html/entities.rs @@ -50,42 +50,55 @@ pub fn decode_entity(entity: &str) -> Option { /// For backward compatibility (matching Angular's behavior), this decodes /// HTML entities that appear in interpolation expressions. /// -/// Pattern: `&entity;` where entity can be: -/// - Named: `amp`, `lt`, `gt`, `quot`, etc. -/// - Decimal: `#123` -/// - Hex: `#x7B` or `#X7B` +/// Uses greedy matching to replicate Angular TS's `/&([^;]+);/g` regex behavior +/// in `parser.ts` line 365. The regex matches from `&` to the first `;` with +/// ANY characters in between. When decoding fails, the regex cursor advances +/// past the `;`, which means earlier `&` characters can "consume" later `;` +/// characters and prevent entity decoding. +/// +/// Example: `foo && bar ? '∞' : baz` — the regex matches from the first +/// `&` of `&&` all the way to the `;` in `∞`, creating one invalid match. +/// The entity is not decoded because the entire span is consumed. pub fn decode_entities_in_string(s: &str) -> String { let mut result = String::with_capacity(s.len()); let mut chars = s.char_indices().peekable(); while let Some((i, ch)) = chars.next() { if ch == '&' { - // Look for entity pattern: &...; - let mut entity_end = None; + // Greedy match: find the first ';' (mimics /&([^;]+);/g) let remaining = &s[i..]; + let mut entity_end_byte = None; + let mut chars_after_ampersand = 0u32; - // Find the semicolon + // Skip the '&' at j=0, find the first ';' for (j, c) in remaining.char_indices() { - if c == ';' { - entity_end = Some(j); - break; - } - // Entities shouldn't be too long, and should contain valid chars - if j > 32 || (!c.is_ascii_alphanumeric() && c != '#' && c != '&') { - break; + if j > 0 { + chars_after_ampersand += 1; + if c == ';' { + entity_end_byte = Some(j); + break; + } } } - if let Some(end) = entity_end { + if let Some(end) = entity_end_byte { let entity = &remaining[..=end]; if let Some(decoded) = decode_entity(entity) { result.push_str(&decoded); - // Skip past the entity in the input - for _ in 0..end { + // Skip past the entity in the outer iterator. + // Use char count (not byte offset) since .next() advances by character. + for _ in 0..chars_after_ampersand { chars.next(); } continue; } + // Failed to decode: push the full matched text and advance past ';' + // This matches regex behavior where the cursor advances past the match + result.push_str(entity); + for _ in 0..chars_after_ampersand { + chars.next(); + } + continue; } } result.push(ch); @@ -2277,4 +2290,64 @@ mod tests { // "Afr" maps to Mathematical Fraktur A (U+1D504) assert!(entities.contains_key("Afr")); } + + #[test] + fn test_decode_entities_in_string_basic() { + assert_eq!(decode_entities_in_string("hello & world"), "hello & world"); + assert_eq!(decode_entities_in_string("<div>"), "
"); + assert_eq!(decode_entities_in_string("no entities here"), "no entities here"); + } + + #[test] + fn test_decode_entities_greedy_matching() { + // Angular TS regex /&([^;]+);/g greedily matches from && to ∞'s semicolon, + // consuming the entire span as one failed match — so ∞ is NOT decoded. + let input = "foo && bar ? '∞' : baz"; + assert_eq!( + decode_entities_in_string(input), + "foo && bar ? '∞' : baz", + "greedy match from && should prevent ∞ decoding" + ); + } + + #[test] + fn test_decode_entities_multibyte_between_ampersand_and_semicolon() { + // When multi-byte characters appear between & and ;, the skip count must use + // character count (not byte offset) to avoid over-consuming from the iterator. + // 'café' has 4 chars but 5 bytes (é is 2 bytes in UTF-8). + // &café; is not a valid entity, so the full span is pushed as-is. + // The text after the semicolon ("rest") must be preserved. + let input = "&café;rest"; + assert_eq!( + decode_entities_in_string(input), + "&café;rest", + "multi-byte chars between & and ; must not cause data loss after ;" + ); + } + + #[test] + fn test_decode_entities_multibyte_cjk_between_ampersand_and_semicolon() { + // CJK characters are 3 bytes each in UTF-8. + // &日本; has 3 chars between & and ; but 7 bytes. + // Using byte offset as skip count would consume 7 chars instead of 3, + // eating into " after" and silently dropping characters. + let input = "&日本;after"; + assert_eq!( + decode_entities_in_string(input), + "&日本;after", + "CJK chars between & and ; must not cause data loss" + ); + } + + #[test] + fn test_decode_entities_emoji_between_ampersand_and_semicolon() { + // Emoji are 4 bytes each in UTF-8. + // &😀; byte offset of ; is 5, but only 1 char to skip after &. + let input = "&😀;tail"; + assert_eq!( + decode_entities_in_string(input), + "&😀;tail", + "4-byte emoji between & and ; must not cause data loss" + ); + } } diff --git a/crates/oxc_angular_compiler/src/pipeline/emit.rs b/crates/oxc_angular_compiler/src/pipeline/emit.rs index d95cdc933..0a15550be 100644 --- a/crates/oxc_angular_compiler/src/pipeline/emit.rs +++ b/crates/oxc_angular_compiler/src/pipeline/emit.rs @@ -1525,6 +1525,10 @@ pub struct HostBindingCompilationResult<'a> { /// Number of host variables for change detection. /// Only set if > 0. pub host_vars: Option, + /// Additional declarations (pooled constants like pure functions). + /// In Angular TS, template and host binding share the same ConstantPool, + /// so host binding constants get emitted alongside template constants. + pub declarations: OxcVec<'a, OutputStatement<'a>>, } /// Compile host bindings from start to finish. @@ -1538,6 +1542,10 @@ pub struct HostBindingCompilationResult<'a> { pub fn compile_host_bindings<'a>( job: &mut HostBindingCompilationJob<'a>, ) -> HostBindingCompilationResult<'a> { + use crate::output::ast::DeclareVarStmt; + + let allocator = job.allocator; + // Run all transformation phases for host bindings transform_host(job); @@ -1553,7 +1561,28 @@ pub fn compile_host_bindings<'a>( let host_attrs = job.root.attributes.take(); let host_vars = job.root.vars.filter(|&v| v > 0); - HostBindingCompilationResult { host_binding_fn, host_attrs, host_vars } + // Collect declarations from host binding pool constants. + // In Angular TS, template and host binding share the same ConstantPool, + // so host binding pure functions get emitted alongside template constants. + let mut declarations = OxcVec::new_in(allocator); + for constant in job.pool.constants_mut() { + let value = emit_pooled_constant_value(allocator, &mut constant.kind); + declarations.push(OutputStatement::DeclareVar(Box::new_in( + DeclareVarStmt { + name: constant.name.clone(), + value: Some(value), + modifiers: StmtModifier::FINAL, + leading_comment: None, + source_span: None, + }, + allocator, + ))); + } + for stmt in job.pool.statements.iter() { + declarations.push(clone_output_statement(stmt, allocator)); + } + + HostBindingCompilationResult { host_binding_fn, host_attrs, host_vars, declarations } } /// Converts an IR binary operator to an output binary operator. diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/ordering.rs b/crates/oxc_angular_compiler/src/pipeline/phases/ordering.rs index 95f78bf4b..423388547 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/ordering.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/ordering.rs @@ -27,6 +27,7 @@ fn is_handled_update_op_kind(kind: OpKind) -> bool { | OpKind::TwoWayProperty | OpKind::DomProperty | OpKind::Attribute + | OpKind::Control ) } @@ -64,8 +65,10 @@ fn update_op_priority(op: &UpdateOp<'_>) -> u32 { (OpKind::TwoWayProperty, _) => 6, // Non-interpolation TwoWayProperty (OpKind::Property, false) => 6, // Non-interpolation Property (OpKind::Attribute, false) => 7, // Attribute without interpolation + // Control comes last per Angular's UPDATE_ORDERING (line 69 of ordering.ts) + (OpKind::Control, _) => 8, // DomProperty is not used in template bindings, but include for completeness - (OpKind::DomProperty, _) => 8, + (OpKind::DomProperty, _) => 9, _ => 100, // Other ops go last } } @@ -124,6 +127,7 @@ fn get_update_op_target(op: &UpdateOp<'_>) -> Option { UpdateOp::ClassMap(p) => Some(p.target), UpdateOp::Attribute(p) => Some(p.target), UpdateOp::DomProperty(p) => Some(p.target), + UpdateOp::Control(c) => Some(c.target), _ => None, } } diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs b/crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs index 55cda4ea2..6373d3609 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs @@ -886,7 +886,7 @@ fn reify_update_op<'a>( } UpdateOp::Control(ctrl) => { let expr = convert_ir_expression(allocator, &ctrl.expression, expressions, root_xref); - Some(create_control_stmt(allocator, expr)) + Some(create_control_stmt(allocator, expr, &ctrl.name)) } UpdateOp::Variable(var) => { // Emit variable declaration with initializer for update phase diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/reify/statements/misc.rs b/crates/oxc_angular_compiler/src/pipeline/phases/reify/statements/misc.rs index 57b1223f3..97a665de8 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/reify/statements/misc.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/reify/statements/misc.rs @@ -379,16 +379,22 @@ pub fn create_animation_binding_stmt<'a>( /// /// The control instruction takes: /// - expression: The expression to evaluate for the control value +/// - name: The property name as a string literal /// - sanitizer: Optional sanitizer (only if not null) /// -/// Note: Unlike property(), control() does NOT take a name as the first argument. -/// Ported from Angular's `control()` in `instruction.ts`. +/// Note: Unlike property() which takes (name, expression), control() takes (expression, name). +/// Ported from Angular's `control()` in `instruction.ts` lines 598-614. pub fn create_control_stmt<'a>( allocator: &'a oxc_allocator::Allocator, value: OutputExpression<'a>, + name: &Atom<'a>, ) -> OutputStatement<'a> { let mut args = OxcVec::new_in(allocator); args.push(value); + args.push(OutputExpression::Literal(Box::new_in( + LiteralExpr { value: LiteralValue::String(name.clone()), source_span: None }, + allocator, + ))); // Note: sanitizer would be pushed here if not null, but it's always null for ControlOp create_instruction_call_stmt(allocator, Identifiers::CONTROL, args) } diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/variable_optimization.rs b/crates/oxc_angular_compiler/src/pipeline/phases/variable_optimization.rs index ce409397e..50848226b 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/variable_optimization.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/variable_optimization.rs @@ -389,32 +389,48 @@ fn collect_create_op_fences(op: &CreateOp<'_>) -> Fence { /// /// Note: SavedView variables (from `getCurrentView()`) can be removed if unused /// because `GetCurrentViewExpr` has no side effects. +/// +/// CRITICAL: The order of optimization must match Angular's TypeScript implementation +/// (variable_optimization.ts lines 32-80). Specifically: +/// 1. inlineAlwaysInlineVariables on all op lists (arrow funcs, create, update, handlers) +/// 2. optimizeVariablesInOpList on arrow function ops +/// 3. optimizeVariablesInOpList on listener handler_ops (+ optimizeSaveRestoreView) +/// 4. optimizeVariablesInOpList on create + update ops +/// +/// Steps 2-3 MUST run BEFORE step 4. This is because `count_variable_usages` for +/// create ops visits into listener handler_ops. If handler_ops haven't been optimized +/// yet, unused ReadVariable references in handler_ops inflate the usage counts of +/// variables declared in create ops (e.g., SavedView), preventing their removal. pub fn optimize_variables(job: &mut ComponentCompilationJob<'_>) { // Step 1: Inline AlwaysInline variables unconditionally - // Per TypeScript's variable_optimization.ts lines 33-46 + // Per TypeScript's variable_optimization.ts lines 33-51 inline_always_inline_variables(job); // Step 2: Inline Identifier variables whose initializer is just `ctx`. // Per TypeScript's allowConservativeInlining (lines 527-535): - // ```typescript - // case ir.SemanticVariableKind.Identifier: - // if (decl.initializer instanceof o.ReadVarExpr && decl.initializer.name === 'ctx') { - // return true; // Allow inlining for conditional aliases - // } - // return false; - // ``` // This handles conditional aliases like `@if (icon(); as icon)` where the child view // creates an Identifier variable `icon` with initializer `ctx`. TypeScript inlines // these to use `ctx` directly: `ɵɵclassMap(ctx)` instead of `const icon = ctx; ɵɵclassMap(icon)`. inline_identifier_ctx_variables(job); - // Step 3: Remove unused variables and inline single-use variables + // Step 3: Optimize arrow function ops (per Angular's variable_optimization.ts lines 53-56) + // These must be optimized BEFORE create/update ops so that usage counts from + // arrow function expressions don't interfere with create/update variable counting. + optimize_arrow_function_ops(job); + + // Step 4: Optimize listener handler_ops separately (per Angular's variable_optimization.ts lines 58-70) + // This MUST run BEFORE create/update ops optimization. When handler_ops contain + // unused Reference variables, save_and_restore_view adds RestoreView + SavedView. + // Optimizing handler_ops first removes these unused references and the associated + // RestoreView/ResetView via optimizeSaveRestoreView. This reduces the usage count + // of SavedView variables in create ops, allowing them to be properly removed. + optimize_listener_handler_ops(job); + + // Step 5: Remove unused variables and inline single-use variables in create/update ops. + // Per Angular's variable_optimization.ts lines 77-78: + // optimizeVariablesInOpList(unit.create, skipArrowFunctionOps); + // optimizeVariablesInOpList(unit.update, skipArrowFunctionOps); // Iterate until no more variables can be removed. - // This handles cases where variable A is only used by variable B, - // and B is unused - we need to remove B first, then A becomes unused. - // - // IMPORTANT: This must run BEFORE Context inlining so that unused Identifier - // variables are removed first, reducing the usage count of Context variables. loop { let removed = optimize_variables_once(job); if !removed { @@ -422,39 +438,20 @@ pub fn optimize_variables(job: &mut ComponentCompilationJob<'_>) { } } - // Step 4: Inline Context variables (nextContext()) into other Variable ops. + // Step 6: Inline Context variables (nextContext()) into other Variable ops. // Per TypeScript's allowConservativeInlining (lines 536-538): // "Context can only be inlined into other variables." - // This is safe because we only inline into Variable ops, not into arbitrary ops. - // - // This runs AFTER unused variable removal so that Context variables have - // minimal usage counts (ideally 1 for single-use inlining). inline_context_variables_into_variable_ops(job); - // Step 5: Run unused variable removal again after context inlining. + // Step 7: Run unused variable removal again after context inlining. // Context inlining may reduce usage counts of Context variables to 0, making them - // eligible for removal. Without this, unused Context variables would be converted - // to statements (standalone nextContext() calls) instead of being removed entirely. - // - // Example: If we have ctx_a = nextContext() followed by ctx_b = nextContext(), - // and ctx_b is inlined into another variable, ctx_b's usage drops to 0. At this point, - // ctx_a may also become removable if nothing depends on its context write. + // eligible for removal. loop { let removed = optimize_variables_once(job); if !removed { break; } } - - // Step 6: Optimize arrow function ops (per Angular's variable_optimization.ts lines 53-56) - // for (const expr of unit.functions) { - // optimizeVariablesInOpList(expr.ops, job.compatibility, null); - // optimizeSaveRestoreView(expr.ops); - // } - optimize_arrow_function_ops(job); - - // Step 7: Optimize listener handler_ops separately (per Angular's variable_optimization.ts lines 58-70) - optimize_listener_handler_ops(job); } /// Inlines variables that should always be inlined. @@ -1699,7 +1696,7 @@ fn optimize_arrow_function_ops<'a>(job: &mut ComponentCompilationJob<'a>) { optimize_handler_ops(&mut func.ops, None, allocator, &mut local_diagnostics); // Step 2: Apply save/restore view optimization - optimize_save_restore_view(&mut func.ops, allocator); + optimize_save_restore_view(&mut func.ops, &mut None, allocator); } } @@ -1743,7 +1740,11 @@ fn optimize_listener_handler_ops<'a>(job: &mut ComponentCompilationJob<'a>) { allocator, &mut local_diagnostics, ); - optimize_save_restore_view(&mut listener.handler_ops, allocator); + optimize_save_restore_view( + &mut listener.handler_ops, + &mut listener.handler_expression, + allocator, + ); } CreateOp::TwoWayListener(listener) => { optimize_handler_ops( @@ -1752,7 +1753,7 @@ fn optimize_listener_handler_ops<'a>(job: &mut ComponentCompilationJob<'a>) { allocator, &mut local_diagnostics, ); - optimize_save_restore_view(&mut listener.handler_ops, allocator); + optimize_save_restore_view(&mut listener.handler_ops, &mut None, allocator); } CreateOp::AnimationListener(listener) => { optimize_handler_ops( @@ -1761,7 +1762,7 @@ fn optimize_listener_handler_ops<'a>(job: &mut ComponentCompilationJob<'a>) { allocator, &mut local_diagnostics, ); - optimize_save_restore_view(&mut listener.handler_ops, allocator); + optimize_save_restore_view(&mut listener.handler_ops, &mut None, allocator); } CreateOp::Animation(animation) => { optimize_handler_ops( @@ -1770,11 +1771,9 @@ fn optimize_listener_handler_ops<'a>(job: &mut ComponentCompilationJob<'a>) { allocator, &mut local_diagnostics, ); - // Note: We intentionally do NOT call optimize_save_restore_view on - // Animation handler_ops. Angular's ngtsc output keeps restoreView/resetView - // in animation callbacks even when the return value doesn't reference the - // view context (e.g., `return "animate-in"`). Skipping this optimization - // for Animation handlers matches the observed Angular output. + // Per Angular's variable_optimization.ts lines 61-66, + // Animation handlers also get optimizeSaveRestoreView. + optimize_save_restore_view(&mut animation.handler_ops, &mut None, allocator); } _ => {} } @@ -1823,18 +1822,61 @@ fn optimize_handler_ops<'a>( /// /// Ported from Angular's `optimizeSaveRestoreView` in `variable_optimization.ts` (lines 575-596). /// -/// We can only optimize if we have exactly two ops: -/// 1. A call to `restoreView` (ExpressionStatement with RestoreViewExpr). -/// 2. A return statement with a `resetView` in it. +/// In Angular's TypeScript, handler_ops is an OpList that contains both the restoreView expression +/// statement and the return statement. In Oxc, the return value for Listeners is stored separately +/// in `handler_expression`. This function handles both cases: +/// +/// **Case 1: handler_expression is Some (Listener)** +/// handler_ops has 1 op (restoreView expression statement) and handler_expression has ResetView(expr). +/// We remove the restoreView op and unwrap ResetView from handler_expression. /// -/// If these conditions are met: -/// - Remove the restoreView call (first op) -/// - Unwrap the resetView in the return statement (replace ResetView(expr) with just expr) +/// **Case 2: handler_expression is None (TwoWayListener, AnimationListener)** +/// handler_ops has 2 ops: restoreView expression statement + return ResetView(expr). +/// We remove the restoreView and unwrap the ResetView in the return statement. fn optimize_save_restore_view<'a>( handler_ops: &mut OxcVec<'a, UpdateOp<'a>>, + handler_expression: &mut Option>>, allocator: &'a oxc_allocator::Allocator, ) { - // We need exactly 2 ops to optimize + // Case 1: handler_expression contains the return value (Listener) + // handler_ops should have exactly 1 op (restoreView expression statement) + // and handler_expression should contain ResetView(expr) + if let Some(expr) = handler_expression.as_ref() { + if handler_ops.len() == 1 { + // Check the single op: must be a Statement with ExpressionStatement containing RestoreViewExpr + let first_is_restore_view = matches!( + &handler_ops[0], + UpdateOp::Statement(stmt) if matches!( + &stmt.statement, + OutputStatement::Expression(expr_stmt) if matches!( + &expr_stmt.expr, + OutputExpression::WrappedIrNode(wrapped) if matches!( + wrapped.node.as_ref(), + IrExpression::RestoreView(_) + ) + ) + ) + ); + + // Check handler_expression: must be ResetView(expr) + let expr_is_reset_view = matches!(expr.as_ref(), IrExpression::ResetView(_)); + + if first_is_restore_view && expr_is_reset_view { + // Extract the inner expression from ResetView + if let IrExpression::ResetView(reset_view) = expr.as_ref() { + let inner_expr = reset_view.expr.clone_in(allocator); + // Remove the restoreView op + handler_ops.clear(); + // Unwrap the ResetView from handler_expression + *handler_expression = Some(OxcBox::new_in(inner_expr, allocator)); + } + return; + } + } + } + + // Case 2: No handler_expression (TwoWayListener, AnimationListener) + // Both ops are in handler_ops if handler_ops.len() != 2 { return; } @@ -1878,9 +1920,6 @@ fn optimize_save_restore_view<'a>( } // Both conditions met - apply the optimization - // 1. Remove the first op (restoreView call) - // 2. Unwrap the ResetView in the return statement - // Extract the inner expression from the ResetView in the return statement let inner_expr = if let UpdateOp::Statement(stmt) = &handler_ops[1] { if let OutputStatement::Return(ret_stmt) = &stmt.statement { diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 3941eadb7..feab4078a 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -145,13 +145,13 @@ fn test_multiple_interpolations() { #[test] fn test_html_entity_between_interpolations() { - // HTML entity × between two interpolations should produce \u00D7 in the output + // HTML entity × between two interpolations should produce raw × in the output let js = compile_template_to_js("
{{ a }}×{{ b }}
", "TestComponent"); - // Should produce: textInterpolate2("", ctx.a, "\u00D7", ctx.b) - // Note: × (multiplication sign) = U+00D7, escaped as \u00D7 + // Should produce: textInterpolate2("", ctx.a, "×", ctx.b) + // Note: × (multiplication sign) = U+00D7, emitted as raw UTF-8 assert!( - js.contains(r#"textInterpolate2("",ctx.a,"\u00D7",ctx.b)"#), - "Expected textInterpolate2 with escaped times character. Got:\n{js}" + js.contains("textInterpolate2(\"\",ctx.a,\"\u{00D7}\",ctx.b)"), + "Expected textInterpolate2 with raw times character. Got:\n{js}" ); } @@ -159,12 +159,12 @@ fn test_html_entity_between_interpolations() { fn test_html_entity_at_start_of_interpolation() { // Entity at start: ×{{ a }} let js = compile_template_to_js("
×{{ a }}
", "TestComponent"); - // Should produce: textInterpolate1("\u00D7", ctx.a) - // Note: × (multiplication sign) = U+00D7, escaped as \u00D7 + // Should produce: textInterpolate1("×", ctx.a) + // Note: × (multiplication sign) = U+00D7, emitted as raw UTF-8 assert!( - js.contains(r#"textInterpolate1("\u00D7",ctx.a)"#) - || js.contains(r#"textInterpolate("\u00D7",ctx.a)"#), - "Expected textInterpolate with escaped times character at start. Got:\n{js}" + js.contains("textInterpolate1(\"\u{00D7}\",ctx.a)") + || js.contains("textInterpolate(\"\u{00D7}\",ctx.a)"), + "Expected textInterpolate with raw times character at start. Got:\n{js}" ); } @@ -173,11 +173,11 @@ fn test_multiple_html_entities_between_interpolations() { // Multiple entities: {{ a }} × {{ b }} let js = compile_template_to_js("
{{ a }} × {{ b }}
", "TestComponent"); - // Should produce: textInterpolate2("", ctx.a, "\u00A0\u00D7\u00A0", ctx.b) - // Note:   = U+00A0, × = U+00D7, both escaped as \uNNNN + // Should produce: textInterpolate2("", ctx.a, "\u{00A0}×\u{00A0}", ctx.b) + // Note:   = U+00A0, × = U+00D7, both emitted as raw UTF-8 assert!( - js.contains(r#"textInterpolate2("",ctx.a,"\u00A0\u00D7\u00A0",ctx.b)"#), - "Expected textInterpolate2 with escaped Unicode entities. Got:\n{js}" + js.contains("textInterpolate2(\"\",ctx.a,\"\u{00A0}\u{00D7}\u{00A0}\",ctx.b)"), + "Expected textInterpolate2 with raw Unicode entities. Got:\n{js}" ); } @@ -3803,14 +3803,14 @@ fn test_animation_enter_string_literal_in_embedded_view() { #[test] fn test_animation_enter_string_literal_only_in_embedded_view() { // Tests the case where an animation handler returning a string literal is the ONLY - // listener-like op in an embedded view. Angular's ngtsc always keeps restoreView/resetView - // in animation handler callbacks in embedded views, even when the return value is a simple - // string literal that doesn't reference the view context. + // listener-like op in an embedded view. Per Angular's variable_optimization.ts (lines 61-66), + // Animation handlers also get `optimizeSaveRestoreView`, so when the return value is a + // simple string literal that doesn't reference the view context, restoreView/resetView + // should be optimized away. // - // Expected NG output pattern: + // Expected output pattern: // i0.ɵɵanimateEnter(function ...() { - // i0.ɵɵrestoreView(_r1); - // return i0.ɵɵresetView("animate-in"); + // return "animate-in"; // }); let js = compile_template_to_js( r#"@if (show) { @@ -3824,14 +3824,9 @@ fn test_animation_enter_string_literal_only_in_embedded_view() { !js.contains("_unnamed_"), "Generated JS contains _unnamed_ references.\nGenerated JS:\n{js}" ); - assert!( - js.contains("restoreView"), - "Animation handler in embedded view should keep restoreView.\nGenerated JS:\n{js}" - ); - assert!( - js.contains("resetView"), - "Animation handler in embedded view should keep resetView.\nGenerated JS:\n{js}" - ); + // The animation handler only returns a string literal, so restoreView/resetView + // should be optimized away. But the view's listener (if any) may still need it, + // so we check that the animation callback itself doesn't have unnecessary wrapping. insta::assert_snapshot!("animation_enter_string_literal_only_embedded_view", js); } @@ -5437,3 +5432,428 @@ fn test_field_property_not_control_binding() { js ); } + +// ============================================================================ +// Regression: optimize_save_restore_view should remove restoreView/resetView +// when the listener body doesn't reference any parent context variables. +// ============================================================================ + +#[test] +fn test_optimize_save_restore_view_stop_propagation() { + // Listener in embedded view that returns $event.stopPropagation() + // should NOT have restoreView/resetView wrapping because the listener + // doesn't reference any parent context variables. + let allocator = Allocator::default(); + let source = r#" +import { Component } from '@angular/core'; + +@Component({ + selector: 'test-comp', + template: '
', + standalone: true, +}) +export class TestComponent { + show = true; +} +"#; + + let result = transform_angular_file( + &allocator, + "test.component.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let code = &result.code; + + // Angular optimizes away restoreView/resetView when the listener doesn't + // reference any parent context. The output should just be: + // return $event.stopPropagation(); + // NOT: + // i0.ɵɵrestoreView(_r1); + // return i0.ɵɵresetView($event.stopPropagation()); + assert!( + !code.contains("restoreView") || !code.contains("$event.stopPropagation"), + "Listener that only calls $event.stopPropagation() should not have restoreView/resetView. Got:\n{code}" + ); +} + +#[test] +fn test_optimize_save_restore_view_return_false() { + // Listener in embedded view that returns false + // should NOT have restoreView/resetView wrapping. + let allocator = Allocator::default(); + let source = r#" +import { Component } from '@angular/core'; + +@Component({ + selector: 'test-comp', + template: '', + standalone: true, +}) +export class TestComponent { + show = true; +} +"#; + + let result = transform_angular_file( + &allocator, + "test.component.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let code = &result.code; + + // The listener should just return false without restoreView/resetView + assert!( + !code.contains("restoreView"), + "Listener that returns false should not have restoreView. Got:\n{code}" + ); + assert!( + !code.contains("resetView"), + "Listener that returns false should not have resetView. Got:\n{code}" + ); +} + +#[test] +fn test_optimize_save_restore_view_prevent_default() { + // Listener in embedded view that returns $event.preventDefault() + // should NOT have restoreView/resetView wrapping. + let allocator = Allocator::default(); + let source = r#" +import { Component } from '@angular/core'; + +@Component({ + selector: 'test-comp', + template: '
Form
', + standalone: true, +}) +export class TestComponent { + show = true; +} +"#; + + let result = transform_angular_file( + &allocator, + "test.component.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let code = &result.code; + + // The listener should just return $event.preventDefault() without restoreView/resetView + assert!( + !code.contains("restoreView"), + "Listener that only calls $event.preventDefault() should not have restoreView. Got:\n{code}" + ); + assert!( + !code.contains("resetView"), + "Listener that only calls $event.preventDefault() should not have resetView. Got:\n{code}" + ); +} + +// ============================================================================ +// Unicode / Non-ASCII Character Tests +// ============================================================================ + +#[test] +fn test_unicode_text_not_escaped() { + // Unicode characters like en-dash should be emitted as raw UTF-8, not escaped. + // Angular's TypeScript emitter does NOT escape non-ASCII printable characters. + let allocator = Allocator::default(); + let source = r#" +import { Component } from '@angular/core'; + +@Component({ + selector: 'test-comp', + template: 'Hello \u{2013} World', + standalone: true, +}) +export class TestComponent {} +"#; + + let result = transform_angular_file( + &allocator, + "test.component.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let code = &result.code; + // Should contain raw en-dash, not escaped + assert!( + code.contains("\u{2013}") && !code.contains("\\u2013"), + "En-dash should be emitted as raw UTF-8, not as \\u2013. Got:\n{code}" + ); +} + +#[test] +fn test_unicode_non_breaking_space_not_escaped() { + // Non-breaking space (U+00A0) should also be emitted as raw UTF-8 + let allocator = Allocator::default(); + let source = " +import { Component } from '@angular/core'; + +@Component({ + selector: 'test-comp', + template: 'Hello\u{00A0}World', + standalone: true, +}) +export class TestComponent {} +"; + + let result = transform_angular_file( + &allocator, + "test.component.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let code = &result.code; + // Should contain raw non-breaking space, not escaped + assert!( + code.contains("\u{00A0}") && !code.contains("\\u00A0"), + "Non-breaking space should be emitted as raw UTF-8, not as \\u00A0. Got:\n{code}" + ); +} + +/// Test that variable naming in embedded view listeners matches Angular TS output. +/// +/// This is based on a real-world example from the ClickUp codebase where Angular TS +/// generates `_r1` and `ctx_r1` but OXC generates `_r2` and `ctx_r2` (off by 1). +/// +/// The root cause is that the root view's SavedView variable (prepended by +/// save_and_restore_view) uses up a counter slot even though it gets optimized +/// away. The naming phase should produce the same numbering as Angular TS. +#[test] +fn test_variable_naming_in_embedded_view_listener() { + let allocator = Allocator::default(); + // Template has ng-template with a listener that accesses the component context. + // This is a simplified version of ObjectMentionsSelectV2Component. + let source = r#" +import { Component } from '@angular/core'; + +@Component({ + selector: 'test-comp', + template: ` + + + + `, + standalone: true, +}) +export class TestComponent { + onClick(e: any) {} +} +"#; + + let result = transform_angular_file( + &allocator, + "test.component.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let code = &result.code; + + // Angular TS generates: + // const _r1 = i0.ɵɵgetCurrentView(); <-- SavedView in embedded view + // ... + // i0.ɵɵrestoreView(_r1); + // const ctx_r1 = i0.ɵɵnextContext(); <-- Context variable + // + // If the root view's unused SavedView variable consumes a counter slot, + // OXC would produce _r2 and ctx_r2 instead. + assert!( + code.contains("_r1") && !code.contains("_r2"), + "Embedded view should use _r1 for saved view, not _r2. Got:\n{code}" + ); +} + +/// Test that variable naming matches Angular TS when root view has listeners. +/// +/// When the root view has listeners (that don't need RestoreView), the root's +/// SavedView variable should still be optimized away. Angular TS's root template +/// does NOT have getCurrentView() in this case. +#[test] +fn test_variable_naming_root_listener_no_savedview() { + let allocator = Allocator::default(); + // Template: root view has a listener + an ng-template with its own listener. + // The root listener uses `ctx` directly, so the root's SavedView is unnecessary. + let source = r#" +import { Component } from '@angular/core'; + +@Component({ + selector: 'test-comp', + template: ` +
Root
+ + + + `, + standalone: true, +}) +export class TestComponent { + onRootClick() {} + onClick(e: any) {} +} +"#; + + let result = transform_angular_file( + &allocator, + "test.component.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let code = &result.code; + + // The root template should NOT have getCurrentView() since the root listener + // doesn't need RestoreView (it accesses ctx directly). + // The embedded view should use _r1 (not _r2). + // + // If the root's unused SavedView leaks into the naming counter, + // the embedded view would use _r2 instead of _r1. + let root_template_section = code.split("function TestComponent_Template(").nth(1).unwrap_or(""); + + let has_root_getcurrentview = + root_template_section.split("function ").next().unwrap_or("").contains("getCurrentView"); + + assert!( + !has_root_getcurrentview, + "Root template should NOT have getCurrentView() since root listeners don't need RestoreView. Got:\n{code}" + ); + + // The embedded view should use _r1, not _r2 + assert!(code.contains("_r1"), "Embedded view should use _r1 for saved view. Got:\n{code}"); + // _r2 should NOT appear + assert!( + !code.contains("_r2"), + "No _r2 should appear (would indicate counter offset from unused root SavedView). Got:\n{code}" + ); +} + +/// Test variable naming with a template reference (like #notifyFooter). +/// +/// Based on the real ClickUp ObjectMentionsSelectV2Component which uses: +/// with a listener inside +/// Angular TS uses _r1 for the embedded view, but OXC may use _r2 if the +/// root's SavedView consumes the counter. +#[test] +fn test_variable_naming_with_template_ref() { + let allocator = Allocator::default(); + let source = r#" +import { Component } from '@angular/core'; + +@Component({ + selector: 'test-comp', + template: ` +
Root
+ + + + `, + standalone: true, +}) +export class TestComponent { + onRootClick() {} + onClick(e: any) {} +} +"#; + + let result = transform_angular_file( + &allocator, + "test.component.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + + let code = &result.code; + // Angular TS uses _r1 for the embedded view's saved view + assert!(code.contains("_r1"), "Embedded view should use _r1 for saved view. Got:\n{code}"); + // If the root's SavedView consumed the counter, we'd see _r2 + assert!(!code.contains("_r2"), "No _r2 should appear. Got:\n{code}"); +} + +/// Test that host binding pure function declarations are emitted in the output. +/// +/// When a component has a host `[class]` binding with an array literal containing +/// a dynamic value, the compiler extracts a pure function constant (e.g., `_c0`). +/// This constant must be emitted in the output — not silently dropped. +/// +/// Regression test for: host binding pool constants not being emitted in +/// compile_template_to_js_with_options path. +#[test] +fn test_host_binding_pure_function_declarations_emitted() { + use oxc_angular_compiler::{HostMetadataInput, compile_template_to_js_with_options}; + + let allocator = Allocator::default(); + let template = "

hello

"; + + let options = ComponentTransformOptions { + host: Some(HostMetadataInput { + properties: vec![("[class]".to_string(), r#"["my-class", typeClass()]"#.to_string())], + attributes: vec![], + listeners: vec![], + class_attr: None, + style_attr: None, + }), + selector: Some("my-comp".to_string()), + ..Default::default() + }; + + let result = compile_template_to_js_with_options( + &allocator, + template, + "MyComponent", + "test.ts", + &options, + ); + + match result { + Ok(output) => { + let code = &output.code; + // The pure function constant DEFINITION (e.g., `const _c0 = ...`) must be present. + // Without the fix, the host binding function body references _c0 but the + // const definition is silently dropped, causing a runtime ReferenceError. + assert!( + code.contains("const _c"), + "Host binding pure function constant definition (const _c0 = ...) must be emitted. Got:\n{code}" + ); + // The pureFunction1 call must reference the constant + assert!( + code.contains("pureFunction1"), + "Host binding should use pureFunction1 for array literal with dynamic value. Got:\n{code}" + ); + } + Err(e) => { + panic!("Compilation failed: {e:?}"); + } + } +} diff --git a/crates/oxc_angular_compiler/tests/playground_repro.rs b/crates/oxc_angular_compiler/tests/playground_repro.rs index 3487fadef..86b4473d3 100644 --- a/crates/oxc_angular_compiler/tests/playground_repro.rs +++ b/crates/oxc_angular_compiler/tests/playground_repro.rs @@ -668,11 +668,16 @@ fn test_listener_unused_refs_removed() { "Click handler should NOT contain reference() calls for unused refs.\nListener body:\n{listener_body}" ); - // The listener SHOULD contain restoreView as a statement (not variable) - // since restore view is side-effectful but unused + // The listener should NOT contain restoreView/resetView since the listener + // doesn't reference any parent context variables. The optimizer should + // strip restoreView/resetView entirely, leaving just `return ctx.setMenuWidth();` assert!( - listener_body.contains("restoreView(") && !listener_body.contains("const _ctx = "), - "Click handler should have restoreView as statement, not variable.\nListener body:\n{listener_body}" + !listener_body.contains("restoreView("), + "Click handler should NOT contain restoreView - handler doesn't use refs.\nListener body:\n{listener_body}" + ); + assert!( + !listener_body.contains("resetView("), + "Click handler should NOT contain resetView - handler doesn't use refs.\nListener body:\n{listener_body}" ); } diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__animation_enter_string_literal_embedded_view.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__animation_enter_string_literal_embedded_view.snap index 1aa749796..62b774bd8 100644 --- a/crates/oxc_angular_compiler/tests/snapshots/integration_test__animation_enter_string_literal_embedded_view.snap +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__animation_enter_string_literal_embedded_view.snap @@ -8,8 +8,7 @@ function TestComponent_Conditional_0_Template(rf,ctx) { i0.ɵɵtext(0,"\n "); i0.ɵɵelementStart(1,"div",0); i0.ɵɵanimateEnter(function TestComponent_Conditional_0_Template_animateenter_cb() { - i0.ɵɵrestoreView(_r1); - return i0.ɵɵresetView("animate-in"); + return "animate-in"; }); i0.ɵɵlistener("click",function TestComponent_Conditional_0_Template_div_click_1_listener() { i0.ɵɵrestoreView(_r1); diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__animation_enter_string_literal_only_embedded_view.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__animation_enter_string_literal_only_embedded_view.snap index eb80542a9..a166127d6 100644 --- a/crates/oxc_angular_compiler/tests/snapshots/integration_test__animation_enter_string_literal_only_embedded_view.snap +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__animation_enter_string_literal_only_embedded_view.snap @@ -4,21 +4,19 @@ expression: js --- function TestComponent_Conditional_0_Template(rf,ctx) { if ((rf & 1)) { - const _r1 = i0.ɵɵgetCurrentView(); i0.ɵɵtext(0,"\n "); i0.ɵɵelementStart(1,"div"); i0.ɵɵanimateEnter(function TestComponent_Conditional_0_Template_animateenter_cb() { - i0.ɵɵrestoreView(_r1); - return i0.ɵɵresetView("animate-in"); + return "animate-in"; }); i0.ɵɵtext(2); i0.ɵɵelementEnd(); i0.ɵɵtext(3,"\n "); } if ((rf & 2)) { - const ctx_r1 = i0.ɵɵnextContext(); + const ctx_r0 = i0.ɵɵnextContext(); i0.ɵɵadvance(2); - i0.ɵɵtextInterpolate1("\n ",ctx_r1.label,"\n "); + i0.ɵɵtextInterpolate1("\n ",ctx_r0.label,"\n "); } } function TestComponent_Template(rf,ctx) { diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__unused_template_reference.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__unused_template_reference.snap index ad085be07..6caf47737 100644 --- a/crates/oxc_angular_compiler/tests/snapshots/integration_test__unused_template_reference.snap +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__unused_template_reference.snap @@ -4,12 +4,10 @@ expression: js --- function TestComponent_Template(rf,ctx) { if ((rf & 1)) { - const _r1 = i0.ɵɵgetCurrentView(); i0.ɵɵelement(0,"input",null,0); i0.ɵɵelementStart(2,"button",1); i0.ɵɵlistener("click",function TestComponent_Template_button_click_2_listener() { - i0.ɵɵrestoreView(_r1); - return i0.ɵɵresetView(ctx.doSomething()); + return ctx.doSomething(); }); i0.ɵɵelementEnd(); }