diff --git a/crates/oxc_angular_compiler/src/ir/ops.rs b/crates/oxc_angular_compiler/src/ir/ops.rs index bebf9a54b..4049c70f7 100644 --- a/crates/oxc_angular_compiler/src/ir/ops.rs +++ b/crates/oxc_angular_compiler/src/ir/ops.rs @@ -1264,8 +1264,8 @@ pub struct I18nStartOp<'a> { pub slot: Option, /// I18n context. pub context: Option, - /// Message. - pub message: Option, + /// Message instance ID for metadata lookup. + pub message: Option, /// I18n placeholder data (start_name and close_name for i18n blocks). pub i18n_placeholder: Option>, /// Sub-template index for nested templates inside i18n blocks. @@ -1289,8 +1289,8 @@ pub struct I18nOp<'a> { pub slot: Option, /// I18n context. pub context: Option, - /// Message. - pub message: Option, + /// Message instance ID for metadata lookup. + pub message: Option, /// I18n placeholder data (start_name and close_name for i18n blocks). pub i18n_placeholder: Option>, /// Sub-template index for nested templates inside i18n blocks. @@ -1321,8 +1321,8 @@ pub struct IcuStartOp<'a> { pub xref: XrefId, /// I18n context. pub context: Option, - /// Message. - pub message: Option, + /// Message instance ID for metadata lookup. + pub message: Option, /// ICU placeholder. pub icu_placeholder: Option>, } @@ -1365,8 +1365,11 @@ pub struct I18nContextOp<'a> { /// Maps ICU placeholder names to their formatted string values. /// These are string literals like "Hello ${�0�}!" generated from IcuPlaceholderOp. pub icu_placeholder_literals: oxc_allocator::HashMap<'a, Atom<'a>, Atom<'a>>, - /// Message reference. - pub message: Option, + /// Message instance ID reference (for metadata lookup). + /// + /// Stores the i18n message's instance_id (not an XrefId) to look up metadata + /// in the job's i18n_message_metadata map. + pub message: Option, } /// I18n attributes on an element. @@ -1439,8 +1442,13 @@ pub struct ExtractedAttributeOp<'a> { pub security_context: SecurityContext, /// Whether expression is truthy. pub truthy_expression: bool, - /// i18n message (for i18n attributes). - pub i18n_message: Option, + /// i18n message instance ID (for i18n attributes). + /// + /// This stores the i18n message's instance_id rather than an XrefId to avoid + /// allocating xrefs during ingest. Angular TS stores a direct object reference + /// to the i18n.Message; we use the instance_id as a dedup key instead. + /// The actual xref for the i18n context is allocated later in create_i18n_contexts. + pub i18n_message: Option, /// i18n context. pub i18n_context: Option, /// Trusted value function for security-sensitive constant attributes. @@ -1522,8 +1530,10 @@ pub struct PropertyOp<'a> { pub is_structural: bool, /// I18n context. pub i18n_context: Option, - /// I18n message. - pub i18n_message: Option, + /// I18n message instance ID. + /// + /// Stores the i18n message's instance_id for dedup, not an XrefId. + pub i18n_message: Option, /// Binding kind (for DomOnly mode and animation handling). pub binding_kind: BindingKind, } @@ -1608,8 +1618,10 @@ pub struct AttributeOp<'a> { pub sanitizer: Option>, /// I18n context. pub i18n_context: Option, - /// I18n message. - pub i18n_message: Option, + /// I18n message instance ID. + /// + /// Stores the i18n message's instance_id for dedup, not an XrefId. + pub i18n_message: Option, /// Whether this is a text attribute (static attribute from template). /// /// Text attributes are extractable to the consts array and don't need @@ -1811,8 +1823,10 @@ pub struct BindingOp<'a> { pub unit: Option>, /// Security context. pub security_context: SecurityContext, - /// I18n message. - pub i18n_message: Option, + /// I18n message instance ID. + /// + /// Stores the i18n message's instance_id for dedup, not an XrefId. + pub i18n_message: Option, /// Whether this binding came from a text attribute (e.g., `class="cls"` vs `[class]="expr"`). /// /// This is used for compatibility with TemplateDefinitionBuilder which treats @@ -1843,8 +1857,10 @@ pub struct AnimationOp<'a> { pub handler_ops: Vec<'a, UpdateOp<'a>>, /// Function name for the handler. pub handler_fn_name: Option>, - /// I18n message. - pub i18n_message: Option, + /// I18n message instance ID. + /// + /// Stores the i18n message's instance_id for dedup, not an XrefId. + pub i18n_message: Option, /// Security context. pub security_context: SecurityContext, /// Sanitizer function. diff --git a/crates/oxc_angular_compiler/src/pipeline/compilation.rs b/crates/oxc_angular_compiler/src/pipeline/compilation.rs index 7c65061ed..f0a1896ca 100644 --- a/crates/oxc_angular_compiler/src/pipeline/compilation.rs +++ b/crates/oxc_angular_compiler/src/pipeline/compilation.rs @@ -139,16 +139,12 @@ pub struct ComponentCompilationJob<'a> { pub mode: TemplateCompilationMode, /// Whether this is for an i18n template. pub is_i18n_template: bool, - /// Metadata for i18n messages keyed by xref. - pub i18n_message_metadata: FxHashMap>, - /// Cache of i18n xrefs keyed by message identity (custom_id or computed id). + /// Metadata for i18n messages keyed by instance_id. /// - /// This ensures that when the same i18n message is encountered multiple times - /// (e.g., when copying attributes from an element to its conditional), they - /// share the same xref. This is crucial for correct const deduplication because - /// the i18n_const_collection phase assigns i18n variable names (I18N_0, I18N_1) - /// based on the context xref, which in turn depends on the message xref. - pub i18n_xref_by_message_key: FxHashMap, + /// The instance_id is a unique u32 assigned to each i18n message during parsing. + /// This avoids allocating xrefs during ingest for i18n messages on attribute bindings, + /// matching Angular TS which stores direct object references on BindingOp.i18nMessage. + pub i18n_message_metadata: FxHashMap>, /// Whether to use external message IDs in Closure Compiler variable names. /// /// When true, generates variable names like `MSG_EXTERNAL_abc123$$SUFFIX`. @@ -227,7 +223,6 @@ impl<'a> ComponentCompilationJob<'a> { mode: TemplateCompilationMode::default(), is_i18n_template: false, i18n_message_metadata: FxHashMap::default(), - i18n_xref_by_message_key: FxHashMap::default(), i18n_use_external_ids: true, // Default matches Angular's JIT behavior relative_context_file_path: None, relocation_entries: Vec::new_in(allocator), @@ -257,23 +252,6 @@ impl<'a> ComponentCompilationJob<'a> { id } - /// Gets or creates an i18n xref for a message with the given key. - /// - /// This ensures that when the same i18n message is encountered multiple times - /// (e.g., when copying attributes from an element to its conditional wrapper), - /// they share the same xref. This is crucial for correct const deduplication. - /// - /// The key should be derived from the message's identity (custom_id or computed id). - pub fn get_or_create_i18n_xref(&mut self, message_key: String) -> XrefId { - if let Some(&xref) = self.i18n_xref_by_message_key.get(&message_key) { - xref - } else { - let xref = self.allocate_xref_id(); - self.i18n_xref_by_message_key.insert(message_key, xref); - xref - } - } - /// Stores an expression and returns its ID. /// /// Use this instead of inline expressions to avoid cloning. diff --git a/crates/oxc_angular_compiler/src/pipeline/ingest.rs b/crates/oxc_angular_compiler/src/pipeline/ingest.rs index 0ef284125..309e26f86 100644 --- a/crates/oxc_angular_compiler/src/pipeline/ingest.rs +++ b/crates/oxc_angular_compiler/src/pipeline/ingest.rs @@ -1137,9 +1137,9 @@ fn ingest_element<'a>( None } else { let i18n_xref = job.allocate_xref_id(); + let instance_id = message.instance_id; - // Store i18n message metadata for later phases - // Clone legacy_ids using the allocator + // Store i18n message metadata keyed by instance_id let mut legacy_ids = Vec::new_in(allocator); for id in message.legacy_ids.iter() { legacy_ids.push(id.clone()); @@ -1169,7 +1169,7 @@ fn ingest_element<'a>( Some(message.message_string.clone()) }, }; - job.i18n_message_metadata.insert(i18n_xref, metadata); + job.i18n_message_metadata.insert(instance_id, metadata); // Create I18nStartOp let i18n_start = CreateOp::I18nStart(I18nStartOp { @@ -1179,12 +1179,12 @@ fn ingest_element<'a>( }, xref: i18n_xref, slot: None, - context: None, // Will be set by create_i18n_contexts phase - message: Some(i18n_xref), // Message xref for metadata lookup - i18n_placeholder: None, // Root i18n block has no placeholder - sub_template_index: None, // Will be set by propagate_i18n_blocks phase - root: None, // Root i18n block has no root - message_index: None, // Will be set by i18n_const_collection phase + context: None, // Will be set by create_i18n_contexts phase + message: Some(instance_id), // Instance ID for metadata lookup + i18n_placeholder: None, // Root i18n block has no placeholder + sub_template_index: None, // Will be set by propagate_i18n_blocks phase + root: None, // Root i18n block has no root + message_index: None, // Will be set by i18n_const_collection phase }); if let Some(view) = job.view_mut(view_xref) { @@ -1384,28 +1384,19 @@ fn ingest_static_attributes_with_i18n<'a>( // Handle i18n message if present (for i18n-* attribute markers) // This matches Angular's asMessage(attr.i18n) in ingest.ts line 1329 // - // IMPORTANT: Use a cached xref based on the message's instance_id to ensure - // that when the SAME attribute is encountered twice (once for the conditional via - // ingestControlFlowInsertionPoint, once for the element via this function), both - // uses share the same xref. This matches TypeScript's behavior where Map keys use - // object identity. - // - // Different attributes (even with the same content) should get DIFFERENT xrefs, - // which is crucial for correct const deduplication - each element with an i18n - // attribute should get its own const entry. + // Angular TS stores the i18n.Message object reference directly. We store the + // instance_id as a dedup key. When the SAME attribute is encountered twice + // (once for the conditional via ingestControlFlowInsertionPoint, once for the + // element via this function), they share the same instance_id since it's assigned + // during parsing and survives moves/copies. // - // We use instance_id rather than pointer address because Rust moves data around - // during iteration (e.g., `for child in branch.children` moves the children), - // which changes memory addresses. The instance_id is assigned during parsing - // and survives moves. + // Different attributes (even with the same content) get DIFFERENT instance_ids, + // which is crucial for correct const deduplication. let i18n_message = if let Some(I18nMeta::Message(ref message)) = attr.i18n { - // Use the instance ID as the cache key (survives Rust moves unlike pointer) - let message_key = format!("i18n_instance_{}", message.instance_id); - - let i18n_xref = job.get_or_create_i18n_xref(message_key); + let instance_id = message.instance_id; // Store i18n message metadata for later phases (only if not already stored) - if !job.i18n_message_metadata.contains_key(&i18n_xref) { + if !job.i18n_message_metadata.contains_key(&instance_id) { let mut legacy_ids = Vec::new_in(allocator); for id in message.legacy_ids.iter() { legacy_ids.push(id.clone()); @@ -1435,10 +1426,10 @@ fn ingest_static_attributes_with_i18n<'a>( Some(message.message_string.clone()) }, }; - job.i18n_message_metadata.insert(i18n_xref, metadata); + job.i18n_message_metadata.insert(instance_id, metadata); } - Some(i18n_xref) + Some(instance_id) } else { None }; @@ -1620,38 +1611,48 @@ fn ingest_binding_owned<'a>( // Handle i18n message if present (for i18n-* attribute bindings) // Ported from Angular's ingestElementBindings in ingest.ts + // + // Angular TS stores the i18n.Message object reference directly on the BindingOp + // without allocating an xref. We store the instance_id as a dedup key instead. + // The xref for the i18n context is allocated later in create_i18n_contexts. let i18n_message = if let Some(I18nMeta::Message(ref message)) = input.i18n { - let i18n_xref = job.allocate_xref_id(); + let instance_id = message.instance_id; - // Store i18n message metadata for later phases - let mut legacy_ids = Vec::new_in(allocator); - for id in message.legacy_ids.iter() { - legacy_ids.push(id.clone()); - } + // Store i18n message metadata for later phases (keyed by instance_id) + if !job.i18n_message_metadata.contains_key(&instance_id) { + let mut legacy_ids = Vec::new_in(allocator); + for id in message.legacy_ids.iter() { + legacy_ids.push(id.clone()); + } - let metadata = I18nMessageMetadata { - message_id: if message.id.is_empty() { None } else { Some(message.id.clone()) }, - custom_id: if message.custom_id.is_empty() { - None - } else { - Some(message.custom_id.clone()) - }, - meaning: if message.meaning.is_empty() { None } else { Some(message.meaning.clone()) }, - description: if message.description.is_empty() { - None - } else { - Some(message.description.clone()) - }, - legacy_ids, - message_string: if message.message_string.is_empty() { - None - } else { - Some(message.message_string.clone()) - }, - }; - job.i18n_message_metadata.insert(i18n_xref, metadata); + let metadata = I18nMessageMetadata { + message_id: if message.id.is_empty() { None } else { Some(message.id.clone()) }, + custom_id: if message.custom_id.is_empty() { + None + } else { + Some(message.custom_id.clone()) + }, + meaning: if message.meaning.is_empty() { + None + } else { + Some(message.meaning.clone()) + }, + description: if message.description.is_empty() { + None + } else { + Some(message.description.clone()) + }, + legacy_ids, + message_string: if message.message_string.is_empty() { + None + } else { + Some(message.message_string.clone()) + }, + }; + job.i18n_message_metadata.insert(instance_id, metadata); + } - Some(i18n_xref) + Some(instance_id) } else { None }; @@ -1925,36 +1926,40 @@ fn ingest_template<'a>( // Ported from Angular's ingest.ts lines 384-397 let i18n_message = if template_kind == TemplateKind::NgTemplate { if let Some(I18nMeta::Message(ref message)) = template.i18n { + let instance_id = message.instance_id; // Clone legacy_ids using the allocator let mut legacy_ids = Vec::new_in(allocator); for id in message.legacy_ids.iter() { legacy_ids.push(id.clone()); } - Some(I18nMessageMetadata { - message_id: if message.id.is_empty() { None } else { Some(message.id.clone()) }, - custom_id: if message.custom_id.is_empty() { - None - } else { - Some(message.custom_id.clone()) - }, - meaning: if message.meaning.is_empty() { - None - } else { - Some(message.meaning.clone()) - }, - description: if message.description.is_empty() { - None - } else { - Some(message.description.clone()) - }, - legacy_ids, - message_string: if message.message_string.is_empty() { - None - } else { - Some(message.message_string.clone()) + Some(( + instance_id, + I18nMessageMetadata { + message_id: if message.id.is_empty() { None } else { Some(message.id.clone()) }, + custom_id: if message.custom_id.is_empty() { + None + } else { + Some(message.custom_id.clone()) + }, + meaning: if message.meaning.is_empty() { + None + } else { + Some(message.meaning.clone()) + }, + description: if message.description.is_empty() { + None + } else { + Some(message.description.clone()) + }, + legacy_ids, + message_string: if message.message_string.is_empty() { + None + } else { + Some(message.message_string.clone()) + }, }, - }) + )) } else { None } @@ -2261,23 +2266,23 @@ fn ingest_template<'a>( // and end ops. For structural directive templates, the i18n ops will be added when ingesting the // element/template the directive is placed on. // Ported from Angular's ingest.ts lines 384-397 - if let Some(metadata) = i18n_message { + if let Some((instance_id, metadata)) = i18n_message { let i18n_xref = job.allocate_xref_id(); - // Store i18n message metadata for later phases - job.i18n_message_metadata.insert(i18n_xref, metadata); + // Store i18n message metadata keyed by instance_id + job.i18n_message_metadata.insert(instance_id, metadata); // Create I18nStartOp and insert after the head of the child view's create list let i18n_start = I18nStartOp { base: CreateOpBase { source_span: Some(template_start_span), ..Default::default() }, xref: i18n_xref, slot: None, - context: None, // Will be set by create_i18n_contexts phase - message: Some(i18n_xref), // Message xref for metadata lookup - i18n_placeholder: None, // Root i18n block has no placeholder - sub_template_index: None, // Will be set by propagate_i18n_blocks phase - root: None, // Root i18n block has no root - message_index: None, // Will be set by i18n_const_collection phase + context: None, // Will be set by create_i18n_contexts phase + message: Some(instance_id), // Instance ID for metadata lookup + i18n_placeholder: None, // Root i18n block has no placeholder + sub_template_index: None, // Will be set by propagate_i18n_blocks phase + root: None, // Root i18n block has no root + message_index: None, // Will be set by i18n_const_collection phase }; // Create I18nEndOp and insert before the tail of the child view's create list @@ -4339,27 +4344,19 @@ fn ingest_control_flow_insertion_point<'a, 'b>( // Handle i18n message if present (for i18n-* attribute markers) // This matches Angular's asMessage(attr.i18n) in ingest.ts line 1879 // - // IMPORTANT: Use a cached xref based on MESSAGE INSTANCE ID to ensure that when - // the SAME attribute is encountered twice (once for the conditional via - // ingestControlFlowInsertionPoint, once for the element via ingestStaticAttributes), - // both uses share the same xref. This matches TypeScript's behavior where Map keys - // use object identity. - // - // Each i18n message has a unique instance_id assigned during parsing. This survives - // moves/copies and ensures correct identity tracking even after Rust's move semantics - // relocate the data. + // Angular TS stores the i18n.Message object reference directly. We store the + // instance_id as a dedup key. When the SAME attribute is encountered twice + // (once for the conditional via ingestControlFlowInsertionPoint, once for the + // element via ingestStaticAttributes), they share the same instance_id since + // it's assigned during parsing and survives moves/copies. // - // Different attributes (even with the same content) should get DIFFERENT xrefs, - // which is crucial for correct const deduplication - each element with an i18n - // attribute should get its own const entry. + // Different attributes (even with the same content) get DIFFERENT instance_ids, + // which is crucial for correct const deduplication. let i18n_message = if let Some(I18nMeta::Message(ref message)) = attr.i18n { - // Use the instance ID as the cache key - let message_key = format!("i18n_instance_{}", message.instance_id); - - let i18n_xref = job.get_or_create_i18n_xref(message_key); + let instance_id = message.instance_id; // Store i18n message metadata for later phases (only if not already stored) - if !job.i18n_message_metadata.contains_key(&i18n_xref) { + if !job.i18n_message_metadata.contains_key(&instance_id) { let mut legacy_ids = Vec::new_in(allocator); for id in message.legacy_ids.iter() { legacy_ids.push(id.clone()); @@ -4389,10 +4386,10 @@ fn ingest_control_flow_insertion_point<'a, 'b>( Some(message.message_string.clone()) }, }; - job.i18n_message_metadata.insert(i18n_xref, metadata); + job.i18n_message_metadata.insert(instance_id, metadata); } - Some(i18n_xref) + Some(instance_id) } else { None }; diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/create_i18n_contexts.rs b/crates/oxc_angular_compiler/src/pipeline/phases/create_i18n_contexts.rs index 7fb1d0b09..432c24e5f 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/create_i18n_contexts.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/create_i18n_contexts.rs @@ -24,7 +24,10 @@ pub fn create_i18n_contexts(job: &mut ComponentCompilationJob<'_>) { // Phase 1: Create i18n context ops for i18n attrs. // For attributes with i18n messages, we create an I18nContext op. - let mut attr_context_by_message: FxHashMap = FxHashMap::default(); + // + // The dedup key is the i18n message's instance_id (u32), matching Angular TS where + // the Map key is the i18n.Message object reference (object identity). + let mut attr_context_by_message: FxHashMap = FxHashMap::default(); let view_xrefs_for_attrs: Vec = std::iter::once(job.root.xref).chain(job.views.keys().copied()).collect(); @@ -32,7 +35,7 @@ pub fn create_i18n_contexts(job: &mut ComponentCompilationJob<'_>) { #[derive(Clone)] struct AttrI18nInfo<'a> { view_xref: XrefId, - message_xref: XrefId, + message_instance_id: u32, is_create_op: bool, // true for ExtractedAttribute, false for update ops // Additional fields to uniquely identify the attribute target: XrefId, // Element xref this attribute belongs to @@ -47,10 +50,10 @@ pub fn create_i18n_contexts(job: &mut ComponentCompilationJob<'_>) { // Check CreateOps (ExtractedAttribute) for op in view.create.iter() { if let CreateOp::ExtractedAttribute(attr_op) = op { - if let Some(msg_xref) = attr_op.i18n_message { + if let Some(instance_id) = attr_op.i18n_message { attr_ops_needing_context.push(AttrI18nInfo { view_xref: *view_xref, - message_xref: msg_xref, + message_instance_id: instance_id, is_create_op: true, target: attr_op.target, name: attr_op.name.clone(), @@ -63,10 +66,10 @@ pub fn create_i18n_contexts(job: &mut ComponentCompilationJob<'_>) { for op in view.update.iter() { match op { UpdateOp::Property(prop_op) => { - if let Some(msg_xref) = prop_op.i18n_message { + if let Some(instance_id) = prop_op.i18n_message { attr_ops_needing_context.push(AttrI18nInfo { view_xref: *view_xref, - message_xref: msg_xref, + message_instance_id: instance_id, is_create_op: false, target: prop_op.target, name: prop_op.name.clone(), @@ -74,10 +77,10 @@ pub fn create_i18n_contexts(job: &mut ComponentCompilationJob<'_>) { } } UpdateOp::Attribute(attr_op) => { - if let Some(msg_xref) = attr_op.i18n_message { + if let Some(instance_id) = attr_op.i18n_message { attr_ops_needing_context.push(AttrI18nInfo { view_xref: *view_xref, - message_xref: msg_xref, + message_instance_id: instance_id, is_create_op: false, target: attr_op.target, name: attr_op.name.clone(), @@ -90,19 +93,19 @@ pub fn create_i18n_contexts(job: &mut ComponentCompilationJob<'_>) { } } - // Create I18nContext ops keyed by i18n_message xref. + // Create I18nContext ops keyed by i18n message instance_id. // // This matches TypeScript's behavior where `attrContextByMessage` uses the i18n.Message // object as the key. In TypeScript, when an attribute is copied (e.g., from element to // conditional via ingestControlFlowInsertionPoint), both uses share the same i18n.Message // object reference, so they get the same context. // - // In Rust, we use the i18n_message xref as the key. Since our ingestion code uses - // pointer-based caching for i18n xrefs, attributes from the same source share the same - // i18n_message xref, which means they'll also share the same context here. + // We use the instance_id as the key. Since the instance_id is assigned during parsing + // and survives Rust moves/copies, attributes from the same source share the same + // instance_id and get the same context here. for info in &attr_ops_needing_context { - // Skip if we've already created a context for this message xref - if attr_context_by_message.contains_key(&info.message_xref) { + // Skip if we've already created a context for this message instance_id + if attr_context_by_message.contains_key(&info.message_instance_id) { continue; } @@ -115,7 +118,7 @@ pub fn create_i18n_contexts(job: &mut ComponentCompilationJob<'_>) { params: oxc_allocator::HashMap::new_in(allocator), postprocessing_params: oxc_allocator::HashMap::new_in(allocator), icu_placeholder_literals: oxc_allocator::HashMap::new_in(allocator), - message: Some(info.message_xref), + message: Some(info.message_instance_id), }); // Add context op to the view @@ -125,12 +128,12 @@ pub fn create_i18n_contexts(job: &mut ComponentCompilationJob<'_>) { view.create.push(context_op); } - attr_context_by_message.insert(info.message_xref, context_xref); + attr_context_by_message.insert(info.message_instance_id, context_xref); } // Assign contexts to the attribute ops using the message-based map for info in attr_ops_needing_context { - if let Some(&context_xref) = attr_context_by_message.get(&info.message_xref) { + if let Some(&context_xref) = attr_context_by_message.get(&info.message_instance_id) { let view = if info.view_xref.0 == 0 { Some(&mut job.root) } else { @@ -174,7 +177,7 @@ pub fn create_i18n_contexts(job: &mut ComponentCompilationJob<'_>) { let mut block_context_by_i18n_block: FxHashMap = FxHashMap::default(); // First pass: collect root i18n blocks and create contexts for them - let mut root_i18n_blocks: Vec<(XrefId, XrefId, Option)> = Vec::new(); // (view_xref, i18n_xref, message) + let mut root_i18n_blocks: Vec<(XrefId, XrefId, Option)> = Vec::new(); // (view_xref, i18n_xref, message_instance_id) // Collect from root view { @@ -326,15 +329,14 @@ fn create_icu_contexts_for_view( ) { let allocator = job.allocator; - // Collect ICU info: (icu_xref, icu_message, current_i18n_xref, current_i18n_message, current_i18n_context) - let mut icu_info: Vec<(XrefId, Option, XrefId, Option, Option)> = - Vec::new(); + // Collect ICU info: (icu_xref, icu_message_id, current_i18n_xref, current_i18n_message_id, current_i18n_context) + let mut icu_info: Vec<(XrefId, Option, XrefId, Option, Option)> = Vec::new(); { let view = if view_xref.0 == 0 { Some(&job.root) } else { job.view(view_xref) }; if let Some(view) = view { - let mut current_i18n: Option<(XrefId, Option, Option)> = None; // (xref, message, context) + let mut current_i18n: Option<(XrefId, Option, Option)> = None; // (xref, message_instance_id, context) for op in view.create.iter() { match op { diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/extract_i18n_messages.rs b/crates/oxc_angular_compiler/src/pipeline/phases/extract_i18n_messages.rs index d812fc0ca..b72c6e90a 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/extract_i18n_messages.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/extract_i18n_messages.rs @@ -118,9 +118,9 @@ pub fn extract_i18n_messages(job: &mut ComponentCompilationJob<'_>) { let i18n_block = i18n_contexts.get(&ctx_op.xref).and_then(|(_, block)| *block); - let metadata = ctx_op - .message - .and_then(|msg_xref| job.i18n_message_metadata.get(&msg_xref)); + let metadata = ctx_op.message.and_then(|instance_id| { + job.i18n_message_metadata.get(&instance_id) + }); messages_to_add.push(I18nMessageOp { base: CreateOpBase::default(), diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/propagate_i18n_blocks.rs b/crates/oxc_angular_compiler/src/pipeline/phases/propagate_i18n_blocks.rs index eb9fc29cd..62bc0b4e6 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/propagate_i18n_blocks.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/propagate_i18n_blocks.rs @@ -79,7 +79,7 @@ fn propagate_i18n_blocks_to_templates( // Track the current i18n block: (root_xref, message, sub_template_index) // root_xref is always the root of the i18n block tree (same for nested blocks) - let mut i18n_block: Option<(XrefId, Option, Option)> = None; + let mut i18n_block: Option<(XrefId, Option, Option)> = None; for (kind, xref, has_placeholder, extra_view, extra_has_placeholder) in ops_info { match kind { @@ -174,7 +174,7 @@ fn wrap_template_with_i18n( job: &mut ComponentCompilationJob<'_>, view_xref: XrefId, root_i18n_xref: XrefId, - message: Option, + message: Option, ) { let view = match job.view(view_xref) { Some(v) => v, diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/wrap_icus.rs b/crates/oxc_angular_compiler/src/pipeline/phases/wrap_icus.rs index ffbd9f384..35a88e35c 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/wrap_icus.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/wrap_icus.rs @@ -24,7 +24,7 @@ pub fn wrap_i18n_icus(job: &mut ComponentCompilationJob<'_>) { fn wrap_icus_in_view(job: &mut ComponentCompilationJob<'_>, view_xref: XrefId) { // First pass: collect information about ICUs that need wrapping - let mut icus_to_wrap: Vec<(NonNull>, Option)> = Vec::new(); + let mut icus_to_wrap: Vec<(NonNull>, Option)> = Vec::new(); { let view = match job.view(view_xref) { diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 72324171e..a1b339e9c 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -3914,3 +3914,31 @@ fn test_nested_if_alias_listener_ctx_reference() { ); insta::assert_snapshot!("nested_if_alias_listener_ctx_reference", js); } + +/// Tests that i18n attribute bindings before a @for block do not inflate the xref IDs +/// used for @for loop index variables. +/// +/// In Angular's TypeScript compiler, BindingOp.i18nMessage stores a direct reference to +/// the i18n.Message object -- no xref is allocated during ingest. The xref for the i18n +/// context is only allocated later during the create_i18n_contexts phase. +/// +/// If Oxc allocates extra xrefs for i18n messages during ingest, the @for body view's +/// xref will be higher than Angular's, causing the generated variable name ɵ$index_N to +/// use the wrong N value. +/// +/// For the template `
text
@for (item of items; track $index) { {{$index}} }`: +/// - Angular TS: div=xref1, i18nAttrs=xref2, forBody=xref3 => ɵ$index_3 +/// - Oxc (buggy): div=xref1, i18nMsg=xref2, i18nAttrs=xref3, forBody=xref4 => ɵ$index_4 +#[test] +fn test_for_index_xref_with_i18n_attribute_binding() { + let js = compile_template_to_js( + r#"
text
+@for (item of items; track $index) { {{$index}} }"#, + "TestComponent", + ); + + // Verify the output matches expected Angular behavior via snapshot. + // The fix ensures Oxc doesn't allocate extra xrefs for i18n messages during ingest, + // matching Angular TS which stores direct i18n.Message object references on BindingOp. + insta::assert_snapshot!("for_index_xref_with_i18n_attribute_binding", js); +} diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_index_xref_with_i18n_attribute_binding.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_index_xref_with_i18n_attribute_binding.snap new file mode 100644 index 000000000..c570aef00 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_index_xref_with_i18n_attribute_binding.snap @@ -0,0 +1,24 @@ +--- +source: crates/oxc_angular_compiler/tests/integration_test.rs +expression: js +--- +function TestComponent_For_4_Template(rf,ctx) { + if ((rf & 1)) { i0.ɵɵtext(0); } + if ((rf & 2)) { + const $index_r1 = ctx.$index; + i0.ɵɵtextInterpolate1(" ",$index_r1," "); + } +} +function TestComponent_Template(rf,ctx) { + if ((rf & 1)) { + i0.ɵɵelementStart(0,"div",0); + i0.ɵɵtext(1,"text"); + i0.ɵɵelementEnd(); + i0.ɵɵtext(2,"\n"); + i0.ɵɵrepeaterCreate(3,TestComponent_For_4_Template,1,1,null,null,i0.ɵɵrepeaterTrackByIndex); + } + if ((rf & 2)) { + i0.ɵɵadvance(3); + i0.ɵɵrepeater(ctx.items); + } +}