Skip to content

Commit bf443cb

Browse files
Brooooooklynclaude
andauthored
fix: i18n attribute bindings should not inflate xref IDs for @for index variables (#7)
In Angular's TS compiler, BindingOp.i18nMessage stores a direct object reference to the i18n.Message -- no XrefId is allocated during ingest. The xref for the i18n context is only allocated later in the create_i18n_contexts phase. Oxc was allocating xrefs for i18n messages during ingest, inflating the xref counter and causing @for body views to get higher xref values than Angular. This made generated variable names like ɵ$index_N use the wrong N value (e.g., ɵ$index_70 instead of ɵ$index_66). Change i18n_message fields from Option<XrefId> to Option<u32> across all ops, using the i18n message's instance_id as a dedup key instead of allocating xrefs. This matches Angular TS's approach of using object identity for dedup without consuming xref slots. Fixes 8 mismatched files in the ClickUp comparison (158 → 150). Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent b1141ac commit bf443cb

File tree

9 files changed

+226
-181
lines changed

9 files changed

+226
-181
lines changed

crates/oxc_angular_compiler/src/ir/ops.rs

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,8 +1264,8 @@ pub struct I18nStartOp<'a> {
12641264
pub slot: Option<SlotId>,
12651265
/// I18n context.
12661266
pub context: Option<XrefId>,
1267-
/// Message.
1268-
pub message: Option<XrefId>,
1267+
/// Message instance ID for metadata lookup.
1268+
pub message: Option<u32>,
12691269
/// I18n placeholder data (start_name and close_name for i18n blocks).
12701270
pub i18n_placeholder: Option<I18nPlaceholder<'a>>,
12711271
/// Sub-template index for nested templates inside i18n blocks.
@@ -1289,8 +1289,8 @@ pub struct I18nOp<'a> {
12891289
pub slot: Option<SlotId>,
12901290
/// I18n context.
12911291
pub context: Option<XrefId>,
1292-
/// Message.
1293-
pub message: Option<XrefId>,
1292+
/// Message instance ID for metadata lookup.
1293+
pub message: Option<u32>,
12941294
/// I18n placeholder data (start_name and close_name for i18n blocks).
12951295
pub i18n_placeholder: Option<I18nPlaceholder<'a>>,
12961296
/// Sub-template index for nested templates inside i18n blocks.
@@ -1321,8 +1321,8 @@ pub struct IcuStartOp<'a> {
13211321
pub xref: XrefId,
13221322
/// I18n context.
13231323
pub context: Option<XrefId>,
1324-
/// Message.
1325-
pub message: Option<XrefId>,
1324+
/// Message instance ID for metadata lookup.
1325+
pub message: Option<u32>,
13261326
/// ICU placeholder.
13271327
pub icu_placeholder: Option<Atom<'a>>,
13281328
}
@@ -1365,8 +1365,11 @@ pub struct I18nContextOp<'a> {
13651365
/// Maps ICU placeholder names to their formatted string values.
13661366
/// These are string literals like "Hello ${�0�}!" generated from IcuPlaceholderOp.
13671367
pub icu_placeholder_literals: oxc_allocator::HashMap<'a, Atom<'a>, Atom<'a>>,
1368-
/// Message reference.
1369-
pub message: Option<XrefId>,
1368+
/// Message instance ID reference (for metadata lookup).
1369+
///
1370+
/// Stores the i18n message's instance_id (not an XrefId) to look up metadata
1371+
/// in the job's i18n_message_metadata map.
1372+
pub message: Option<u32>,
13701373
}
13711374

13721375
/// I18n attributes on an element.
@@ -1439,8 +1442,13 @@ pub struct ExtractedAttributeOp<'a> {
14391442
pub security_context: SecurityContext,
14401443
/// Whether expression is truthy.
14411444
pub truthy_expression: bool,
1442-
/// i18n message (for i18n attributes).
1443-
pub i18n_message: Option<XrefId>,
1445+
/// i18n message instance ID (for i18n attributes).
1446+
///
1447+
/// This stores the i18n message's instance_id rather than an XrefId to avoid
1448+
/// allocating xrefs during ingest. Angular TS stores a direct object reference
1449+
/// to the i18n.Message; we use the instance_id as a dedup key instead.
1450+
/// The actual xref for the i18n context is allocated later in create_i18n_contexts.
1451+
pub i18n_message: Option<u32>,
14441452
/// i18n context.
14451453
pub i18n_context: Option<XrefId>,
14461454
/// Trusted value function for security-sensitive constant attributes.
@@ -1522,8 +1530,10 @@ pub struct PropertyOp<'a> {
15221530
pub is_structural: bool,
15231531
/// I18n context.
15241532
pub i18n_context: Option<XrefId>,
1525-
/// I18n message.
1526-
pub i18n_message: Option<XrefId>,
1533+
/// I18n message instance ID.
1534+
///
1535+
/// Stores the i18n message's instance_id for dedup, not an XrefId.
1536+
pub i18n_message: Option<u32>,
15271537
/// Binding kind (for DomOnly mode and animation handling).
15281538
pub binding_kind: BindingKind,
15291539
}
@@ -1608,8 +1618,10 @@ pub struct AttributeOp<'a> {
16081618
pub sanitizer: Option<Atom<'a>>,
16091619
/// I18n context.
16101620
pub i18n_context: Option<XrefId>,
1611-
/// I18n message.
1612-
pub i18n_message: Option<XrefId>,
1621+
/// I18n message instance ID.
1622+
///
1623+
/// Stores the i18n message's instance_id for dedup, not an XrefId.
1624+
pub i18n_message: Option<u32>,
16131625
/// Whether this is a text attribute (static attribute from template).
16141626
///
16151627
/// Text attributes are extractable to the consts array and don't need
@@ -1811,8 +1823,10 @@ pub struct BindingOp<'a> {
18111823
pub unit: Option<Atom<'a>>,
18121824
/// Security context.
18131825
pub security_context: SecurityContext,
1814-
/// I18n message.
1815-
pub i18n_message: Option<XrefId>,
1826+
/// I18n message instance ID.
1827+
///
1828+
/// Stores the i18n message's instance_id for dedup, not an XrefId.
1829+
pub i18n_message: Option<u32>,
18161830
/// Whether this binding came from a text attribute (e.g., `class="cls"` vs `[class]="expr"`).
18171831
///
18181832
/// This is used for compatibility with TemplateDefinitionBuilder which treats
@@ -1843,8 +1857,10 @@ pub struct AnimationOp<'a> {
18431857
pub handler_ops: Vec<'a, UpdateOp<'a>>,
18441858
/// Function name for the handler.
18451859
pub handler_fn_name: Option<Atom<'a>>,
1846-
/// I18n message.
1847-
pub i18n_message: Option<XrefId>,
1860+
/// I18n message instance ID.
1861+
///
1862+
/// Stores the i18n message's instance_id for dedup, not an XrefId.
1863+
pub i18n_message: Option<u32>,
18481864
/// Security context.
18491865
pub security_context: SecurityContext,
18501866
/// Sanitizer function.

crates/oxc_angular_compiler/src/pipeline/compilation.rs

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -139,16 +139,12 @@ pub struct ComponentCompilationJob<'a> {
139139
pub mode: TemplateCompilationMode,
140140
/// Whether this is for an i18n template.
141141
pub is_i18n_template: bool,
142-
/// Metadata for i18n messages keyed by xref.
143-
pub i18n_message_metadata: FxHashMap<XrefId, I18nMessageMetadata<'a>>,
144-
/// Cache of i18n xrefs keyed by message identity (custom_id or computed id).
142+
/// Metadata for i18n messages keyed by instance_id.
145143
///
146-
/// This ensures that when the same i18n message is encountered multiple times
147-
/// (e.g., when copying attributes from an element to its conditional), they
148-
/// share the same xref. This is crucial for correct const deduplication because
149-
/// the i18n_const_collection phase assigns i18n variable names (I18N_0, I18N_1)
150-
/// based on the context xref, which in turn depends on the message xref.
151-
pub i18n_xref_by_message_key: FxHashMap<String, XrefId>,
144+
/// The instance_id is a unique u32 assigned to each i18n message during parsing.
145+
/// This avoids allocating xrefs during ingest for i18n messages on attribute bindings,
146+
/// matching Angular TS which stores direct object references on BindingOp.i18nMessage.
147+
pub i18n_message_metadata: FxHashMap<u32, I18nMessageMetadata<'a>>,
152148
/// Whether to use external message IDs in Closure Compiler variable names.
153149
///
154150
/// When true, generates variable names like `MSG_EXTERNAL_abc123$$SUFFIX`.
@@ -227,7 +223,6 @@ impl<'a> ComponentCompilationJob<'a> {
227223
mode: TemplateCompilationMode::default(),
228224
is_i18n_template: false,
229225
i18n_message_metadata: FxHashMap::default(),
230-
i18n_xref_by_message_key: FxHashMap::default(),
231226
i18n_use_external_ids: true, // Default matches Angular's JIT behavior
232227
relative_context_file_path: None,
233228
relocation_entries: Vec::new_in(allocator),
@@ -257,23 +252,6 @@ impl<'a> ComponentCompilationJob<'a> {
257252
id
258253
}
259254

260-
/// Gets or creates an i18n xref for a message with the given key.
261-
///
262-
/// This ensures that when the same i18n message is encountered multiple times
263-
/// (e.g., when copying attributes from an element to its conditional wrapper),
264-
/// they share the same xref. This is crucial for correct const deduplication.
265-
///
266-
/// The key should be derived from the message's identity (custom_id or computed id).
267-
pub fn get_or_create_i18n_xref(&mut self, message_key: String) -> XrefId {
268-
if let Some(&xref) = self.i18n_xref_by_message_key.get(&message_key) {
269-
xref
270-
} else {
271-
let xref = self.allocate_xref_id();
272-
self.i18n_xref_by_message_key.insert(message_key, xref);
273-
xref
274-
}
275-
}
276-
277255
/// Stores an expression and returns its ID.
278256
///
279257
/// Use this instead of inline expressions to avoid cloning.

0 commit comments

Comments
 (0)