From aa10c40a5d10b4a881da262f4314397b04bc563d Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 28 Oct 2026 15:38:29 +0800 Subject: [PATCH 01/11] fix(constant-pool): share long strings and literals across a file like ngtsc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port Angular's `ConstantPool.getConstLiteral` string rule (packages/compiler/src/constant_pool.ts: `getConstLiteral`, `FixupExpression`, `POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS`, `isLongStringLiteral`). A string of 50+ UTF-16 units stays inline on its first use; its second use makes one shared `const`, and the first use is patched to read it. Component styles now go through it, as in `compileComponentFromMetadata` (render3/view/compiler.ts: `constantPool.getConstLiteral(literal(style))`). ngtsc compiles a file with one pool. Oxc keeps one pool per class, so the file transform hands the pooled literals from one component's pool to the next. Two components with the same long style now share `_c0`, and two `@ViewChild('x')` queries share one `["x"]`. When a later component shares a style an earlier one has inline, the earlier ɵcmp is re-emitted with the constant and the constant is declared before that class. A component that fails to compile hands no literals on, like it leaves the `_c` index. Also stop counting component-specific names (`Cmp_Template`, ...) in the `_c` index handed to the next class, which shifted its constants. Only the shared `_forTrack` and `arrowFn` functions are numbered across the file (ngtsc's `uniqueName` keeps one count per name), so a class named `_A` no longer shifts the next class's `_c` or `Template` names. Co-Authored-By: Claude Opus 5.5 --- .../src/component/definition.rs | 20 +- .../src/component/transform.rs | 193 +++++++++++-- .../src/pipeline/constant_pool.rs | 265 +++++++++++++++--- .../oxc_angular_compiler/src/pipeline/emit.rs | 30 ++ .../tests/const_pool_long_strings_test.rs | 182 ++++++++++++ 5 files changed, 629 insertions(+), 61 deletions(-) create mode 100644 crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs diff --git a/crates/oxc_angular_compiler/src/component/definition.rs b/crates/oxc_angular_compiler/src/component/definition.rs index ff0c56ea4..808b1fe14 100644 --- a/crates/oxc_angular_compiler/src/component/definition.rs +++ b/crates/oxc_angular_compiler/src/component/definition.rs @@ -422,10 +422,22 @@ fn generate_cmp_definition<'a>( } let style_value = Ident::from_in(style.as_str(), &allocator); - style_entries.push(OutputExpression::Literal(Box::new_in( - LiteralExpr { value: LiteralValue::String(style_value), source_span: None }, - &allocator, - ))); + // A long style is shared once the file uses it twice (Angular's + // `constantPool.getConstLiteral(literal(style))`). + style_entries.push(job.pool.get_const_literal( + OutputExpression::Literal(Box::new_in( + LiteralExpr { value: LiteralValue::String(style_value), source_span: None }, + &allocator, + )), + false, + )); + } + // A style used twice here was inline on its first use; point that use + // at the constant too, like Angular's `FixupExpression` does. + for entry in &mut style_entries { + if let Some(shared) = job.pool.shared_literal(entry) { + *entry = shared; + } } if !style_entries.is_empty() { diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index ea6984a72..da10e7aab 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -56,8 +56,8 @@ use crate::ng_module::{ extract_ng_module_metadata_in, find_ng_module_decorator, generate_full_ng_module_definition, }; use crate::output::ast::{ - ArrowFunctionBody, ArrowFunctionExpr, DeclareFunctionStmt, FunctionExpr, OutputExpression, - OutputStatement, RawSourceExpr, ReadPropExpr, ReadVarExpr, StmtModifier, + ArrowFunctionBody, ArrowFunctionExpr, DeclareFunctionStmt, DeclareVarStmt, FunctionExpr, + OutputExpression, OutputStatement, RawSourceExpr, ReadPropExpr, ReadVarExpr, StmtModifier, }; use crate::output::emitter::JsEmitter; use crate::parser::ParseTemplateOptions; @@ -67,8 +67,12 @@ use crate::pipe::{ extract_pipe_metadata_in, find_pipe_decorator, generate_full_pipe_definition_from_decorator, }; use crate::pipeline::compilation::{DeferBlockDepsEmitMode, TemplateCompilationMode}; +use crate::pipeline::constant_pool::{ + ConstantPool, InheritedFixup, PooledConstantKind, SharedLiterals, is_long_string_literal, +}; use crate::pipeline::emit::{ HostBindingCompilationResult, compile_host_bindings, compile_template, + declare_pooled_constants_from, }; use crate::pipeline::ingest::{ HostBindingInput, IngestOptions, ingest_component, ingest_component_with_options, @@ -3173,6 +3177,11 @@ pub fn transform_angular_file( // TypeScript Angular uses ONE file-level constant pool; we simulate this // by tracking the next index and passing it to each component. let mut shared_pool_index: u32 = 0; + // Likewise, the literals the file's components pooled so far, and the + // components whose ɵcmp has a long style inline that a later component + // may share: (class name, ɵcmp, its emitted text). + let mut shared_literals = SharedLiterals::default(); + let mut inline_style_cmps: Vec<(String, OutputExpression<'_>, String)> = Vec::new(); // The components compiled for HMR, whose inline template and styles the // HMR fingerprint leaves out. @@ -3613,6 +3622,7 @@ pub fn transform_angular_file( view_queries, content_queries, shared_pool_index, + &mut shared_literals, &mut file_namespace_registry, &import_map, resolved_resources, @@ -3620,9 +3630,16 @@ pub fn transform_angular_file( Some(&import_elision_mut), false, ) { - Ok(compilation_result) => { + Ok(mut compilation_result) => { // Update the shared pool index for the next component shared_pool_index = compilation_result.next_pool_index; + apply_inherited_fixups( + allocator, + std::mem::take(&mut compilation_result.inherited_fixups), + &mut inline_style_cmps, + &mut class_definitions, + &mut compilation_result.declarations_js, + ); // `@defer` loads these with a dynamic `import()` now. for name in &compilation_result.implicitly_deferred_imports { @@ -3905,6 +3922,13 @@ pub fn transform_angular_file( class_name.clone(), (property_assignments, decls_before_class, decls_after_class), ); + if let Some(cmp_definition) = compilation_result.cmp_definition.take() { + inline_style_cmps.push(( + class_name.clone(), + cmp_definition, + compilation_result.cmp_js.clone(), + )); + } class_positions.push(( class_name, compute_effective_start( @@ -4844,7 +4868,7 @@ pub fn transform_angular_file( } /// Result of full component compilation including ɵcmp/ɵfac. -struct FullCompilationResult { +struct FullCompilationResult<'a> { /// Compiled template function as JavaScript, for HMR template updates. /// `None` when HMR is off (nothing reads it then), for an HMR update /// compile (see `hmr_update`) and in partial mode. @@ -4900,6 +4924,15 @@ struct FullCompilationResult { /// The template fields of the ɵcmp, for an HMR update compile only. hmr_update: Option, + + /// The ɵcmp `cmp_js` was emitted from, kept when its styles hold a long + /// string inline that a later component of the file may share. + cmp_definition: Option>, + + /// The long styles an earlier component of the file has inline that this + /// one shared: the caller declares each before that component and points + /// its ɵcmp at the constant. + inherited_fixups: Vec>, } /// What an HMR update module needs from a component's full compile: the @@ -5240,6 +5273,7 @@ pub fn compile_component_hmr_update( view_queries, content_queries, 0, + &mut SharedLiterals::default(), &mut namespace_registry, &import_map, Some(resources), @@ -5309,7 +5343,7 @@ fn compile_component_partial<'a>( content_queries: &[R3QueryMetadata<'a>], pool_starting_index: u32, namespace_registry: &mut NamespaceRegistry<'a>, -) -> FullCompilationResult { +) -> FullCompilationResult<'a> { let inputs = crate::partial::PartialComponentInputs { template, is_inline: metadata.template.is_some(), @@ -5358,6 +5392,8 @@ fn compile_component_partial<'a>( template_style_urls: Vec::new(), missing_link_urls: Vec::new(), hmr_update: None, + cmp_definition: None, + inherited_fixups: Vec::new(), } } @@ -5455,13 +5491,14 @@ fn compile_component_full<'a>( view_queries: OxcVec<'a, R3QueryMetadata<'a>>, content_queries: OxcVec<'a, R3QueryMetadata<'a>>, pool_starting_index: u32, + shared_literals: &mut SharedLiterals<'a>, namespace_registry: &mut NamespaceRegistry<'a>, import_map: &ImportMap<'a>, resolved_resources: Option<&ResolvedResources>, deferrable_selector: Option>, import_elision: Option<&ImportElisionAnalyzer<'_>>, hmr_update: bool, -) -> Result> { +) -> Result, Vec> { use oxc_allocator::FromIn; let mut diagnostics = Vec::new(); @@ -5774,6 +5811,10 @@ fn compile_component_full<'a>( ingest_options, ); job.core_namespace = namespace_registry.angular_core_ns(); + // Reuse the literals the file's earlier components pooled: ngtsc compiles + // the whole file with one constant pool. They are handed on only once this + // component compiled, as a failed one does not move `shared_pool_index`. + job.pool.inherit_literals(shared_literals.clone()); // BEFORE template compilation: Pool attrs const if first selector has attributes. // This matches TypeScript Angular which adds attrs to the pool BEFORE template ingestion. @@ -5860,6 +5901,11 @@ fn compile_component_full<'a>( // The namespace registry is shared across all components in the file to ensure // consistent namespace assignments between factory generation and import statements. // Pass the pre-pooled attrs_ref so the attrs entry uses the correct constant index + // The styles pool their long strings after the host bindings' constants. + if let Some(index) = host_binding_next_pool_index { + job.pool.continue_from(index); + } + let template_constant_count = job.pool.len(); let definitions = generate_component_definitions( &allocator, metadata, @@ -5901,7 +5947,7 @@ fn compile_component_full<'a>( "ngContentSelectors", ) .map(|selectors| emitter.emit_expression(selectors)), - styles: cmp_definition_styles(&definitions.cmp_definition), + styles: cmp_definition_styles(&definitions.cmp_definition, &job.pool), }); // Emit host binding declarations (pooled constants like pure functions) @@ -5910,6 +5956,13 @@ fn compile_component_full<'a>( declarations_js.push('\n'); } + // Emit the long styles this component shared (the pool's last constants) + for decl in &declare_pooled_constants_from(&mut job.pool, template_constant_count) { + declarations_js.push_str(&emitter.emit_statement(decl)); + declarations_js.push('\n'); + } + let inherited_fixups = job.pool.take_inherited_fixups(); + // For HMR, we emit the template separately using compile_template_to_js // The ɵcmp already contains the template function inline. Only the HMR // `template_updates` map reads it, so skip the second template compile @@ -5926,9 +5979,14 @@ fn compile_component_full<'a>( } else { None }; + *shared_literals = job.pool.take_literals(); let cmp_js = emitter.emit_expression(&definitions.cmp_definition); let fac_js = emitter.emit_expression(&definitions.fac_definition); + // A later component may share a long style this one has inline; keep the + // ɵcmp to point it at the constant then. + let cmp_definition = cmp_definition_has_inline_long_style(&definitions.cmp_definition) + .then_some(definitions.cmp_definition); // Generate HMR initialization code if enabled let hmr_initializer_js = if options.hmr { @@ -5994,11 +6052,9 @@ fn compile_component_full<'a>( }; // Get the next pool index for the next component in this file - // If host bindings were compiled, use the host binding pool's next index - // (since it continues from the template pool) - // Otherwise, use the template pool's next index - let next_pool_index = - host_binding_next_pool_index.unwrap_or_else(|| job.pool.next_name_index()); + // (the template pool continued from the host binding pool's next index + // before the styles were pooled) + let next_pool_index = job.pool.next_name_index(); // Collect any diagnostics from the compilation job // (Done after using job to avoid borrow issues). @@ -6024,9 +6080,25 @@ fn compile_component_full<'a>( template_style_urls, missing_link_urls, hmr_update, + cmp_definition, + inherited_fixups, }) } +/// The value of `key` in a `ɵɵdefineComponent({...})` call, to change it. +fn cmp_definition_field_mut<'b, 'a>( + cmp_definition: &'b mut OutputExpression<'a>, + key: &str, +) -> Option<&'b mut OutputExpression<'a>> { + let OutputExpression::InvokeFunction(call) = cmp_definition else { return None }; + let Some(OutputExpression::LiteralMap(config)) = call.args.first_mut() else { return None }; + config + .entries + .iter_mut() + .find(|entry| !entry.is_spread && entry.key.as_str() == key) + .map(|entry| &mut entry.value) +} + /// The value of `key` in a `ɵɵdefineComponent({...})` call. fn cmp_definition_field<'b, 'a>( cmp_definition: &'b OutputExpression<'a>, @@ -6042,8 +6114,12 @@ fn cmp_definition_field<'b, 'a>( } /// The finished CSS strings in a `ɵɵdefineComponent({...})` call's `styles` -/// (encapsulated and minified as configured, empty ones dropped). -fn cmp_definition_styles(cmp_definition: &OutputExpression<'_>) -> Vec { +/// (encapsulated and minified as configured, empty ones dropped), reading a +/// shared long style from the component's `pool`. +fn cmp_definition_styles( + cmp_definition: &OutputExpression<'_>, + pool: &ConstantPool<'_>, +) -> Vec { use crate::output::ast::LiteralValue; let Some(OutputExpression::LiteralArray(styles)) = cmp_definition_field(cmp_definition, "styles") @@ -6053,16 +6129,95 @@ fn cmp_definition_styles(cmp_definition: &OutputExpression<'_>) -> Vec { styles .entries .iter() - .filter_map(|style| match style { - OutputExpression::Literal(literal) => match &literal.value { - LiteralValue::String(style) => Some(style.to_string()), + .filter_map(|style| { + let style = match style { + OutputExpression::ReadVar(read) => { + match &pool.constants().iter().find(|c| c.name == read.name)?.kind { + PooledConstantKind::Literal(literal) => literal.as_ref(), + _ => return None, + } + } + style => style, + }; + match style { + OutputExpression::Literal(literal) => match &literal.value { + LiteralValue::String(style) => Some(style.to_string()), + _ => None, + }, _ => None, - }, - _ => None, + } }) .collect() } +/// Whether a `ɵɵdefineComponent({...})` call's `styles` hold a string long +/// enough for the constant pool to share inline (its first use in the file). +fn cmp_definition_has_inline_long_style(cmp_definition: &OutputExpression<'_>) -> bool { + let Some(OutputExpression::LiteralArray(styles)) = + cmp_definition_field(cmp_definition, "styles") + else { + return false; + }; + styles.entries.iter().any(is_long_string_literal) +} + +/// Points the earlier component's inline first use of a long style that a +/// later component shared at the shared constant, and declares it before that +/// component, like the `FixupExpression` ngtsc's single file pool resolves. +/// +/// `inline_style_cmps` holds the components whose ɵcmp has a long style inline, +/// with the ɵcmp text in their `class_definitions` entry. Without one (that +/// component failed to compile), the constant goes before the current class. +fn apply_inherited_fixups<'a>( + allocator: &'a Allocator, + fixups: Vec>, + inline_style_cmps: &mut [(String, OutputExpression<'a>, String)], + class_definitions: &mut HashMap, + current_declarations_js: &mut String, +) { + let emitter = JsEmitter::new(); + for fixup in fixups { + let declaration = + emitter.emit_statement(&OutputStatement::DeclareVar(oxc_allocator::Box::new_in( + DeclareVarStmt { + name: fixup.name, + value: Some(fixup.value.clone_in(allocator)), + modifiers: StmtModifier::FINAL, + leading_comment: None, + source_span: None, + }, + &allocator, + ))); + let first_use = inline_style_cmps.iter_mut().find_map(|(class_name, cmp, cmp_js)| { + let Some(OutputExpression::LiteralArray(styles)) = + cmp_definition_field_mut(cmp, "styles") + else { + return None; + }; + let style = + styles.entries.iter_mut().find(|style| style.is_equivalent(&fixup.value))?; + *style = OutputExpression::ReadVar(oxc_allocator::Box::new_in( + ReadVarExpr { name: fixup.name, source_span: None }, + &allocator, + )); + Some((class_name, emitter.emit_expression(cmp), cmp_js)) + }); + let first_use = first_use.and_then(|(class_name, new_cmp_js, cmp_js)| { + Some((class_definitions.get_mut(class_name.as_str())?, new_cmp_js, cmp_js)) + }); + let Some(((property_assignments, decls_before_class, _), new_cmp_js, cmp_js)) = first_use + else { + current_declarations_js.push_str(&declaration); + current_declarations_js.push('\n'); + continue; + }; + *property_assignments = property_assignments.replacen(cmp_js.as_str(), &new_cmp_js, 1); + decls_before_class.push_str(&declaration); + decls_before_class.push('\n'); + *cmp_js = new_cmp_js; + } +} + /// Resolve template content from inline or external source. /// /// Precedence matches Angular's AOT compiler (`parseTemplateDeclaration` in diff --git a/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs b/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs index 640a40ee4..558afc7b6 100644 --- a/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs +++ b/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs @@ -11,10 +11,26 @@ use rustc_hash::FxHashMap; use crate::ir::expression::IrExpression; use crate::output::ast::{ - DeclareFunctionStmt, DeclareVarStmt, FunctionExpr, OutputExpression, OutputStatement, - ReadVarExpr, StmtModifier, + DeclareFunctionStmt, DeclareVarStmt, FunctionExpr, LiteralValue, OutputExpression, + OutputStatement, ReadVarExpr, StmtModifier, }; +/// Strings at least this long (in UTF-16 code units, like JS `length`) are +/// shared once they are used twice; shorter ones always stay inline. +/// +/// Ported from Angular's `POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS`. +const POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS: usize = 50; + +/// The literals pooled so far in a file, handed from one class's pool to the +/// next. +/// +/// ngtsc compiles every class of a file with one `ConstantPool`, so a literal +/// pooled by one class is reused by the next. Oxc gives each class its own +/// pool (with names chained by the starting index), so the file transform +/// carries the literals over with [`ConstantPool::take_literals`] and +/// [`ConstantPool::inherit_literals`]. +pub type SharedLiterals<'a> = FxHashMap>; + /// A pool for deduplicating constant values. /// /// Constants are keyed by a string representation and deduplicated @@ -37,6 +53,11 @@ pub struct ConstantPool<'a> { /// Claimed names for unique_name deduplication within a single component. /// Maps base name to usage count. claimed_names: FxHashMap, + /// Literals pooled by `get_const_literal`, by key. + literals: SharedLiterals<'a>, + /// Long strings an earlier class of the file used inline once, shared by + /// this pool. Their declaration and first use belong to that earlier class. + inherited_fixups: std::vec::Vec>, /// Statements to emit (e.g., child view functions). pub statements: Vec<'a, crate::output::ast::OutputStatement<'a>>, } @@ -52,6 +73,8 @@ impl<'a> ConstantPool<'a> { next_name_index: 0, unique_name_offset: 0, claimed_names: FxHashMap::default(), + literals: FxHashMap::default(), + inherited_fixups: std::vec::Vec::new(), statements: Vec::new_in(&allocator), } } @@ -79,6 +102,8 @@ impl<'a> ConstantPool<'a> { next_name_index: starting_index, unique_name_offset: starting_index, claimed_names: FxHashMap::default(), + literals: FxHashMap::default(), + inherited_fixups: std::vec::Vec::new(), statements: Vec::new_in(&allocator), } } @@ -89,9 +114,17 @@ impl<'a> ConstantPool<'a> { /// generated (offset + claimed count), ensuring proper state transfer when /// compiling multiple components in the same file. pub fn next_name_index(&self) -> u32 { - // Calculate the total unique names generated: offset + sum of all claimed counts - let total_unique_names: u32 = - self.unique_name_offset + self.claimed_names.values().sum::(); + // Calculate the total unique names generated: offset + claimed counts of + // the generic names the offset applies to. Component-specific names + // (like `ComponentName_Template`) take no index: ngtsc numbers `_c` + // constants on their own. + let total_unique_names: u32 = self.unique_name_offset + + self + .claimed_names + .iter() + .filter(|(base, _)| is_generic_name(base)) + .map(|(_, count)| count) + .sum::(); self.next_name_index.max(total_unique_names) } @@ -408,11 +441,11 @@ impl<'a> ConstantPool<'a> { // Get the per-base-name count for deduplication within this component let count = self.claimed_names.get(base).copied().unwrap_or(0); - // Only apply the global offset for "generic" names (`_forTrack`, `_c`, `arrowFn`). + // Only apply the global offset for "generic" names (`_forTrack`, `arrowFn`). // These names could collide across components. // Component-specific names (like `ComponentName_Template`) don't need the // offset because they already contain the component class name. - let apply_offset = base.starts_with('_') || base == "arrowFn"; + let apply_offset = is_generic_name(base); let effective_count = if apply_offset { self.unique_name_offset + count } else { count }; let name = if effective_count == 0 && !always_include_suffix { @@ -427,51 +460,96 @@ impl<'a> ConstantPool<'a> { Ident::from(self.allocator.alloc_str(&name)) } - /// Gets or creates a constant literal and returns a reference expression. + /// Gets or creates a constant literal and returns the expression to use. /// - /// This is used to share constant literals (arrays, primitives) across the output. - /// If `share` is true, the constant may be deduplicated with existing constants. + /// A short string literal is returned as is. A long string (see + /// `POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS`) stays inline on its + /// first use and becomes a shared constant on its second one, unless + /// `force_shared` asks for the constant right away; any other literal + /// should be passed with `force_shared`. The inline first use is not + /// patched here: see [`ConstantPool::shared_literal`] and + /// [`ConstantPool::take_inherited_fixups`]. /// /// Ported from Angular's `ConstantPool.getConstLiteral()`. - /// - /// # Arguments - /// - /// * `literal` - The literal expression to pool - /// * `share` - Whether to attempt deduplication (currently always adds new constant) - /// - /// # Returns - /// - /// An `OutputExpression::ReadVar` that references the pooled constant. pub fn get_const_literal( &mut self, literal: OutputExpression<'a>, - _share: bool, + force_shared: bool, ) -> OutputExpression<'a> { - // Generate a key from the expression for deduplication - let key = expression_to_key(&literal); + if matches!(literal, OutputExpression::Literal(_)) && !is_long_string_literal(&literal) { + return literal; + } - // Check if already pooled - if let Some(pooled) = self.strings.get(&key) { - let constant = &self.values[pooled.index as usize]; - return OutputExpression::ReadVar(Box::new_in( - ReadVarExpr { name: constant.name.clone(), source_span: None }, - &self.allocator, - )); + let key = expression_to_key(&literal); + let pooled = self.literals.get(&key).copied(); + if let Some(PooledLiteral { name: Some(name), .. }) = pooled { + return self.read_var(name); + } + if pooled.is_none() && !force_shared { + // First use of a long string: it stays inline until a second one. + self.literals.insert(key, PooledLiteral { name: None, inherited: false }); + return literal; } - // Add new constant - let index = self.values.len() as u32; let name = self.generate_name("_c"); + if pooled.is_some_and(|pooled| pooled.inherited) { + // The inline first use is in an earlier class, so the constant + // has to be declared before that class. + self.inherited_fixups.push(InheritedFixup { name, value: literal }); + } else { + let index = self.values.len() as u32; + self.values.push(PooledConstant { + name, + value: PooledValue { index }, + kind: PooledConstantKind::Literal(Box::new_in(literal, &self.allocator)), + }); + } + self.literals.insert(key, PooledLiteral { name: Some(name), inherited: false }); + self.read_var(name) + } - self.values.push(PooledConstant { - name: name.clone(), - value: PooledValue { index }, - kind: PooledConstantKind::Literal(Box::new_in(literal, &self.allocator)), - }); + /// Returns the shared constant for `expr` if it is the inline first use of + /// a long string that a later use has shared since. + /// + /// Angular returns a `FixupExpression` for every pooled literal, which the + /// second use points at the new constant. Callers that may hold a first use + /// run it through this once all uses are pooled. + pub fn shared_literal(&self, expr: &OutputExpression<'a>) -> Option> { + if !is_long_string_literal(expr) { + return None; + } + let name = self.literals.get(&expression_to_key(expr))?.name?; + Some(self.read_var(name)) + } - self.strings.insert(key, PooledValue { index }); + /// Takes the literals this pool and the ones before it pooled, to hand + /// them to the next class's pool. + pub fn take_literals(&mut self) -> SharedLiterals<'a> { + std::mem::take(&mut self.literals) + } + + /// Reuses the literals pooled by the earlier classes of the file. + pub fn inherit_literals(&mut self, mut literals: SharedLiterals<'a>) { + for pooled in literals.values_mut() { + pooled.inherited = true; + } + self.literals = literals; + } + + /// Takes the long strings an earlier class used inline once and this pool + /// shared. The caller declares each before that class and points its + /// first use at the constant. + pub fn take_inherited_fixups(&mut self) -> std::vec::Vec> { + std::mem::take(&mut self.inherited_fixups) + } - // Return a ReadVar expression referencing the constant + /// Moves the next constant name past `index`, the next name of a pool + /// compiled after this one (ngtsc shares one pool between them). + pub fn continue_from(&mut self, index: u32) { + self.next_name_index = self.next_name_index.max(index); + } + + fn read_var(&self, name: Ident<'a>) -> OutputExpression<'a> { OutputExpression::ReadVar(Box::new_in( ReadVarExpr { name, source_span: None }, &self.allocator, @@ -479,6 +557,28 @@ impl<'a> ConstantPool<'a> { } } +/// Whether `unique_name` numbers `base` across the components of a file (the +/// shared `_forTrack` and `arrowFn` functions), as opposed to a +/// component-specific name like `Cmp_Template`, even for a class whose name +/// starts with `_`. +fn is_generic_name(base: &str) -> bool { + matches!(base, "_forTrack" | "arrowFn") +} + +/// Whether `expr` is a string literal long enough to share. +/// +/// Ported from Angular's `isLongStringLiteral`. +pub fn is_long_string_literal(expr: &OutputExpression<'_>) -> bool { + matches!( + expr, + OutputExpression::Literal(lit) if matches!( + &lit.value, + LiteralValue::String(value) + if value.encode_utf16().count() >= POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS + ) + ) +} + /// Generates a string key from an OutputExpression for deduplication. fn expression_to_key(expr: &OutputExpression<'_>) -> String { use crate::output::ast::*; @@ -502,6 +602,26 @@ fn expression_to_key(expr: &OutputExpression<'_>) -> String { } } +/// A literal pooled by [`ConstantPool::get_const_literal`]. +#[derive(Debug, Clone, Copy)] +pub struct PooledLiteral<'a> { + /// The shared constant's name, or `None` while the literal is a long + /// string used once, inline. + pub name: Option>, + /// Whether an earlier class of the file pooled the literal. + pub inherited: bool, +} + +/// A long string that an earlier class of the file used inline once and a +/// later class shared, from [`ConstantPool::take_inherited_fixups`]. +#[derive(Debug)] +pub struct InheritedFixup<'a> { + /// The shared constant's name. + pub name: Ident<'a>, + /// The literal, inline in the earlier class. + pub value: OutputExpression<'a>, +} + /// A pooled value reference. #[derive(Debug, Clone, Copy)] pub struct PooledValue { @@ -643,4 +763,73 @@ mod tests { assert_ne!(idx1, idx3); assert_eq!(pool.len(), 2); } + + fn string<'a>(allocator: &'a Allocator, value: &str) -> OutputExpression<'a> { + OutputExpression::Literal(Box::new_in( + crate::output::ast::LiteralExpr { + value: LiteralValue::String(Ident::from(allocator.alloc_str(value))), + source_span: None, + }, + &allocator, + )) + } + + fn read_var_name<'a>(expr: &OutputExpression<'a>) -> Option<&'a str> { + match expr { + OutputExpression::ReadVar(read) => Some(read.name.as_str()), + _ => None, + } + } + + #[test] + fn test_long_string_is_shared_from_its_second_use() { + let allocator = Allocator::default(); + let mut pool = ConstantPool::new(&allocator); + let long = "x".repeat(POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS); + + let first = pool.get_const_literal(string(&allocator, &long), false); + assert!(matches!(first, OutputExpression::Literal(_))); + assert!(pool.shared_literal(&first).is_none()); + assert!(pool.is_empty()); + + let second = pool.get_const_literal(string(&allocator, &long), false); + assert_eq!(read_var_name(&second), Some("_c0")); + assert_eq!(pool.shared_literal(&first).as_ref().and_then(read_var_name), Some("_c0")); + let third = pool.get_const_literal(string(&allocator, &long), false); + assert_eq!(read_var_name(&third), Some("_c0")); + assert_eq!(pool.len(), 1); + } + + #[test] + fn test_short_string_stays_inline() { + let allocator = Allocator::default(); + let mut pool = ConstantPool::new(&allocator); + // 49 UTF-16 code units, though more bytes in UTF-8. + let short = "é".repeat(POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS - 1); + + for _ in 0..2 { + let expr = pool.get_const_literal(string(&allocator, &short), true); + assert!(matches!(expr, OutputExpression::Literal(_))); + } + assert!(pool.is_empty()); + } + + #[test] + fn test_long_string_first_used_by_an_earlier_pool_is_a_fixup() { + let allocator = Allocator::default(); + let long = "x".repeat(POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS); + let mut first_pool = ConstantPool::new(&allocator); + first_pool.get_const_literal(string(&allocator, &long), false); + + let mut pool = ConstantPool::with_starting_index(&allocator, 3); + pool.inherit_literals(first_pool.take_literals()); + let second = pool.get_const_literal(string(&allocator, &long), false); + assert_eq!(read_var_name(&second), Some("_c3")); + // Declared before the earlier class, not with this pool's constants. + assert!(pool.is_empty()); + let fixups = pool.take_inherited_fixups(); + assert_eq!(fixups.len(), 1); + assert_eq!(fixups[0].name.as_str(), "_c3"); + assert!(fixups[0].value.is_equivalent(&string(&allocator, &long))); + } } diff --git a/crates/oxc_angular_compiler/src/pipeline/emit.rs b/crates/oxc_angular_compiler/src/pipeline/emit.rs index 51c28ab68..4833db7fd 100644 --- a/crates/oxc_angular_compiler/src/pipeline/emit.rs +++ b/crates/oxc_angular_compiler/src/pipeline/emit.rs @@ -19,6 +19,7 @@ use oxc_str::Ident; use crate::ir::ops::XrefId; use super::compilation::{ComponentCompilationJob, HostBindingCompilationJob, ViewCompilationUnit}; +use super::constant_pool::ConstantPool; use super::phases; // ============================================================================ @@ -298,6 +299,35 @@ pub fn compile_template<'a>( TemplateCompilationResult { template_fn, declarations } } +/// Declares the constants pooled from index `start` on. +/// +/// For a component's long styles: they are pooled after `compile_template` +/// has declared the template's constants (Angular's +/// `compileComponentFromMetadata` pools them last). +pub fn declare_pooled_constants_from<'a>( + pool: &mut ConstantPool<'a>, + start: usize, +) -> OxcVec<'a, OutputStatement<'a>> { + use crate::output::ast::DeclareVarStmt; + + let allocator = pool.allocator(); + let mut declarations = OxcVec::new_in(&allocator); + for constant in pool.constants_mut().iter_mut().skip(start) { + let value = emit_pooled_constant_value(allocator, &mut constant.kind); + declarations.push(OutputStatement::DeclareVar(Box::new_in( + DeclareVarStmt { + name: constant.name, + value: Some(value), + modifiers: StmtModifier::FINAL, + leading_comment: None, + source_span: None, + }, + &allocator, + ))); + } + declarations +} + /// Converts a pure function body expression to an output expression. /// /// This function handles `PureFunctionParameterExpr` by converting them to diff --git a/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs b/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs new file mode 100644 index 000000000..f55ca5d0e --- /dev/null +++ b/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs @@ -0,0 +1,182 @@ +//! Long strings (50+ chars) go through ngtsc's constant pool like this: the first use stays +//! inline, and a second use in the same file turns them into one shared `const`, which the +//! first use then reads too. Component styles go through it. ngtsc has one pool per file, so +//! the classes of a file share the constants they pool. + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{ + ComponentHmrUpdateInput, TransformOptions, compile_component_hmr_update, transform_angular_file, +}; + +/// 76 chars, over the 50-char threshold. +const LONG: &str = ".a-long-selector-name .another-long-class { color: red; font-size: 12px; }"; +const LONG_SCOPED: &str = ".a-long-selector-name[_ngcontent-%COMP%] .another-long-class[_ngcontent-%COMP%] { color: red; font-size: 12px; }"; +const OTHER: &str = ".b-long-selector-name .another-long-class { color: blue; font-size: 14px; }"; +const OTHER_SCOPED: &str = ".b-long-selector-name[_ngcontent-%COMP%] .another-long-class[_ngcontent-%COMP%] { color: blue; font-size: 14px; }"; + +fn transform(source: &str) -> String { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, "test.ts", source, None, None); + assert!(!result.has_errors(), "{:?}", result.diagnostics); + result.code +} + +/// `code` without whitespace, so the emitter's line wrapping doesn't matter. +fn squash(code: &str) -> String { + code.chars().filter(|c| !c.is_whitespace()).collect() +} + +fn component(class_name: &str, metadata: &str) -> String { + format!( + "@Component({{selector: '{}', template: '', {metadata} standalone: true}})\n\ + export class {class_name} {{}}\n", + class_name.to_lowercase() + ) +} + +fn declaration(name: &str, value: &str) -> String { + squash(&format!("const {name} = {value:?};")) +} + +#[test] +fn long_style_of_two_components_is_one_constant() { + let source = format!( + "import {{Component}} from '@angular/core';\n{}{}", + component("A", &format!("styles: ['{LONG}'],")), + component("B", &format!("styles: ['{LONG}'],")), + ); + let code = transform(&source); + let squashed = squash(&code); + assert_eq!(squashed.matches(&declaration("_c0", LONG_SCOPED)).count(), 1, "{code}"); + assert_eq!(squashed.matches("styles:[_c0]").count(), 2, "{code}"); + assert!(!squashed.contains("_c1"), "{code}"); + // The first component reads the constant while its class is defined. + assert!(code.find("const _c0").unwrap() < code.find("export class A").unwrap(), "{code}"); +} + +#[test] +fn long_style_of_two_unencapsulated_components_is_one_constant() { + let source = format!( + "import {{Component, ViewEncapsulation}} from '@angular/core';\n{}{}", + component("A", &format!("styles: ['{LONG}'], encapsulation: ViewEncapsulation.None,")), + component("B", &format!("styles: ['{LONG}'], encapsulation: ViewEncapsulation.None,")), + ); + let code = squash(&transform(&source)); + assert_eq!(code.matches(&declaration("_c0", LONG)).count(), 1, "{code}"); + assert_eq!(code.matches("styles:[_c0],encapsulation:2").count(), 2, "{code}"); +} + +#[test] +fn long_style_used_twice_by_one_component_is_one_constant() { + let source = format!( + "import {{Component}} from '@angular/core';\n{}", + component("A", &format!("styles: ['{LONG}', '{LONG}'],")), + ); + let code = squash(&transform(&source)); + assert_eq!(code.matches(&declaration("_c0", LONG_SCOPED)).count(), 1, "{code}"); + assert!(code.contains("styles:[_c0,_c0]"), "{code}"); +} + +#[test] +fn long_style_used_once_stays_inline() { + let source = format!( + "import {{Component}} from '@angular/core';\n{}{}", + component("A", &format!("styles: ['{LONG}'],")), + component("B", "styles: ['.a { color: red }', '.a { color: red }'],"), + ); + let code = squash(&transform(&source)); + assert!(code.contains(&squash(&format!("styles:[{LONG_SCOPED:?}]"))), "{code}"); + assert_eq!(code.matches(r#"".a[_ngcontent-%COMP%]"#).count(), 2, "{code}"); + assert!(!code.contains("_c0"), "{code}"); +} + +#[test] +fn long_styles_are_named_after_the_constants_pooled_before_them() { + // ngtsc: `_c0` is B's pure function, then `_c1` is LONG (shared by B) and + // `_c2` OTHER (shared by C), all after A's inline first uses. + let source = format!( + "import {{Component}} from '@angular/core';\n{}{}{}", + component("A", &format!("styles: ['{OTHER}', '{LONG}'],")), + "@Component({selector: 'b', template: '
', \ + styles: ['LONG'], standalone: true})\nexport class B { v = 1; }\n" + .replace("LONG", LONG), + component("C", &format!("styles: ['{OTHER}'],")), + ); + let code = squash(&transform(&source)); + assert!(code.contains("const_c0=(a0)=>[a0,1];"), "{code}"); + assert!(code.contains(&declaration("_c1", LONG_SCOPED)), "{code}"); + assert!(code.contains(&declaration("_c2", OTHER_SCOPED)), "{code}"); + assert!(code.contains("styles:[_c2,_c1]"), "{code}"); + assert_eq!(code.matches("styles:[_c1]").count(), 1, "{code}"); + assert_eq!(code.matches("styles:[_c2]").count(), 1, "{code}"); +} + +#[test] +fn query_predicate_of_two_components_is_one_constant() { + let source = "import {Component, ViewChild} from '@angular/core'; +@Component({selector: 'a', template: '', standalone: true}) +export class A { @ViewChild('x') a: any; } +@Component({selector: 'b', template: '', standalone: true}) +export class B { @ViewChild('x') b: any; } +"; + let code = squash(&transform(source)); + assert_eq!(code.matches(r#"const_c0=["x"];"#).count(), 1, "{code}"); + assert_eq!(code.matches("i0.ɵɵviewQuery(_c0,5)").count(), 2, "{code}"); + assert!(!code.contains("_c1"), "{code}"); +} + +#[test] +fn hmr_update_sends_a_shared_long_style_as_text() { + let source = format!( + "import {{Component}} from '@angular/core';\n\ + @Component({{selector: 'a', template: 'x', styles: ['{LONG}', '{LONG}']}})\n\ + export class A {{}}\n" + ); + let allocator = Allocator::default(); + let input = ComponentHmrUpdateInput { class_name: "A", ..Default::default() }; + let options = TransformOptions { hmr: true, ..Default::default() }; + let update = + compile_component_hmr_update(&allocator, "test.ts", &source, &input, Some(&options), None); + assert!(update.diagnostics.is_empty(), "{:?}", update.diagnostics); + assert_eq!(update.styles, Some(vec![LONG_SCOPED.to_string(), LONG_SCOPED.to_string()])); +} + +#[test] +fn constants_after_an_underscore_class_skip_its_template_names() { + // ngtsc numbers `_c` constants on their own: `_A_Template` and + // `_A_Conditional_0_Template` take no index, and `_B_Template` no suffix. + let source = format!( + "import {{Component, ViewChild}} from '@angular/core';\n\ + @Component({{selector: 'a', template: '@if (v) {{}}', styles: ['{LONG}'], standalone: true}})\n\ + export class _A {{ v = 1; }}\n\ + @Component({{selector: 'b', template: '@if (v) {{}}', styles: ['{LONG}'], standalone: true}})\n\ + export class _B {{ v = 1; @ViewChild('x') q: any; }}\n" + ); + let code = squash(&transform(&source)); + assert!(code.contains(r#"const_c0=["x"];"#), "{code}"); + assert!(code.contains(&declaration("_c1", LONG_SCOPED)), "{code}"); + assert_eq!(code.matches("styles:[_c1]").count(), 2, "{code}"); + assert!(code.contains("i0.ɵɵviewQuery(_c0,5)"), "{code}"); + assert!(!code.contains("_c2"), "{code}"); + assert!(code.contains("function_B_Template(rf,ctx)"), "{code}"); + assert!(code.contains("function_B_Conditional_0_Template(rf,ctx)"), "{code}"); +} + +#[test] +fn failed_component_hands_no_constants_on() { + // ngtsc emits nothing for the file. Oxc still emits the next component, + // which must declare its own constants rather than read the failed one's. + let source = "import {Component, ViewChild} from '@angular/core'; +@Component({selector: 'b', template: '
Hi
', standalone: true}) +export class B { @ViewChild('x') q: any; } +@Component({selector: 'c', template: '', standalone: true}) +export class C { @ViewChild('x') q: any; @ViewChild('y') r: any; } +"; + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, "test.ts", source, None, None); + assert!(result.has_errors()); + let code = squash(&result.code); + assert!(code.contains(r#"const_c0=["x"];"#), "{code}"); + assert!(code.contains(r#"const_c1=["y"];"#), "{code}"); + assert!(code.contains("i0.ɵɵviewQuery(_c0,5)(_c1,5)"), "{code}"); +} From 2086147f1673543ea39ae19b47789b8829936f2d Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 28 Oct 2026 16:11:29 +0800 Subject: [PATCH 02/11] fix(constant-pool): quote strings in literal keys so shared literals don't collide A literal's pool key joined its strings unquoted, so `["a", "b,str:c,str:d"]` and `["a", "b", "c", "d"]` had the same key. With literals now shared across the classes of a file, the second component read the first one's `attrs` constant. Strings are now quoted and escaped in the key. Co-Authored-By: Claude Opus 5.5 --- .../src/pipeline/constant_pool.rs | 3 ++- .../tests/const_pool_long_strings_test.rs | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs b/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs index 558afc7b6..6bc3f040c 100644 --- a/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs +++ b/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs @@ -589,7 +589,8 @@ fn expression_to_key(expr: &OutputExpression<'_>) -> String { LiteralValue::Undefined => "undefined".to_string(), LiteralValue::Boolean(b) => format!("bool:{}", b), LiteralValue::Number(n) => format!("num:{}", n), - LiteralValue::String(s) => format!("str:{}", s), + // Quoted and escaped, so no string can read as several entries. + LiteralValue::String(s) => format!("str:{s:?}"), }, OutputExpression::LiteralArray(arr) => { let entries: std::vec::Vec = diff --git a/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs b/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs index f55ca5d0e..dbac5d0e3 100644 --- a/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs +++ b/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs @@ -180,3 +180,20 @@ export class C { @ViewChild('x') q: any; @ViewChild('y') r: any; } assert!(code.contains(r#"const_c1=["y"];"#), "{code}"); assert!(code.contains("i0.ɵɵviewQuery(_c0,5)(_c1,5)"), "{code}"); } + +#[test] +fn literals_whose_strings_join_alike_stay_apart() { + // `["a", "b,str:c,str:d"]` and `["a", "b", "c", "d"]` are different arrays, so B must not + // read A's `attrs` constant. + let code = squash(&transform( + "import {Component} from '@angular/core'; +@Component({selector: '[a=\"b,str:c,str:d\"]', template: 'x', standalone: true}) +export class A {} +@Component({selector: '[a=b][c=d]', template: 'y', standalone: true}) +export class B {} +", + )); + assert!(code.contains(r#"const_c0=["a","b,str:c,str:d"];"#), "{code}"); + assert!(code.contains(r#"const_c1=["a","b","c","d"];"#), "{code}"); + assert!(code.contains("type:B,selectors:[[\"\",\"a\",\"b\",\"c\",\"d\"]],attrs:_c1"), "{code}"); +} From fec390efbe2194e39c15982f9a73b518b3c4087c Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 29 Oct 2026 11:32:10 +0800 Subject: [PATCH 03/11] fix(constant-pool): share directive query predicates with the rest of the file #657 gave each directive its own query pool, which started empty and was never handed on, so `["x"]` was declared again for every directive and for the components after it. The directive pool now takes the file's pooled literals and passes them on, like a component's, as ngtsc has one pool per file. Co-Authored-By: Claude Opus 5.5 --- .../src/component/transform.rs | 1 + .../src/directive/compiler.rs | 11 ++++++++- .../src/directive/definition.rs | 24 +++++++++++++++---- .../tests/const_pool_long_strings_test.rs | 19 +++++++++++++++ 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 16359c00c..99a50c725 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -4177,6 +4177,7 @@ pub fn transform_angular_file( file_namespace_registry.angular_core_ns().as_str(), &directive_metadata, shared_pool_index, + &mut shared_literals, options.compilation_mode, options.angular_version, ); diff --git a/crates/oxc_angular_compiler/src/directive/compiler.rs b/crates/oxc_angular_compiler/src/directive/compiler.rs index 254a591b5..3a6afc902 100644 --- a/crates/oxc_angular_compiler/src/directive/compiler.rs +++ b/crates/oxc_angular_compiler/src/directive/compiler.rs @@ -29,7 +29,7 @@ use crate::output::ast::{ ReadVarExpr, StmtModifier, }; use crate::parser::expression::BindingParser; -use crate::pipeline::constant_pool::ConstantPool; +use crate::pipeline::constant_pool::{ConstantPool, SharedLiterals}; use crate::pipeline::emit::{ HostBindingCompilationResult, compile_host_bindings, emit_pooled_constant_value, }; @@ -72,16 +72,21 @@ pub fn compile_directive<'a>( core_namespace, metadata, pool_starting_index, + &mut SharedLiterals::default(), angular_version, ) } /// Internal implementation of directive compilation. +/// +/// `shared_literals` holds the literals pooled by the earlier classes of the file. The +/// directive's query predicates reuse them and add to them, as ngtsc has one pool per file. pub fn compile_directive_from_metadata<'a>( allocator: &'a Allocator, core_namespace: &'a str, metadata: &R3DirectiveMetadata<'a>, pool_starting_index: u32, + shared_literals: &mut SharedLiterals<'a>, angular_version: Option, ) -> DirectiveCompileResult<'a> { // Build the base directive fields, passing pool_starting_index for host bindings @@ -90,6 +95,7 @@ pub fn compile_directive_from_metadata<'a>( core_namespace, metadata, pool_starting_index, + shared_literals, angular_version, ); @@ -121,6 +127,7 @@ fn build_base_directive_fields<'a>( core_namespace: &'a str, metadata: &R3DirectiveMetadata<'a>, pool_starting_index: u32, + shared_literals: &mut SharedLiterals<'a>, angular_version: Option, ) -> (Vec<'a, LiteralMapEntry<'a>>, u32, oxc_allocator::Vec<'a, OutputStatement<'a>>) { let mut entries = Vec::new_in(&allocator); @@ -143,6 +150,7 @@ fn build_base_directive_fields<'a>( // Query predicates go to the constant pool, content queries first, like // `baseDirectiveFields` in Angular's `render3/view/compiler.ts`. let mut query_pool = ConstantPool::with_starting_index(allocator, pool_starting_index); + query_pool.inherit_literals(std::mem::take(shared_literals)); // contentQueries: (rf, ctx, dirIndex) => { ... } if !metadata.queries.is_empty() { @@ -177,6 +185,7 @@ fn build_base_directive_fields<'a>( // Emit the pooled predicates before the host binding constants, which continue // from the query pool's next index so `_cN` names don't clash. let mut next_pool_index = query_pool.next_name_index(); + *shared_literals = query_pool.take_literals(); for constant in query_pool.constants_mut() { let value = emit_pooled_constant_value(allocator, &mut constant.kind); host_declarations.push(OutputStatement::DeclareVar(Box::new_in( diff --git a/crates/oxc_angular_compiler/src/directive/definition.rs b/crates/oxc_angular_compiler/src/directive/definition.rs index 1e3486342..ccd9c0651 100644 --- a/crates/oxc_angular_compiler/src/directive/definition.rs +++ b/crates/oxc_angular_compiler/src/directive/definition.rs @@ -14,7 +14,7 @@ use oxc_allocator::{Allocator, Vec}; -use super::compiler::compile_directive; +use super::compiler::compile_directive_from_metadata; use super::metadata::R3DirectiveMetadata; use crate::CompilationMode; use crate::factory::{ @@ -25,6 +25,7 @@ use crate::output::ast::{OutputExpression, OutputStatement}; use crate::partial::directive::{ compile_declare_directive_from_metadata, compile_declare_factory_for_directive, }; +use crate::pipeline::constant_pool::SharedLiterals; /// Result of generating directive definitions. pub struct DirectiveDefinitions<'a> { @@ -80,6 +81,7 @@ pub fn generate_directive_definitions<'a>( core_namespace: &'a str, metadata: &R3DirectiveMetadata<'a>, pool_starting_index: u32, + shared_literals: &mut SharedLiterals<'a>, compilation_mode: CompilationMode, angular_version: Option, ) -> DirectiveDefinitions<'a> { @@ -96,6 +98,7 @@ pub fn generate_directive_definitions<'a>( core_namespace, metadata, pool_starting_index, + shared_literals, angular_version, ); DirectiveDefinitions { dir_definition, fac_definition, next_pool_index, statements } @@ -122,7 +125,7 @@ pub fn generate_directive_definitions<'a>( /// Generate the ɵdir definition. /// -/// This calls `compile_directive()` which creates an expression like: +/// This calls `compile_directive_from_metadata()` which creates an expression like: /// ```javascript /// /*@__PURE__*/ i0.ɵɵdefineDirective({ /// type: MyDirective, @@ -140,13 +143,15 @@ fn generate_dir_definition<'a>( core_namespace: &'a str, metadata: &R3DirectiveMetadata<'a>, pool_starting_index: u32, + shared_literals: &mut SharedLiterals<'a>, angular_version: Option, ) -> (OutputExpression<'a>, u32, oxc_allocator::Vec<'a, OutputStatement<'a>>) { - let result = compile_directive( + let result = compile_directive_from_metadata( allocator, core_namespace, metadata, pool_starting_index, + shared_literals, angular_version, ); (result.expression, result.next_pool_index, result.statements) @@ -277,6 +282,7 @@ mod tests { "i0", &metadata, 0, + &mut SharedLiterals::default(), CompilationMode::Full, None, ); @@ -449,8 +455,14 @@ mod tests { let allocator = Allocator::default(); let metadata = create_test_metadata(&allocator); - let (dir, _next_pool_index, _statements) = - generate_dir_definition(&allocator, "i0", &metadata, 0, None); + let (dir, _next_pool_index, _statements) = generate_dir_definition( + &allocator, + "i0", + &metadata, + 0, + &mut SharedLiterals::default(), + None, + ); let emitter = JsEmitter::new(); let js = emitter.emit_expression(&dir); @@ -505,6 +517,7 @@ mod tests { "i0", &metadata1, 0, + &mut SharedLiterals::default(), CompilationMode::Full, None, ); @@ -549,6 +562,7 @@ mod tests { "i0", &metadata2, next_index, + &mut SharedLiterals::default(), CompilationMode::Full, None, ); diff --git a/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs b/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs index dbac5d0e3..1c8822b44 100644 --- a/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs +++ b/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs @@ -197,3 +197,22 @@ export class B {} assert!(code.contains(r#"const_c1=["a","b","c","d"];"#), "{code}"); assert!(code.contains("type:B,selectors:[[\"\",\"a\",\"b\",\"c\",\"d\"]],attrs:_c1"), "{code}"); } + +#[test] +fn directive_query_predicates_are_shared_with_the_other_classes() { + // ngtsc's one pool per file gives the directive and both components the same `["x"]`. + let code = squash(&transform( + "import {Component, Directive, ViewChild, ContentChild} from '@angular/core'; +@Component({selector: 'a', template: '', standalone: true}) +export class A { @ViewChild('x') q: any; } +@Directive({selector: '[d]', standalone: true}) +export class D { @ContentChild('x') q: any; @ViewChild('y') r: any; } +@Component({selector: 'b', template: '', standalone: true}) +export class B { @ViewChild('y') q: any; } +", + )); + assert_eq!(code.matches(r#"=["x"];"#).count(), 1, "{code}"); + assert_eq!(code.matches(r#"=["y"];"#).count(), 1, "{code}"); + assert!(code.contains(r#"const_c0=["x"];"#), "{code}"); + assert!(code.contains("i0.ɵɵcontentQuery(dirIndex,_c0,5)"), "{code}"); +} From 0d67279fd923bc4d9eebcaf2dfea9408dee1ad5a Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 29 Oct 2026 11:42:24 +0800 Subject: [PATCH 04/11] fix(constant-pool): hand claimed view function names on to the next class `$A` and `_A` (or `A_` and `A$`) sanitize to the same prefix, so two classes of a file could both declare `_A_Conditional_0_Template` at the top level, a SyntaxError. ngtsc's one pool per file claims names across classes and numbers the second one `_A_Conditional_0_Template1`. The pooled literals handed from class to class (now `SharedPool`) carry the class-specific names each pool claimed, so the next pool numbers its own the same way. `_forTrack` and `arrowFn` keep the file offset. Co-Authored-By: Claude Opus 5.5 --- .../src/component/transform.rs | 20 +++---- .../src/directive/compiler.rs | 18 +++--- .../src/directive/definition.rs | 18 +++--- .../src/pipeline/constant_pool.rs | 57 ++++++++++++------- .../tests/const_pool_long_strings_test.rs | 17 ++++++ 5 files changed, 83 insertions(+), 47 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 99a50c725..4648167ed 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -69,7 +69,7 @@ use crate::pipe::{ }; use crate::pipeline::compilation::{DeferBlockDepsEmitMode, TemplateCompilationMode}; use crate::pipeline::constant_pool::{ - ConstantPool, InheritedFixup, PooledConstantKind, SharedLiterals, is_long_string_literal, + ConstantPool, InheritedFixup, PooledConstantKind, SharedPool, is_long_string_literal, }; use crate::pipeline::emit::{ HostBindingCompilationResult, compile_host_bindings, compile_template, @@ -3276,7 +3276,7 @@ pub fn transform_angular_file( // Likewise, the literals the file's components pooled so far, and the // components whose ɵcmp has a long style inline that a later component // may share: (class name, ɵcmp, its emitted text). - let mut shared_literals = SharedLiterals::default(); + let mut shared_pool = SharedPool::default(); let mut inline_style_cmps: Vec<(String, OutputExpression<'_>, String)> = Vec::new(); // The components compiled for HMR, whose inline template and styles the @@ -3747,7 +3747,7 @@ pub fn transform_angular_file( view_queries, content_queries, shared_pool_index, - &mut shared_literals, + &mut shared_pool, &mut file_namespace_registry, &import_map, resolved_resources, @@ -4177,7 +4177,7 @@ pub fn transform_angular_file( file_namespace_registry.angular_core_ns().as_str(), &directive_metadata, shared_pool_index, - &mut shared_literals, + &mut shared_pool, options.compilation_mode, options.angular_version, ); @@ -5411,7 +5411,7 @@ pub fn compile_component_hmr_update( view_queries, content_queries, 0, - &mut SharedLiterals::default(), + &mut SharedPool::default(), &mut namespace_registry, &import_map, Some(resources), @@ -5629,7 +5629,7 @@ fn compile_component_full<'a>( view_queries: OxcVec<'a, R3QueryMetadata<'a>>, content_queries: OxcVec<'a, R3QueryMetadata<'a>>, pool_starting_index: u32, - shared_literals: &mut SharedLiterals<'a>, + shared_pool: &mut SharedPool<'a>, namespace_registry: &mut NamespaceRegistry<'a>, import_map: &ImportMap<'a>, resolved_resources: Option<&ResolvedResources>, @@ -6013,10 +6013,10 @@ fn compile_component_full<'a>( ingest_options, ); job.core_namespace = namespace_registry.angular_core_ns(); - // Reuse the literals the file's earlier components pooled: ngtsc compiles - // the whole file with one constant pool. They are handed on only once this + // Reuse the literals the file's earlier classes pooled, and avoid the names + // they claimed: ngtsc compiles the whole file with one constant pool. They are handed on only once this // component compiled, as a failed one does not move `shared_pool_index`. - job.pool.inherit_literals(shared_literals.clone()); + job.pool.inherit_shared(shared_pool.clone()); // BEFORE template compilation: Pool attrs const if first selector has attributes. // This matches TypeScript Angular which adds attrs to the pool BEFORE template ingestion. @@ -6181,7 +6181,7 @@ fn compile_component_full<'a>( } else { None }; - *shared_literals = job.pool.take_literals(); + *shared_pool = job.pool.take_shared(); let cmp_js = emitter.emit_expression(&definitions.cmp_definition); let fac_js = emitter.emit_expression(&definitions.fac_definition); diff --git a/crates/oxc_angular_compiler/src/directive/compiler.rs b/crates/oxc_angular_compiler/src/directive/compiler.rs index 3a6afc902..2e72c48ce 100644 --- a/crates/oxc_angular_compiler/src/directive/compiler.rs +++ b/crates/oxc_angular_compiler/src/directive/compiler.rs @@ -29,7 +29,7 @@ use crate::output::ast::{ ReadVarExpr, StmtModifier, }; use crate::parser::expression::BindingParser; -use crate::pipeline::constant_pool::{ConstantPool, SharedLiterals}; +use crate::pipeline::constant_pool::{ConstantPool, SharedPool}; use crate::pipeline::emit::{ HostBindingCompilationResult, compile_host_bindings, emit_pooled_constant_value, }; @@ -72,21 +72,21 @@ pub fn compile_directive<'a>( core_namespace, metadata, pool_starting_index, - &mut SharedLiterals::default(), + &mut SharedPool::default(), angular_version, ) } /// Internal implementation of directive compilation. /// -/// `shared_literals` holds the literals pooled by the earlier classes of the file. The -/// directive's query predicates reuse them and add to them, as ngtsc has one pool per file. +/// `shared_pool` holds what the earlier classes of the file pooled. The directive's query +/// predicates reuse those literals and add to them, as ngtsc has one pool per file. pub fn compile_directive_from_metadata<'a>( allocator: &'a Allocator, core_namespace: &'a str, metadata: &R3DirectiveMetadata<'a>, pool_starting_index: u32, - shared_literals: &mut SharedLiterals<'a>, + shared_pool: &mut SharedPool<'a>, angular_version: Option, ) -> DirectiveCompileResult<'a> { // Build the base directive fields, passing pool_starting_index for host bindings @@ -95,7 +95,7 @@ pub fn compile_directive_from_metadata<'a>( core_namespace, metadata, pool_starting_index, - shared_literals, + shared_pool, angular_version, ); @@ -127,7 +127,7 @@ fn build_base_directive_fields<'a>( core_namespace: &'a str, metadata: &R3DirectiveMetadata<'a>, pool_starting_index: u32, - shared_literals: &mut SharedLiterals<'a>, + shared_pool: &mut SharedPool<'a>, angular_version: Option, ) -> (Vec<'a, LiteralMapEntry<'a>>, u32, oxc_allocator::Vec<'a, OutputStatement<'a>>) { let mut entries = Vec::new_in(&allocator); @@ -150,7 +150,7 @@ fn build_base_directive_fields<'a>( // Query predicates go to the constant pool, content queries first, like // `baseDirectiveFields` in Angular's `render3/view/compiler.ts`. let mut query_pool = ConstantPool::with_starting_index(allocator, pool_starting_index); - query_pool.inherit_literals(std::mem::take(shared_literals)); + query_pool.inherit_shared(std::mem::take(shared_pool)); // contentQueries: (rf, ctx, dirIndex) => { ... } if !metadata.queries.is_empty() { @@ -185,7 +185,7 @@ fn build_base_directive_fields<'a>( // Emit the pooled predicates before the host binding constants, which continue // from the query pool's next index so `_cN` names don't clash. let mut next_pool_index = query_pool.next_name_index(); - *shared_literals = query_pool.take_literals(); + *shared_pool = query_pool.take_shared(); for constant in query_pool.constants_mut() { let value = emit_pooled_constant_value(allocator, &mut constant.kind); host_declarations.push(OutputStatement::DeclareVar(Box::new_in( diff --git a/crates/oxc_angular_compiler/src/directive/definition.rs b/crates/oxc_angular_compiler/src/directive/definition.rs index ccd9c0651..ff682b244 100644 --- a/crates/oxc_angular_compiler/src/directive/definition.rs +++ b/crates/oxc_angular_compiler/src/directive/definition.rs @@ -25,7 +25,7 @@ use crate::output::ast::{OutputExpression, OutputStatement}; use crate::partial::directive::{ compile_declare_directive_from_metadata, compile_declare_factory_for_directive, }; -use crate::pipeline::constant_pool::SharedLiterals; +use crate::pipeline::constant_pool::SharedPool; /// Result of generating directive definitions. pub struct DirectiveDefinitions<'a> { @@ -81,7 +81,7 @@ pub fn generate_directive_definitions<'a>( core_namespace: &'a str, metadata: &R3DirectiveMetadata<'a>, pool_starting_index: u32, - shared_literals: &mut SharedLiterals<'a>, + shared_pool: &mut SharedPool<'a>, compilation_mode: CompilationMode, angular_version: Option, ) -> DirectiveDefinitions<'a> { @@ -98,7 +98,7 @@ pub fn generate_directive_definitions<'a>( core_namespace, metadata, pool_starting_index, - shared_literals, + shared_pool, angular_version, ); DirectiveDefinitions { dir_definition, fac_definition, next_pool_index, statements } @@ -143,7 +143,7 @@ fn generate_dir_definition<'a>( core_namespace: &'a str, metadata: &R3DirectiveMetadata<'a>, pool_starting_index: u32, - shared_literals: &mut SharedLiterals<'a>, + shared_pool: &mut SharedPool<'a>, angular_version: Option, ) -> (OutputExpression<'a>, u32, oxc_allocator::Vec<'a, OutputStatement<'a>>) { let result = compile_directive_from_metadata( @@ -151,7 +151,7 @@ fn generate_dir_definition<'a>( core_namespace, metadata, pool_starting_index, - shared_literals, + shared_pool, angular_version, ); (result.expression, result.next_pool_index, result.statements) @@ -282,7 +282,7 @@ mod tests { "i0", &metadata, 0, - &mut SharedLiterals::default(), + &mut SharedPool::default(), CompilationMode::Full, None, ); @@ -460,7 +460,7 @@ mod tests { "i0", &metadata, 0, - &mut SharedLiterals::default(), + &mut SharedPool::default(), None, ); @@ -517,7 +517,7 @@ mod tests { "i0", &metadata1, 0, - &mut SharedLiterals::default(), + &mut SharedPool::default(), CompilationMode::Full, None, ); @@ -562,7 +562,7 @@ mod tests { "i0", &metadata2, next_index, - &mut SharedLiterals::default(), + &mut SharedPool::default(), CompilationMode::Full, None, ); diff --git a/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs b/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs index 0c429f50d..6259b76ee 100644 --- a/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs +++ b/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs @@ -21,15 +21,23 @@ use crate::output::ast::{ /// Ported from Angular's `POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS`. const POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS: usize = 50; -/// The literals pooled so far in a file, handed from one class's pool to the -/// next. +/// What one class's pool hands to the next class's pool in a file. /// /// ngtsc compiles every class of a file with one `ConstantPool`, so a literal -/// pooled by one class is reused by the next. Oxc gives each class its own -/// pool (with names chained by the starting index), so the file transform -/// carries the literals over with [`ConstantPool::take_literals`] and -/// [`ConstantPool::inherit_literals`]. -pub type SharedLiterals<'a> = FxHashMap>; +/// pooled by one class is reused by the next, and a name one class claims is +/// not claimed again by another. Oxc gives each class its own pool (with `_c` +/// names chained by the starting index), so the file transform carries these +/// over with [`ConstantPool::take_shared`] and [`ConstantPool::inherit_shared`]. +#[derive(Debug, Default, Clone)] +pub struct SharedPool<'a> { + /// Literals pooled by `get_const_literal`, by key. + literals: FxHashMap>, + /// Names claimed by `unique_name` that are not numbered by the file + /// offset (`Cmp_Template`, `Cmp_Conditional_0_Template`, ...). Two classes + /// can sanitize to the same prefix (`$A` and `_A`), and their view + /// functions are declared side by side. + claimed_names: FxHashMap, +} /// A pool for deduplicating constant values. /// @@ -50,11 +58,12 @@ pub struct ConstantPool<'a> { /// This is used to ensure unique names don't conflict when compiling /// multiple components in the same file. unique_name_offset: u32, - /// Claimed names for unique_name deduplication within a single component. - /// Maps base name to usage count. + /// Claimed names for unique_name deduplication, including the + /// class-specific names of the earlier classes of the file. Maps base name + /// to usage count. claimed_names: FxHashMap, /// Literals pooled by `get_const_literal`, by key. - literals: SharedLiterals<'a>, + literals: FxHashMap>, /// Long strings an earlier class of the file used inline once, shared by /// this pool. Their declaration and first use belong to that earlier class. inherited_fixups: std::vec::Vec>, @@ -435,8 +444,9 @@ impl<'a> ConstantPool<'a> { /// "generic" names like `_forTrack` that don't include a component name. /// /// For component-specific names (like `ComponentName_Template`), the offset - /// is NOT applied because these names are already unique by virtue of - /// containing the component class name. + /// is NOT applied: they contain the class name, and the names claimed by + /// the earlier classes of the file come in through + /// [`ConstantPool::inherit_shared`]. /// /// # Arguments /// @@ -528,18 +538,27 @@ impl<'a> ConstantPool<'a> { Some(self.read_var(name)) } - /// Takes the literals this pool and the ones before it pooled, to hand - /// them to the next class's pool. - pub fn take_literals(&mut self) -> SharedLiterals<'a> { - std::mem::take(&mut self.literals) + /// Takes the literals and class-specific names this pool and the ones + /// before it pooled and claimed, to hand them to the next class's pool. + pub fn take_shared(&mut self) -> SharedPool<'a> { + let claimed_names = self + .claimed_names + .iter() + .filter(|(base, _)| !is_generic_name(base)) + .map(|(base, count)| (base.clone(), *count)) + .collect(); + SharedPool { literals: std::mem::take(&mut self.literals), claimed_names } } - /// Reuses the literals pooled by the earlier classes of the file. - pub fn inherit_literals(&mut self, mut literals: SharedLiterals<'a>) { + /// Reuses the literals pooled, and avoids the names claimed, by the + /// earlier classes of the file. + pub fn inherit_shared(&mut self, shared: SharedPool<'a>) { + let SharedPool { mut literals, claimed_names } = shared; for pooled in literals.values_mut() { pooled.inherited = true; } self.literals = literals; + self.claimed_names.extend(claimed_names); } /// Takes the long strings an earlier class used inline once and this pool @@ -830,7 +849,7 @@ mod tests { first_pool.get_const_literal(string(&allocator, &long), false); let mut pool = ConstantPool::with_starting_index(&allocator, 3); - pool.inherit_literals(first_pool.take_literals()); + pool.inherit_shared(first_pool.take_shared()); let second = pool.get_const_literal(string(&allocator, &long), false); assert_eq!(read_var_name(&second), Some("_c3")); // Declared before the earlier class, not with this pool's constants. diff --git a/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs b/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs index 1c8822b44..8e8f2b318 100644 --- a/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs +++ b/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs @@ -216,3 +216,20 @@ export class B { @ViewChild('y') q: any; } assert!(code.contains(r#"const_c0=["x"];"#), "{code}"); assert!(code.contains("i0.ɵɵcontentQuery(dirIndex,_c0,5)"), "{code}"); } + +#[test] +fn view_functions_of_classes_with_the_same_sanitized_name_stay_apart() { + // `$A` and `_A` both sanitize to `_A`. ngtsc's one pool per file numbers the second + // class's view functions, so the top-level declarations don't clash. + let code = squash(&transform( + "import {Component} from '@angular/core'; +@Component({selector: 'a', template: '@if (x) {}', standalone: true}) +export class $A { x = 1; } +@Component({selector: 'b', template: '@if (x) {}', standalone: true}) +export class _A { x = 1; } +", + )); + assert_eq!(code.matches("function_A_Conditional_0_Template(").count(), 1, "{code}"); + assert_eq!(code.matches("function_A_Conditional_0_Template1(").count(), 1, "{code}"); + assert!(code.contains("template:function_A_Template1("), "{code}"); +} From 4cdce8e09bdea6eb7830742d38573b9d2150e5b3 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 29 Oct 2026 12:40:33 +0800 Subject: [PATCH 05/11] fix(constant-pool): number and share pool names file-wide like ngtsc ngtsc compiles a file with one ConstantPool. Three gaps remained: - `_forTrack`/`arrowFn` were numbered from the file's `_c` index. Angular's `uniqueName` counts each base name on its own and `freshName` counts `_c`, so the offset is gone and every claimed name is handed on. - Equal shared functions (`getSharedFunctionReference`: track, arrow and `@defer` deps fns) and pure functions (`getSharedConstant`) were declared again by each class. The shared pool now carries them, so a later class reuses the const the earlier class declared. A deps fn reads an imported class through the identifier in the component's own `imports` (a `WrappedNodeExpr` from `LocalIdentifierStrategy`), and `isEquivalent` compares nodes, so such a function is shared within one component only. A wrapped node is now compared by name and span and emitted as written. - A component compiled its template before its host bindings. Angular's `compileComponentFromMetadata` runs `baseDirectiveFields` (queries, then `createHostBindingsFunction`) first, then the selector attrs and the template, so host constants take the lower `_c` indices and are declared first. The HMR update now carries the host constants the template reads. The linker does the same: host bindings first, with one pool, so the template reuses their pure and arrow functions. Ported from @angular/compiler 22.2.2 `ConstantPool` (constant_pool.ts), `WrappedNodeExpr.isEquivalent` (output_ast.ts) and `compileComponentFromMetadata` / `baseDirectiveFields` (render3/view/compiler.ts), and compiler-cli 22.2.3 `LocalIdentifierStrategy` / `Reference.getIdentityIn` (ngtsc/imports). Co-Authored-By: Claude Opus 5.5 --- .../src/component/decorator.rs | 7 +- .../src/component/defer_resolver.rs | 24 +- .../src/component/metadata.rs | 5 +- .../src/component/transform.rs | 141 +++++----- .../src/directive/compiler.rs | 7 +- crates/oxc_angular_compiler/src/linker/mod.rs | 51 ++-- crates/oxc_angular_compiler/src/output/ast.rs | 6 +- .../src/output/emitter.rs | 17 +- .../src/pipeline/constant_pool.rs | 176 +++++++------ .../phases/pure_function_extraction.rs | 3 +- .../tests/const_pool_file_wide_test.rs | 242 ++++++++++++++++++ .../tests/defer_implicit_imports_test.rs | 52 +++- .../tests/integration_test.rs | 15 +- 13 files changed, 540 insertions(+), 206 deletions(-) create mode 100644 crates/oxc_angular_compiler/tests/const_pool_file_wide_test.rs diff --git a/crates/oxc_angular_compiler/src/component/decorator.rs b/crates/oxc_angular_compiler/src/component/decorator.rs index cafe312ca..3722ee90c 100644 --- a/crates/oxc_angular_compiler/src/component/decorator.rs +++ b/crates/oxc_angular_compiler/src/component/decorator.rs @@ -492,18 +492,19 @@ fn extract_identifier_array<'a>( } /// The class each element of an array literal names, by position: `X`, or -/// `forwardRef(() => X)` with `@angular/core`'s `forwardRef`. +/// `forwardRef(() => X)` with `@angular/core`'s `forwardRef`, with the span of +/// the identifier. fn extract_import_entry_classes<'a>( allocator: &'a Allocator, expr: &'a Expression<'a>, consts: &StringConsts<'_>, -) -> Vec<'a, Option>> { +) -> Vec<'a, Option<(Ident<'a>, Span)>> { let mut result = Vec::new_in(&allocator); if let Expression::ArrayExpression(arr) = expr { for element in &arr.elements { let class = element.as_expression().and_then(|element| { match try_unwrap_forward_ref(element, Some(consts)).unwrap_or(element) { - Expression::Identifier(id) => Some(id.name), + Expression::Identifier(id) => Some((id.name, id.span)), _ => None, } }); diff --git a/crates/oxc_angular_compiler/src/component/defer_resolver.rs b/crates/oxc_angular_compiler/src/component/defer_resolver.rs index 3801879ad..9195b5835 100644 --- a/crates/oxc_angular_compiler/src/component/defer_resolver.rs +++ b/crates/oxc_angular_compiler/src/component/defer_resolver.rs @@ -52,6 +52,7 @@ use crate::class_metadata::{ use crate::i18n::serializer::is_i18n_attribute; use crate::output::ast::{ ArrowFunctionBody, ArrowFunctionExpr, LiteralArrayExpr, OutputExpression, ReadVarExpr, + WrappedNodeExpr, }; use crate::parser::html::split_ns_name; use crate::pipeline::selector::CssSelector; @@ -286,10 +287,19 @@ pub(super) fn defer_block_dependencies<'a>( /// `dependencies` that `deferred` lists, and a plain reference to the others. /// `None` when the block has no dependencies (`compileDeferBlocks` in ngc's /// component `handler.ts`). +/// +/// ngc's plain reference is the `WrappedNodeExpr` its `LocalIdentifierStrategy` +/// emits (`Reference.getIdentityIn`): the name of a class declared in the +/// file, which is the same node for every component and stays a plain read +/// here, or for an imported class the identifier in this component's +/// `imports`, found in `import_entry_classes`. Wrapped nodes are equal only +/// when they are the same node, so the constant pool shares a function that +/// reads an imported class within this component only. pub(super) fn compile_defer_block_resolver<'a>( allocator: &'a Allocator, dependencies: &[Ident<'a>], deferred: &[Ident<'a>], + import_entry_classes: &[Option<(Ident<'a>, oxc_span::Span)>], import_map: &ImportMap<'a>, ) -> Option> { if dependencies.is_empty() { @@ -304,11 +314,19 @@ pub(super) fn compile_defer_block_resolver<'a>( } else { None }; - entries.push(deferrable.unwrap_or_else(|| { - OutputExpression::ReadVar(oxc_allocator::Box::new_in( + let imports_identifier = import_entry_classes + .iter() + .flatten() + .find(|(class, _)| class == name && import_map.contains_key(name)); + entries.push(deferrable.unwrap_or_else(|| match imports_identifier { + Some((_, span)) => OutputExpression::WrappedNode(oxc_allocator::Box::new_in( + WrappedNodeExpr { node_id: *name, source_span: Some(*span) }, + &allocator, + )), + None => OutputExpression::ReadVar(oxc_allocator::Box::new_in( ReadVarExpr { name: *name, source_span: None }, &allocator, - )) + )), })); } let array = OutputExpression::LiteralArray(oxc_allocator::Box::new_in( diff --git a/crates/oxc_angular_compiler/src/component/metadata.rs b/crates/oxc_angular_compiler/src/component/metadata.rs index 1e1d60453..46617b4f9 100644 --- a/crates/oxc_angular_compiler/src/component/metadata.rs +++ b/crates/oxc_angular_compiler/src/component/metadata.rs @@ -530,8 +530,9 @@ pub struct ComponentMetadata<'a> { /// array literal, by position: `X`, or `forwardRef(() => X)` with /// `@angular/core`'s `forwardRef` (which [`Self::imports`] leaves out), /// as ngtsc resolves them through its `forwardRefResolver`; `None` for - /// anything else, such as `X as any`, which ngtsc keeps eager. - pub import_entry_classes: Vec<'a, Option>>, + /// anything else, such as `X as any`, which ngtsc keeps eager. Each class + /// comes with the span of its identifier in the array. + pub import_entry_classes: Vec<'a, Option<(Ident<'a>, Span)>>, /// Exported names for template references. /// diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 4648167ed..3eb2e4e98 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -5827,7 +5827,7 @@ fn compile_component_full<'a>( .import_entry_classes .iter() .flatten() - .copied() + .map(|(name, _)| *name) .filter_map(|name| { let (selector, can_defer) = import_selector(name.as_str())?; if can_defer { @@ -5887,7 +5887,7 @@ fn compile_component_full<'a>( if let Some(OutputExpression::LiteralArray(raw_imports)) = &mut metadata.raw_imports { let mut classes = metadata.import_entry_classes.iter(); raw_imports.entries.retain(|_| { - !classes.next().copied().flatten().is_some_and(|name| is_removed(&name)) + !classes.next().copied().flatten().is_some_and(|(name, _)| is_removed(&name)) }); // Nothing left to resolve at runtime: like a standalone // component without `imports`, it gets no `dependencies`. @@ -5993,6 +5993,7 @@ fn compile_component_full<'a>( allocator, &dependencies, &metadata.deferred_imports, + &metadata.import_entry_classes, import_map, ); (span, deps_fn) @@ -6018,15 +6019,10 @@ fn compile_component_full<'a>( // component compiled, as a failed one does not move `shared_pool_index`. job.pool.inherit_shared(shared_pool.clone()); - // BEFORE template compilation: Pool attrs const if first selector has attributes. - // This matches TypeScript Angular which adds attrs to the pool BEFORE template ingestion. - // By pooling attrs first, it gets _c0 (or the first available index). - // See: packages/compiler/src/render3/view/compiler.ts lines 192-212 - let attrs_ref = pool_selector_attrs(allocator, &mut job, metadata); - - // BEFORE template compilation: Pool query predicates so they - // appear in the constant table as `attrs -> query predicates -> - // pure functions`. + // BEFORE template compilation: Pool query predicates, then the host + // bindings' constants, then the selector attrs, as Angular's + // `compileComponentFromMetadata` runs `baseDirectiveFields` before it + // pools the attrs and ingests the template. // // Order matters: upstream ngtsc pools **content queries first, then // view queries** (compiler/src/render3/view/compiler.ts emits @@ -6063,25 +6059,21 @@ fn compile_component_full<'a>( None }; - let compiled = compile_template(&mut job); - - if !job.emit_errors.is_empty() { - diagnostics.append(&mut job.emit_errors); - return Err(diagnostics); - } - - // Stage 6: Compile host bindings (if any) - // Pass the template pool's current index to ensure host binding constants - // continue from where template compilation left off (avoiding duplicate names) - let template_pool_index = job.pool.next_name_index(); + // Compile host bindings (if any) in `baseDirectiveFields`, after the + // queries. Their pool continues from the query predicates, and the + // template's pool from theirs, as both are one pool in Angular. + let query_constant_count = job.pool.len(); + let mut component_pool = job.pool.take_shared(); let host_binding_output = compile_component_host_bindings( &allocator, job.core_namespace.as_str(), metadata, - template_pool_index, + job.pool.next_name_index(), + &mut component_pool, options.angular_version, options.legacy_optional_chaining, ); + job.pool.inherit_shared(component_pool); // Extract the result and update pool index if host bindings were compiled let (host_binding_result, host_binding_next_pool_index, host_binding_declarations) = @@ -6098,15 +6090,27 @@ fn compile_component_full<'a>( } None => (None, None, OxcVec::new_in(&allocator)), }; + if let Some(index) = host_binding_next_pool_index { + job.pool.continue_from(index); + } + + // Pool attrs const if first selector has attributes, after the host + // bindings and before the template. + // See: packages/compiler/src/render3/view/compiler.ts lines 192-212 + let attrs_ref = pool_selector_attrs(allocator, &mut job, metadata); + + let compiled = compile_template(&mut job); + + if !job.emit_errors.is_empty() { + diagnostics.append(&mut job.emit_errors); + return Err(diagnostics); + } // Stage 7: Generate ɵcmp/ɵfac definitions // The namespace registry is shared across all components in the file to ensure // consistent namespace assignments between factory generation and import statements. // Pass the pre-pooled attrs_ref so the attrs entry uses the correct constant index - // The styles pool their long strings after the host bindings' constants. - if let Some(index) = host_binding_next_pool_index { - job.pool.continue_from(index); - } + // The styles pool their long strings after the template's constants. let template_constant_count = job.pool.len(); let definitions = generate_component_definitions( &allocator, @@ -6124,17 +6128,23 @@ fn compile_component_full<'a>( // Emit JavaScript let emitter = JsEmitter::new(); - // Emit declarations (child view functions, constants) + // Emit declarations in the order they were pooled: the query predicates + // (the template pool's first constants), the host bindings' constants, + // then the template's (constants, child view functions). let mut declarations_js = String::new(); - for decl in compiled.declarations.iter() { + let (query_declarations, template_declarations) = + compiled.declarations.split_at(query_constant_count); + for decl in + query_declarations.iter().chain(&host_binding_declarations).chain(template_declarations) + { declarations_js.push_str(&emitter.emit_statement(decl)); declarations_js.push('\n'); } // An HMR update replaces the template fields of the live definition, so // it takes them from the definition just built: the same text the full - // transform emits. Only the template's declarations go along; the host - // binding's belong to fields the update keeps. + // transform emits. The host bindings' declarations go along too, as the + // template may share their constants. let hmr_update = hmr_update.then(|| HmrUpdateParts { template_js: cmp_definition_field(&definitions.cmp_definition, "template") .map(|template| emitter.emit_expression(template)) @@ -6152,12 +6162,6 @@ fn compile_component_full<'a>( styles: cmp_definition_styles(&definitions.cmp_definition, &job.pool), }); - // 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'); - } - // Emit the long styles this component shared (the pool's last constants) for decl in &declare_pooled_constants_from(&mut job.pool, template_constant_count) { declarations_js.push_str(&emitter.emit_statement(decl)); @@ -6254,8 +6258,6 @@ fn compile_component_full<'a>( }; // Get the next pool index for the next component in this file - // (the template pool continued from the host binding pool's next index - // before the styles were pooled) let next_pool_index = job.pool.next_name_index(); // Collect any diagnostics from the compilation job @@ -6741,13 +6743,14 @@ pub fn compile_template_to_js_with_options<'a>( // Stage 6: Compile host bindings if provided via options let host_pool_starting_index = job.pool.next_name_index(); if let Some(ref host_input) = options.host { - if let Some(host_result) = compile_host_bindings_from_input( + if let Some((host_result, _)) = compile_host_bindings_from_input( &allocator, crate::r3::CORE_ALIAS, host_input, component_name, options.selector.as_deref(), host_pool_starting_index, + &mut job.pool.take_shared(), options.angular_version, options.legacy_optional_chaining, ) { @@ -7052,9 +7055,9 @@ struct HostBindingCompilationOutput<'a> { /// into a compiled host binding result (function, hostAttrs, hostVars). /// /// The `pool_starting_index` parameter is used to ensure constant names don't conflict -/// with those already used by template compilation. In Angular TypeScript, both template -/// and host binding compilation share the same ConstantPool instance. In our implementation, -/// we pass the template pool's next index to achieve the same effect. +/// with those already used by the query predicates. In Angular TypeScript, the queries, +/// host bindings and template share the same ConstantPool instance. In our implementation, +/// we pass the component pool's next index and its `shared_pool` to achieve the same effect. /// /// Returns None if the component has no host bindings. fn compile_component_host_bindings<'a>( @@ -7062,6 +7065,7 @@ fn compile_component_host_bindings<'a>( core_namespace: &'a str, metadata: &ComponentMetadata<'a>, pool_starting_index: u32, + shared_pool: &mut SharedPool<'a>, angular_version: Option, legacy_optional_chaining: Option, ) -> Option> { @@ -7086,7 +7090,7 @@ fn compile_component_host_bindings<'a>( let input = convert_host_metadata_to_input(allocator, host, component_name, component_selector); // Ingest and compile the host bindings with the pool starting index - // This ensures constant names continue from where template compilation left off + // This ensures constant names continue from where the queries left off let mut job = ingest_host_binding_with_version( &allocator, input, @@ -7095,7 +7099,9 @@ fn compile_component_host_bindings<'a>( legacy_optional_chaining, ); job.core_namespace = Ident::from(core_namespace); + job.pool.inherit_shared(std::mem::take(shared_pool)); let result = compile_host_bindings(&mut job); + *shared_pool = job.pool.take_shared(); // Get the next pool index after host binding compilation let next_pool_index = job.pool.next_name_index(); @@ -7406,7 +7412,9 @@ fn pool_selector_attrs<'a>( /// Compile host bindings from HostMetadataInput (owned strings). /// /// This is used by `compile_template_to_js_with_options` when host metadata is provided -/// via TransformOptions for isolated template compilation. +/// via TransformOptions for isolated template compilation, and by the linker. +/// +/// Returns a tuple of (result, next_pool_index). fn compile_host_bindings_from_input<'a>( allocator: &'a Allocator, core_namespace: &'a str, @@ -7414,9 +7422,10 @@ fn compile_host_bindings_from_input<'a>( component_name: &str, selector: Option<&str>, pool_starting_index: u32, + shared_pool: &mut SharedPool<'a>, angular_version: Option, legacy_optional_chaining: Option, -) -> Option> { +) -> Option<(HostBindingCompilationResult<'a>, u32)> { use oxc_allocator::FromIn; // Check if there are any host bindings at all @@ -7449,9 +7458,11 @@ fn compile_host_bindings_from_input<'a>( legacy_optional_chaining, ); job.core_namespace = Ident::from(core_namespace); + job.pool.inherit_shared(std::mem::take(shared_pool)); let result = compile_host_bindings(&mut job); + *shared_pool = job.pool.take_shared(); - Some(result) + Some((result, job.pool.next_name_index())) } /// Result of compiling host bindings for the linker. @@ -7462,6 +7473,9 @@ pub struct LinkerHostBindingOutput { pub host_vars: u32, /// Pool constant declarations (pure functions, etc.) as JS. pub declarations_js: String, + /// The next available pool index after host binding compilation. + /// Used to continue constant numbering in template compilation. + pub next_pool_index: u32, } /// Compile host bindings for the linker, returning the emitted JS function + hostVars count. @@ -7469,23 +7483,27 @@ pub struct LinkerHostBindingOutput { /// This takes host property/listener data extracted from a partial declaration and compiles /// it through the full Angular expression parser and host binding pipeline, producing /// correctly compiled output (unlike raw string interpolation which would fail for complex -/// Angular template expressions). -pub fn compile_host_bindings_for_linker( +/// Angular template expressions). `shared_pool` carries what the host bindings pool to the +/// component's template (see [`compile_template_for_linker`]), which Angular compiles after +/// them with the same pool. +pub fn compile_host_bindings_for_linker<'a>( + allocator: &'a Allocator, host_input: &HostMetadataInput, component_name: &str, selector: Option<&str>, pool_starting_index: u32, + shared_pool: &mut SharedPool<'a>, core_namespace: &str, angular_version: Option, ) -> Option { - let allocator = Allocator::default(); - let result = compile_host_bindings_from_input( - &allocator, + let (result, next_pool_index) = compile_host_bindings_from_input( + allocator, allocator.alloc_str(core_namespace), host_input, component_name, selector, pool_starting_index, + shared_pool, angular_version, // Angular's linker uses native `?.` for declarations at version 22+ (and the // placeholder), which is not modeled here yet: keep legacy, and never let the @@ -7505,11 +7523,11 @@ pub fn compile_host_bindings_for_linker( } let fn_js = result.host_binding_fn.map(|f| { - let expr = OutputExpression::Function(oxc_allocator::Box::new_in(f, &&allocator)); + let expr = OutputExpression::Function(oxc_allocator::Box::new_in(f, &allocator)); emitter.emit_expression(&expr) })?; - Some(LinkerHostBindingOutput { fn_js, host_vars, declarations_js }) + Some(LinkerHostBindingOutput { fn_js, host_vars, declarations_js, next_pool_index }) } /// Output from compiling a template for the linker. @@ -7536,10 +7554,6 @@ pub struct LinkerTemplateOutput { /// The ngContentSelectors array as a JavaScript expression string, if any. pub ng_content_selectors_js: Option, - - /// The next available pool index after template compilation. - /// Used to continue constant numbering in host binding compilation. - pub next_pool_index: u32, } /// Compile a template for the linker, returning all data needed to build a `defineComponent` call. @@ -7547,6 +7561,10 @@ pub struct LinkerTemplateOutput { /// This is similar to `compile_template_to_js_with_options` but returns a richer result /// that includes numeric metadata (decls, vars) and the consts/ngContentSelectors as strings, /// which the linker needs to assemble the `defineComponent({...})` replacement. +/// +/// Angular's `compileComponentFromMetadata` compiles the host bindings first, so the +/// template's pool continues from `pool_starting_index` and reuses what the host bindings +/// pooled, from `shared_pool` (see [`compile_host_bindings_for_linker`]). pub fn compile_template_for_linker<'a>( allocator: &'a Allocator, template: &'a str, @@ -7556,6 +7574,8 @@ pub fn compile_template_for_linker<'a>( core_namespace: &str, angular_version: Option, defer_block_deps: &[Option<&str>], + pool_starting_index: u32, + shared_pool: SharedPool<'a>, ) -> Result> { use crate::pipeline::ingest::{IngestOptions, ingest_component_with_options}; use oxc_allocator::FromIn; @@ -7633,7 +7653,7 @@ pub fn compile_template_for_linker<'a>( template_source: Some(template), all_deferrable_deps_fn: None, defer_blocks, - pool_starting_index: 0, + pool_starting_index, angular_version, // Angular's linker uses native `?.` for declarations at version 22+ (and the // placeholder), which is not modeled here yet: keep legacy, and never let the @@ -7649,6 +7669,7 @@ pub fn compile_template_for_linker<'a>( ingest_options, ); job.core_namespace = Ident::from_in(core_namespace, &allocator); + job.pool.inherit_shared(shared_pool); let compiled = compile_template(&mut job); @@ -7750,7 +7771,6 @@ pub fn compile_template_for_linker<'a>( } let declarations_js = emitter.emit_statements(&all_statements); - let next_pool_index = job.pool.next_name_index(); Ok(LinkerTemplateOutput { declarations_js, @@ -7759,7 +7779,6 @@ pub fn compile_template_for_linker<'a>( vars, consts_js, ng_content_selectors_js, - next_pool_index, }) } diff --git a/crates/oxc_angular_compiler/src/directive/compiler.rs b/crates/oxc_angular_compiler/src/directive/compiler.rs index 2e72c48ce..508ffbc7b 100644 --- a/crates/oxc_angular_compiler/src/directive/compiler.rs +++ b/crates/oxc_angular_compiler/src/directive/compiler.rs @@ -212,6 +212,7 @@ fn build_base_directive_fields<'a>( core_namespace, metadata, next_pool_index, + shared_pool, angular_version, ) { next_pool_index = new_pool_index; @@ -622,7 +623,8 @@ pub fn create_outputs_literal<'a>( /// /// The `pool_starting_index` parameter is used to ensure constant names don't conflict /// when compiling multiple directives in the same file. Each directive continues from -/// where the previous directive's pool left off. +/// where the previous directive's pool left off, and `shared_pool` carries what the +/// earlier classes of the file pooled, to reuse. /// /// Returns a tuple of (result, next_pool_index) where next_pool_index is the /// next available constant pool index after host binding compilation. @@ -631,6 +633,7 @@ fn compile_directive_host_bindings<'a>( core_namespace: &'a str, metadata: &R3DirectiveMetadata<'a>, pool_starting_index: u32, + shared_pool: &mut SharedPool<'a>, angular_version: Option, ) -> Option<(HostBindingCompilationResult<'a>, u32)> { let host = &metadata.host; @@ -658,7 +661,9 @@ fn compile_directive_host_bindings<'a>( None, ); job.core_namespace = Ident::from(core_namespace); + job.pool.inherit_shared(std::mem::take(shared_pool)); let result = compile_host_bindings(&mut job); + *shared_pool = job.pool.take_shared(); // Get the next pool index after host binding compilation let next_pool_index = job.pool.next_name_index(); diff --git a/crates/oxc_angular_compiler/src/linker/mod.rs b/crates/oxc_angular_compiler/src/linker/mod.rs index 0ed49cede..255dcfff0 100644 --- a/crates/oxc_angular_compiler/src/linker/mod.rs +++ b/crates/oxc_angular_compiler/src/linker/mod.rs @@ -46,6 +46,7 @@ use oxc_span::{GetSpan, SourceType, Span}; use crate::AngularVersion; use crate::optimizer::Edit; use crate::output::emitter::escape_string; +use crate::pipeline::constant_pool::SharedPool; use crate::pipeline::phases::parse_extracted_styles::parse_style_string; use crate::pipeline::selector::{R3SelectorElement, parse_selector_to_r3_selector}; @@ -1441,10 +1442,12 @@ fn link_directive( let host_input = extract_host_metadata_input(host_obj); let selector = get_string_property(meta, "selector"); if let Some(host_output) = crate::component::compile_host_bindings_for_linker( + &Allocator::default(), &host_input, type_name, selector, 0, // directives have no template, so pool starts at 0 + &mut SharedPool::default(), ns, options.angular_version, ) { @@ -1791,6 +1794,26 @@ fn link_component( }) .unwrap_or_default(); + // Angular's `compileComponentFromMetadata` compiles the host bindings (in + // `baseDirectiveFields`) before the template, with one pool: the host + // bindings' constants come first, and the template reuses equal ones. + let host_obj = get_object_property(meta, "host"); + let mut shared_pool = SharedPool::default(); + let host_output = host_obj.and_then(|host_obj| { + // Extract host properties and listeners as HostMetadataInput and compile + // through the full Angular expression parser for correct output. + crate::component::compile_host_bindings_for_linker( + &template_allocator, + &extract_host_metadata_input(host_obj), + type_name, + get_string_property(meta, "selector"), + 0, + &mut shared_pool, + ns, + options.angular_version, + ) + }); + let template_output = crate::component::compile_template_for_linker( &template_allocator, template_ref, @@ -1800,6 +1823,8 @@ fn link_component( ns, options.angular_version, &defer_block_deps, + host_output.as_ref().map_or(0, |host_output| host_output.next_pool_index), + shared_pool, ) .ok()?; @@ -1830,26 +1855,15 @@ fn link_component( } // 5-7. Host bindings (hostAttrs, hostVars, hostBindings) - if let Some(host_obj) = get_object_property(meta, "host") { + if let Some(host_obj) = host_obj { // Static attributes → hostAttrs let host_attrs = build_host_attrs(host_obj, source); if !host_attrs.is_empty() { parts.push(format!("hostAttrs: [{host_attrs}]")); } - // Dynamic bindings → hostVars + hostBindings function - // Extract host properties and listeners as HostMetadataInput and compile - // 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_output) = crate::component::compile_host_bindings_for_linker( - &host_input, - type_name, - selector, - template_output.next_pool_index, - ns, - options.angular_version, - ) { + // Dynamic bindings → hostVars + hostBindings function (compiled above) + if let Some(host_output) = host_output { if host_output.host_vars > 0 { parts.push(format!("hostVars: {}", host_output.host_vars)); } @@ -2095,11 +2109,10 @@ fn link_component( let define_component = format!("/*@__PURE__*/ {ns}.\u{0275}\u{0275}defineComponent({{ {} }})", parts.join(", ")); - // 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); - } + // Wrap in IIFE with host binding and template declarations, in the order + // they were pooled + let mut declarations = host_binding_declarations_js; + declarations.push_str(&template_output.declarations_js); if declarations.trim().is_empty() { Some(define_component) } else { diff --git a/crates/oxc_angular_compiler/src/output/ast.rs b/crates/oxc_angular_compiler/src/output/ast.rs index a7ce41583..7e939f92d 100644 --- a/crates/oxc_angular_compiler/src/output/ast.rs +++ b/crates/oxc_angular_compiler/src/output/ast.rs @@ -1158,10 +1158,10 @@ impl<'a> OutputExpression<'a> { _ => false, } } - // Wrapped nodes compare by identity upstream; node_id is the port's - // stable identity for a wrapped external node. + // Wrapped nodes compare by identity upstream; the port's identity + // for a wrapped node of the file is its name and its span. (OutputExpression::WrappedNode(a), OutputExpression::WrappedNode(b)) => { - a.node_id == b.node_id + a.node_id == b.node_id && a.source_span == b.source_span } (OutputExpression::WrappedIrNode(a), OutputExpression::WrappedIrNode(b)) => { std::ptr::eq(a.node.as_ref(), b.node.as_ref()) diff --git a/crates/oxc_angular_compiler/src/output/emitter.rs b/crates/oxc_angular_compiler/src/output/emitter.rs index 09f7bb3ee..cb1e289c4 100644 --- a/crates/oxc_angular_compiler/src/output/emitter.rs +++ b/crates/oxc_angular_compiler/src/output/emitter.rs @@ -897,17 +897,12 @@ impl JsEmitter { OutputExpression::LocalizedString(e) => { self.visit_localized_string(e, ctx); } - OutputExpression::WrappedNode(_) => { - // Wrapped nodes should not appear in JavaScript output. - // This matches Angular's abstract_js_emitter.ts which throws: - // `throw new Error("Cannot emit a WrappedNodeExpr in Javascript.");` - // WrappedNodeExpr is used internally during compilation but should - // be resolved before emission - if we hit this, it's a compiler bug. - ctx.diagnostics.push(OxcDiagnostic::error( - "Cannot emit a WrappedNodeExpr in JavaScript. WrappedNodeExpr should be resolved before emission." - )); - // Emit undefined as fallback - ctx.print("undefined"); + OutputExpression::WrappedNode(e) => { + // A wrapped node of the class's file is emitted as written, as + // ngtsc's `ExpressionTranslatorVisitor.visitWrappedNodeExpr` + // does. Its span is in that file, not in the template the + // source map points to, so it isn't mapped. + ctx.print(&e.node_id); } OutputExpression::WrappedIrNode(_) => { // Wrapped IR expressions should not appear in JavaScript output. diff --git a/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs b/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs index 6259b76ee..40b04c626 100644 --- a/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs +++ b/crates/oxc_angular_compiler/src/pipeline/constant_pool.rs @@ -12,7 +12,7 @@ use rustc_hash::FxHashMap; use crate::ir::expression::IrExpression; use crate::output::ast::{ DeclareFunctionStmt, DeclareVarStmt, FunctionExpr, LiteralValue, OutputExpression, - OutputStatement, ReadVarExpr, StmtModifier, + OutputStatement, ReadVarExpr, StmtModifier, clone_output_statement, }; /// Strings at least this long (in UTF-16 code units, like JS `length`) are @@ -23,19 +23,22 @@ const POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS: usize = 50; /// What one class's pool hands to the next class's pool in a file. /// -/// ngtsc compiles every class of a file with one `ConstantPool`, so a literal -/// pooled by one class is reused by the next, and a name one class claims is -/// not claimed again by another. Oxc gives each class its own pool (with `_c` -/// names chained by the starting index), so the file transform carries these -/// over with [`ConstantPool::take_shared`] and [`ConstantPool::inherit_shared`]. +/// ngtsc compiles every class of a file with one `ConstantPool`, so a literal, +/// pure function or shared function pooled by one class is reused by the +/// next, and a name one class claims is not claimed again by another. Oxc +/// gives each class its own pool (with `_c` names chained by the starting +/// index), so the file transform carries these over with +/// [`ConstantPool::take_shared`] and [`ConstantPool::inherit_shared`]. #[derive(Debug, Default, Clone)] pub struct SharedPool<'a> { /// Literals pooled by `get_const_literal`, by key. literals: FxHashMap>, - /// Names claimed by `unique_name` that are not numbered by the file - /// offset (`Cmp_Template`, `Cmp_Conditional_0_Template`, ...). Two classes - /// can sanitize to the same prefix (`$A` and `_A`), and their view - /// functions are declared side by side. + /// Pure functions pooled by `pool_pure_function`, by key. + shared_constants: FxHashMap>, + /// Functions declared by `get_shared_function_reference`. + shared_functions: std::vec::Vec<&'a OutputStatement<'a>>, + /// Names claimed by `unique_name` (`_forTrack`, `Cmp_Template`, ...), with + /// their usage counts. claimed_names: FxHashMap, } @@ -54,16 +57,18 @@ pub struct ConstantPool<'a> { values: Vec<'a, PooledConstant<'a>>, /// Counter for generating unique names (_c0, _c1, etc.). next_name_index: u32, - /// Global offset for unique_name calls. - /// This is used to ensure unique names don't conflict when compiling - /// multiple components in the same file. - unique_name_offset: u32, - /// Claimed names for unique_name deduplication, including the - /// class-specific names of the earlier classes of the file. Maps base name - /// to usage count. + /// Claimed names for unique_name deduplication, including the names of + /// the earlier classes of the file. Maps base name to usage count. claimed_names: FxHashMap, /// Literals pooled by `get_const_literal`, by key. literals: FxHashMap>, + /// Pure functions pooled by `pool_pure_function`, including the earlier + /// classes' of the file, by key. + shared_constants: FxHashMap>, + /// The functions declared by `get_shared_function_reference`, including + /// the earlier classes' of the file, whose statements may have left + /// `statements` already. + shared_functions: std::vec::Vec<&'a OutputStatement<'a>>, /// Long strings an earlier class of the file used inline once, shared by /// this pool. Their declaration and first use belong to that earlier class. inherited_fixups: std::vec::Vec>, @@ -80,9 +85,10 @@ impl<'a> ConstantPool<'a> { arrays: FxHashMap::default(), values: Vec::new_in(&allocator), next_name_index: 0, - unique_name_offset: 0, claimed_names: FxHashMap::default(), literals: FxHashMap::default(), + shared_constants: FxHashMap::default(), + shared_functions: std::vec::Vec::new(), inherited_fixups: std::vec::Vec::new(), statements: Vec::new_in(&allocator), } @@ -99,9 +105,10 @@ impl<'a> ConstantPool<'a> { /// /// # Arguments /// - /// * `starting_index` - Starting index for constant names (_c0, _c1, etc.) - /// AND unique names (_forTrack0, _forTrack1, etc.). Both use the same - /// starting index for simplicity. + /// * `starting_index` - Starting index for constant names (_c0, _c1, etc.). + /// Names from `unique_name` (_forTrack0, ...) are counted per base name + /// instead, with the earlier classes' counts from + /// [`ConstantPool::inherit_shared`]. pub fn with_starting_index(allocator: &'a Allocator, starting_index: u32) -> Self { Self { allocator, @@ -109,32 +116,21 @@ impl<'a> ConstantPool<'a> { arrays: FxHashMap::default(), values: Vec::new_in(&allocator), next_name_index: starting_index, - unique_name_offset: starting_index, claimed_names: FxHashMap::default(), literals: FxHashMap::default(), + shared_constants: FxHashMap::default(), + shared_functions: std::vec::Vec::new(), inherited_fixups: std::vec::Vec::new(), statements: Vec::new_in(&allocator), } } - /// Returns the next name index that would be used. + /// Returns the next `_c` constant index that would be used. /// - /// This returns the maximum of next_name_index and the total unique names - /// generated (offset + claimed count), ensuring proper state transfer when - /// compiling multiple components in the same file. + /// Angular numbers `_c` constants (`freshName`) apart from the other + /// names `uniqueName` hands out, so only constants move it. pub fn next_name_index(&self) -> u32 { - // Calculate the total unique names generated: offset + claimed counts of - // the generic names the offset applies to. Component-specific names - // (like `ComponentName_Template`) take no index: ngtsc numbers `_c` - // constants on their own. - let total_unique_names: u32 = self.unique_name_offset - + self - .claimed_names - .iter() - .filter(|(base, _)| is_generic_name(base)) - .map(|(_, count)| count) - .sum::(); - self.next_name_index.max(total_unique_names) + self.next_name_index } /// Returns the allocator. @@ -265,22 +261,21 @@ impl<'a> ConstantPool<'a> { /// /// # Returns /// - /// A tuple of (index, function_name) where the function name can be used to - /// reference this pooled function. + /// The function name, which can be used to reference this pooled function. + /// An equal function an earlier class of the file pooled is reused, as + /// Angular's `getSharedConstant` does; that class declares it. pub fn pool_pure_function( &mut self, num_args: u32, body_key: &str, body_expr: IrExpression<'a>, - ) -> (u32, Ident<'a>) { + ) -> Ident<'a> { // Create a key for deduplication based on arg count and body let key = format!("pf:{}:{}", num_args, body_key); // Check if already pooled - if let Some(pooled) = self.strings.get(&key) { - // Return existing index and name - let constant = &self.values[pooled.index as usize]; - return (pooled.index, constant.name.clone()); + if let Some(name) = self.shared_constants.get(&key) { + return *name; } let index = self.values.len() as u32; @@ -302,8 +297,8 @@ impl<'a> ConstantPool<'a> { )), }); - self.strings.insert(key, PooledValue { index }); - (index, name) + self.shared_constants.insert(key, name); + name } /// Gets or creates a shared function reference. @@ -311,7 +306,7 @@ impl<'a> ConstantPool<'a> { /// This is used for defer block dependency resolver functions and track functions. /// The function is added to the constant pool's statements and a reference expression /// is returned. If an equivalent function already exists, a reference to the existing - /// function is returned instead. + /// function is returned instead, even one an earlier class of the file declared. /// /// Angular TypeScript stores ALL shared functions (both arrow and regular) in the /// same `statements` array, maintaining insertion order. This is critical for correct @@ -339,8 +334,10 @@ impl<'a> ConstantPool<'a> { let is_arrow = matches!(fn_expr, OutputExpression::ArrowFunction(_)); // Check if an equivalent function already exists in statements - // Angular TypeScript iterates through statements checking isEquivalent - for stmt in self.statements.iter() { + // Angular TypeScript iterates through statements checking isEquivalent. + // The earlier classes' shared functions come first, and this pool's + // may have left `statements` already. + for stmt in self.shared_functions.iter().copied().chain(self.statements.iter()) { match stmt { // Arrow functions are saved as DeclareVarStmt OutputStatement::DeclareVar(decl) if is_arrow => { @@ -426,6 +423,11 @@ impl<'a> ConstantPool<'a> { ))); } } + // Keep a copy for the next classes of the file to reuse. + if let Some(stmt) = self.statements.last() { + let stmt = self.allocator.alloc(clone_output_statement(stmt, self.allocator)); + self.shared_functions.push(stmt); + } // Return a ReadVar expression that references this shared function OutputExpression::ReadVar(Box::new_in( @@ -437,16 +439,9 @@ impl<'a> ConstantPool<'a> { /// Generates a unique name with the given base name. /// /// Used by naming phases to ensure pool-wide uniqueness. - /// Matches Angular's `ConstantPool.uniqueName()`. - /// - /// When compiling multiple components in the same file, the global - /// unique_name_offset ensures unique names across all components for - /// "generic" names like `_forTrack` that don't include a component name. - /// - /// For component-specific names (like `ComponentName_Template`), the offset - /// is NOT applied: they contain the class name, and the names claimed by - /// the earlier classes of the file come in through - /// [`ConstantPool::inherit_shared`]. + /// Matches Angular's `ConstantPool.uniqueName()`: each base name has its + /// own count, and the counts of the earlier classes of the file come in + /// through [`ConstantPool::inherit_shared`]. /// /// # Arguments /// @@ -454,20 +449,13 @@ impl<'a> ConstantPool<'a> { /// * `always_include_suffix` - If `true`, always append a numeric suffix. /// If `false`, only append suffix if the name has been used before. pub fn unique_name(&mut self, base: &str, always_include_suffix: bool) -> Ident<'a> { - // Get the per-base-name count for deduplication within this component + // Get the per-base-name count for deduplication across the file let count = self.claimed_names.get(base).copied().unwrap_or(0); - // Only apply the global offset for "generic" names (`_forTrack`, `arrowFn`). - // These names could collide across components. - // Component-specific names (like `ComponentName_Template`) don't need the - // offset because they already contain the component class name. - let apply_offset = is_generic_name(base); - let effective_count = if apply_offset { self.unique_name_offset + count } else { count }; - - let name = if effective_count == 0 && !always_include_suffix { + let name = if count == 0 && !always_include_suffix { base.to_string() } else { - format!("{}{}", base, effective_count) + format!("{base}{count}") }; // Increment the per-base-name counter @@ -538,26 +526,27 @@ impl<'a> ConstantPool<'a> { Some(self.read_var(name)) } - /// Takes the literals and class-specific names this pool and the ones - /// before it pooled and claimed, to hand them to the next class's pool. + /// Takes the literals, functions and names this pool and the ones before + /// it pooled and claimed, to hand them to the next class's pool. pub fn take_shared(&mut self) -> SharedPool<'a> { - let claimed_names = self - .claimed_names - .iter() - .filter(|(base, _)| !is_generic_name(base)) - .map(|(base, count)| (base.clone(), *count)) - .collect(); - SharedPool { literals: std::mem::take(&mut self.literals), claimed_names } + SharedPool { + literals: std::mem::take(&mut self.literals), + shared_constants: std::mem::take(&mut self.shared_constants), + shared_functions: std::mem::take(&mut self.shared_functions), + claimed_names: self.claimed_names.clone(), + } } - /// Reuses the literals pooled, and avoids the names claimed, by the - /// earlier classes of the file. + /// Reuses the literals and functions pooled, and avoids the names claimed, + /// by the earlier classes of the file. pub fn inherit_shared(&mut self, shared: SharedPool<'a>) { - let SharedPool { mut literals, claimed_names } = shared; + let SharedPool { mut literals, shared_constants, shared_functions, claimed_names } = shared; for pooled in literals.values_mut() { pooled.inherited = true; } self.literals = literals; + self.shared_constants = shared_constants; + self.shared_functions = shared_functions; self.claimed_names.extend(claimed_names); } @@ -582,14 +571,6 @@ impl<'a> ConstantPool<'a> { } } -/// Whether `unique_name` numbers `base` across the components of a file (the -/// shared `_forTrack` and `arrowFn` functions), as opposed to a -/// component-specific name like `Cmp_Template`, even for a class whose name -/// starts with `_`. -fn is_generic_name(base: &str) -> bool { - matches!(base, "_forTrack" | "arrowFn") -} - /// Whether `expr` is a string literal long enough to share. /// /// Ported from Angular's `isLongStringLiteral`. @@ -860,6 +841,21 @@ mod tests { assert!(fixups[0].value.is_equivalent(&string(&allocator, &long))); } + #[test] + fn test_unique_names_count_across_pools_apart_from_constants() { + let allocator = Allocator::default(); + let mut first_pool = ConstantPool::new(&allocator); + first_pool.pool_string("a"); + first_pool.pool_string("b"); + assert_eq!(first_pool.unique_name("_forTrack", true).as_str(), "_forTrack0"); + + let mut pool = ConstantPool::with_starting_index(&allocator, first_pool.next_name_index()); + pool.inherit_shared(first_pool.take_shared()); + assert_eq!(pool.unique_name("_forTrack", true).as_str(), "_forTrack1"); + assert_eq!(pool.unique_name("arrowFn", true).as_str(), "arrowFn0"); + assert_eq!(pool.next_name_index(), 2); + } + #[test] fn test_shared_function_without_unique_name_takes_no_index() { let allocator = &Allocator::default(); diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/pure_function_extraction.rs b/crates/oxc_angular_compiler/src/pipeline/phases/pure_function_extraction.rs index 0729ff811..9d1d24391 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/pure_function_extraction.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/pure_function_extraction.rs @@ -103,8 +103,7 @@ fn extract_pure_function<'a>( // Pool immediately (matching TypeScript's single-pass behavior) let fn_name = { let mut pool = pool_cell.borrow_mut(); - let (_, name) = pool.pool_pure_function(pf.args.len() as u32, &body_key, body); - name + pool.pool_pure_function(pf.args.len() as u32, &body_key, body) }; // Set fn_ref to a direct variable reference (not ctx.property). diff --git a/crates/oxc_angular_compiler/tests/const_pool_file_wide_test.rs b/crates/oxc_angular_compiler/tests/const_pool_file_wide_test.rs new file mode 100644 index 000000000..f25468a7a --- /dev/null +++ b/crates/oxc_angular_compiler/tests/const_pool_file_wide_test.rs @@ -0,0 +1,242 @@ +//! ngtsc compiles all the classes of a file with one constant pool: `_c` constants +//! (`freshName`) and `uniqueName` names (`_forTrack`, `arrowFn`) are counted across the file +//! apart from each other, an equal pure function or shared function is declared once by the +//! first class and reused by the later ones, and a component pools its host bindings' +//! constants (`baseDirectiveFields`) before its template's. + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{ + CompilationMode, ComponentHmrUpdateInput, TransformOptions, compile_component_hmr_update, link, + transform_angular_file, +}; + +fn transform(source: &str) -> String { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, "test.ts", source, None, None); + assert!(!result.has_errors(), "{:?}", result.diagnostics); + result.code +} + +/// `code` without whitespace, so the emitter's line wrapping doesn't matter. +fn squash(code: &str) -> String { + code.chars().filter(|c| !c.is_whitespace()).collect() +} + +/// The value `squashed` declares `name` with, up to its `;`. +fn value<'c>(squashed: &'c str, name: &str) -> &'c str { + let start = squashed.find(&format!("const{name}=")).expect(squashed) + name.len() + 6; + let end = squashed[start..].find(';').expect(squashed); + &squashed[start..start + end] +} + +/// The names of the top-level `const`s of `code`, in order. +fn constants(code: &str) -> Vec<&str> { + code.lines() + .filter_map(|line| line.strip_prefix("const ")) + .filter_map(|rest| rest.split([' ', '=']).next()) + .collect() +} + +#[test] +fn track_functions_are_numbered_apart_from_constants() { + // ngtsc: `_c0`, `_c1`, `_forTrack0` for A, then `_forTrack1` for B, not + // a name numbered from B's first `_c` index. + let code = transform( + r#" +import { Component } from '@angular/core'; + +@Component({ + selector: 'cmp-a', + template: '

@for (it of items; track it.id) {}', +}) +export class A { k = 1; items: any[] = []; } + +@Component({ + selector: 'cmp-b', + template: '

@for (it of items; track it.name) {}', +}) +export class B { k = 1; items: any[] = []; } +"#, + ); + assert_eq!(constants(&code), ["_c0", "_c1", "_forTrack0", "_c2", "_forTrack1"], "{code}"); + assert!( + squash(&code).contains("ɵɵrepeaterCreate(1,B_For_2_Template,1,0,\"b\",null,_forTrack1)"), + "{code}" + ); +} + +#[test] +fn equal_track_functions_are_declared_once_per_file() { + let code = transform( + r" +import { Component } from '@angular/core'; + +@Component({ selector: 'cmp-a', template: '@for (it of items; track it.id) {}' }) +export class A { items: any[] = []; } + +@Component({ selector: 'cmp-b', template: '

@for (it of items; track it.id) {}' }) +export class B { items: any[] = []; } +", + ); + assert_eq!(constants(&code), ["_forTrack0"], "{code}"); + let squashed = squash(&code); + assert!(squashed.contains("A_For_1_Template,1,0,\"b\",null,_forTrack0)"), "{code}"); + assert!(squashed.contains("B_For_2_Template,1,0,\"i\",null,_forTrack0)"), "{code}"); +} + +#[test] +fn equal_arrow_functions_are_declared_once_per_file() { + let code = transform( + r#" +import { Component } from '@angular/core'; + +@Component({ selector: 'cmp-a', template: '

' }) +export class A { map(f: any) { return f; } } + +@Component({ + selector: 'cmp-b', + template: '

', +}) +export class B { map(f: any) { return f; } } +"#, + ); + assert_eq!(constants(&code), ["arrowFn0", "arrowFn1"], "{code}"); + assert_eq!(squash(&code).matches(",arrowFn0,ctx)").count(), 2, "{code}"); +} + +#[test] +fn equal_pure_functions_of_a_directive_and_a_component_are_one_constant() { + // ngtsc: `const _c0 = (a0, a1) => ({ height: a0, width: a1 });`, read by + // the directive's host bindings and the component's template. + let code = transform( + r#" +import { Component, Directive } from '@angular/core'; + +@Directive({ selector: '[dir]', host: { '[style]': '{height: h, width: w}' } }) +export class Dir { h = 1; w = 2; } + +@Component({ selector: 'cmp', template: '

' }) +export class Cmp { h = 1; w = 2; } +"#, + ); + assert_eq!(constants(&code), ["_c0", "_c1"], "{code}"); + let squashed = squash(&code); + assert_eq!(squashed.matches("ɵɵstyleMap(i0.ɵɵpureFunction2(").count(), 2, "{code}"); + assert_eq!(squashed.matches(",_c0,ctx.h,ctx.w)").count(), 2, "{code}"); + assert_eq!(value(&squashed, "_c1"), "(a0)=>[a0,5]", "{code}"); +} + +#[test] +fn host_binding_constants_come_before_the_template_ones() { + // ngtsc: the query predicate, then the host bindings' pure function, then + // the template's, each declared in that order. + let code = transform( + r#" +import { Component, ViewChild } from '@angular/core'; + +@Component({ + selector: 'cmp', + template: '
', + host: { '[class]': '{hostA: k}' }, +}) +export class Cmp { @ViewChild('x') x: any; k = true; } +"#, + ); + assert_eq!(constants(&code), ["_c0", "_c1", "_c2"], "{code}"); + let squashed = squash(&code); + assert_eq!(value(&squashed, "_c0"), "[\"x\"]", "{code}"); + assert!(value(&squashed, "_c1").contains("hostA:a0"), "{code}"); + assert!(value(&squashed, "_c2").contains("a:a0"), "{code}"); +} + +#[test] +fn template_reuses_a_host_binding_pure_function() { + let code = transform( + r#" +import { Component } from '@angular/core'; + +@Component({ selector: 'cmp', template: '
', host: { '[class]': '{a: k}' } }) +export class Cmp { k = true; } +"#, + ); + assert_eq!(constants(&code), ["_c0"], "{code}"); + assert_eq!(squash(&code).matches("i0.ɵɵpureFunction1(").count(), 2, "{code}"); +} + +#[test] +fn hmr_update_declares_the_host_constants_its_template_reads() { + let source = r#" +import { Component } from '@angular/core'; + +@Component({ selector: 'cmp', template: '
', host: { '[class]': '{a: k}' } }) +export class Cmp { k = true; } +"#; + let allocator = Allocator::default(); + let input = ComponentHmrUpdateInput { class_name: "Cmp", ..Default::default() }; + let options = TransformOptions { hmr: true, ..Default::default() }; + let update = + compile_component_hmr_update(&allocator, "test.ts", source, &input, Some(&options), None); + assert!(update.diagnostics.is_empty(), "{:?}", update.diagnostics); + let module = update.module.expect("update module"); + let squashed = squash(&module); + assert!(value(&squashed, "_c0").contains("a:a0"), "{module}"); + assert!(squashed.contains("_c0,ctx.k"), "{module}"); +} + +#[test] +fn linked_component_keeps_template_and_host_arrow_functions_apart() { + let allocator = Allocator::default(); + let options = + TransformOptions { compilation_mode: CompilationMode::Partial, ..Default::default() }; + let source = r#" +import { Component } from '@angular/core'; + +@Component({ + selector: 'cmp', + template: '

', + host: { '[title]': 'map((q) => q * 3)' }, +}) +export class Cmp { map(f: any) { return f; } } +"#; + let result = transform_angular_file(&allocator, "case.ts", source, Some(&options), None); + assert!(!result.has_errors(), "{:?}", result.diagnostics); + let linked = link(&allocator, &result.code, "case.ts"); + assert!(linked.linked, "{}", result.code); + let squashed = squash(&linked.code); + assert_eq!(squashed.matches("constarrowFn0=").count(), 1, "{}", linked.code); + assert_eq!(squashed.matches("constarrowFn1=").count(), 1, "{}", linked.code); + // ngtsc's linker compiles the host bindings first. + assert_eq!(value(&squashed, "arrowFn0"), "(ctx,view)=>(q)=>(q*3)", "{}", linked.code); + assert_eq!(value(&squashed, "arrowFn1"), "(ctx,view)=>(y)=>(y*2)", "{}", linked.code); +} + +#[test] +fn linked_component_pools_host_constants_first_and_shares_them_with_the_template() { + // ngtsc's linker (`compileComponentFromMetadata`, one pool): the host + // bindings' pure and arrow functions come first, and the template reuses + // the equal ones. + let allocator = Allocator::default(); + let options = + TransformOptions { compilation_mode: CompilationMode::Partial, ..Default::default() }; + let source = r#" +import { Component } from '@angular/core'; + +@Component({ + selector: 'cmp', + template: '', + host: { '[title]': 'f((a) => a + 1)', '[style]': '{width: w, height: h}', '[class]': '{x: w}' }, +}) +export class Cmp { w = 1; h = 2; f(x: any) { return ''; } } +"#; + let result = transform_angular_file(&allocator, "case.ts", source, Some(&options), None); + assert!(!result.has_errors(), "{:?}", result.diagnostics); + let linked = link(&allocator, &result.code, "case.ts"); + assert!(linked.linked, "{}", result.code); + assert_eq!(constants(&linked.code), ["_c0", "_c1", "arrowFn0", "_c2"], "{}", linked.code); + let squashed = squash(&linked.code); + assert!(value(&squashed, "_c0").contains("width:a0,height:a1"), "{}", linked.code); + assert!(value(&squashed, "_c1").contains("x:a0"), "{}", linked.code); + assert_eq!(value(&squashed, "_c2"), "(a0)=>[a0,1]", "{}", linked.code); + assert_eq!(squashed.matches(",_c0,ctx.w,ctx.h)").count(), 2, "{}", linked.code); + assert_eq!(squashed.matches(",arrowFn0,ctx)").count(), 2, "{}", linked.code); +} diff --git a/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs index b364c73d2..aeead3d56 100644 --- a/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs +++ b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs @@ -1471,7 +1471,9 @@ export class A2 {{ #[test] fn block_only_dependencies_leave_dependencies_of_each_component() { // Like ngc, a same-file component used only inside `@defer` is provided - // by the block's function, not by `dependencies`. + // by the block's function, not by `dependencies`. The second component's + // function is the first one's, which ngc's single pool finds equivalent + // (`getSharedFunctionReference`). let dir = TempDir::new().unwrap(); let result = transform( &dir, @@ -1495,7 +1497,53 @@ export class App2 {} let code = &result.code; let compact = compact(code); assert!(compact.contains("constApp1_Defer_1_DepsFn=()=>[LocalCmp];"), "{code}"); - assert!(compact.contains("constApp2_Defer_2_DepsFn=()=>[LocalCmp];"), "{code}"); + assert!(compact.contains("ɵɵdefer(2,1,App1_Defer_1_DepsFn);"), "{code}"); + assert!(!compact.contains("App2_Defer_2_DepsFn"), "{code}"); assert!(!compact.contains("ɵɵgetComponentDepsFactory(App1"), "{code}"); assert!(compact.contains("ɵɵgetComponentDepsFactory(App2,[LocalCmp])"), "{code}"); } + +#[test] +fn deps_fns_reading_an_imported_class_are_one_per_component() { + // ngc reads an imported class through the identifier in each component's + // `imports` (a `WrappedNodeExpr`), and `isEquivalent` compares nodes: the + // blocks of one component share a function, two components don't. + let dir = TempDir::new().unwrap(); + create_test_file( + dir.path(), + "app/deps.ts", + r" +import { Component } from '@angular/core'; + +@Component({ selector: 'app-dep', template: 'dep' }) +export class Dep {} + +@Component({ selector: 'app-dep2', template: 'dep2' }) +export class Dep2 {} +", + ); + let result = transform( + &dir, + r" +import { Component } from '@angular/core'; +import { Dep, Dep2 } from './deps'; + +@Component({ + selector: 'app-b', + imports: [Dep, Dep2], + template: ' @defer { } @defer { }', +}) +export class B {} + +@Component({ selector: 'app-c', imports: [Dep, Dep2], template: ' @defer { }' }) +export class C {} +", + ); + let code = &result.code; + let compact = compact(code); + assert!(compact.contains("constB_Defer_2_DepsFn=()=>[Dep];"), "{code}"); + assert!(compact.contains("ɵɵdefer(2,1,B_Defer_2_DepsFn);"), "{code}"); + assert!(compact.contains("ɵɵdefer(5,4,B_Defer_2_DepsFn);"), "{code}"); + assert!(compact.contains("constC_Defer_2_DepsFn=()=>[Dep];"), "{code}"); + assert!(compact.contains("ɵɵdefer(2,1,C_Defer_2_DepsFn);"), "{code}"); +} diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 1fb7499fe..b3c518913 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -2960,7 +2960,7 @@ export class FirstComponent { @Component({ selector: 'app-second', template: ` - @for (user of users; track user.id) { + @for (user of users; track user.email) { {{ user.email }} } `, @@ -2996,14 +2996,11 @@ export class SecondComponent { ); } - // Verify we have at least 2 unique track variables (one from each component) - assert!( - track_names.len() >= 2, - "Expected at least 2 unique track variables, got {}. Variables: {:?}\nFull output:\n{}", - track_names.len(), - track_names, - code - ); + // One track function from each component, numbered by the `_forTrack` + // count across the file like ngtsc's single pool. + let expected: std::collections::HashSet = + ["_forTrack0", "_forTrack1"].into_iter().map(String::from).collect(); + assert_eq!(track_names, expected, "Full output:\n{code}"); // Also check for function declarations with numbered suffixes // Both components generate template functions with unique names From 3d8c45db3c93b51b080e4fe91921d5b38e247681 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 29 Oct 2026 12:27:42 +0800 Subject: [PATCH 06/11] fix(defer): lazy-load an import every component of the file defers An import used only inside `@defer` blocks by two components of the same file stayed eager: a class was deferrable only when its `imports` entry was its single value reference, so a second component listing it kept the static import. Port ngtsc's file-wide rule instead. `DeferredSymbolTracker` counts each specifier's references and `markAsDeferrableCandidate` removes the `imports` entries of components that use the class only inside `@defer` (`registerDeferrableCandidate` in `resolveDeferBlocks`); `canDefer` allows a declaration when no specifier has references left. Another component's marks are found the first time a decision needs them, by parsing its template. A class two entries name is listed once (`seenDeps` in `resolveAllDeferredDependencies`). Angular sources: compiler-cli ngtsc/imports/src/deferred_symbol_tracker.ts and ngtsc/annotations/component/src/handler.ts. Co-Authored-By: Claude Opus 5.5 --- .../src/component/transform.rs | 355 +++++++++++++----- .../tests/defer_implicit_imports_test.rs | 148 +++++++- 2 files changed, 414 insertions(+), 89 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 3eb2e4e98..9da7837e5 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -3441,6 +3441,48 @@ pub fn transform_angular_file( } } + // The selector of the standalone component or directive an `imports` + // entry refers to, when cross-file analysis can read it or the class is + // declared in this file, and whether a named import brings it in, so + // `@defer` may lazy-load it (#313). Watched files are only reported with + // `resolve_imported_values`, so it is required here too. + #[cfg(feature = "cross_file_elision")] + let import_selector = + cross_file_analyzer.as_ref().filter(|_| options.resolve_imported_values).map(|analyzer| { + let import_map = &import_map; + let program = &parser_ret.program; + let string_consts = &string_consts; + let implicit_standalone = options.implicit_standalone(); + move |local: &str| -> Option<(String, bool)> { + let Some(info) = import_map.get(&Ident::from(local)) else { + let selector = super::cross_file_elision::local_standalone_class_selector( + program, + string_consts, + local, + implicit_standalone, + )?; + return Some((selector, false)); + }; + if !info.is_named_import || info.is_type_only { + return None; + } + let export_name = info.imported_name.as_deref().unwrap_or(local); + let selector = analyzer.standalone_directive_selector( + info.source_module.as_str(), + export_name, + std::path::Path::new(path), + implicit_standalone, + )?; + Some((selector, true)) + } + }); + // ngc's `DeferredSymbolTracker` sees every component of the file before + // any compiles. The `imports` entries another component marks are found + // the first time a decision needs them, by class span. + #[cfg(feature = "cross_file_elision")] + let deferrable_marks: std::cell::RefCell>>> = + std::cell::RefCell::default(); + // 2. Walk AST to find @Component decorated classes and extract metadata for stmt in &parser_ret.program.body { let (class, stmt_start) = match stmt { @@ -3686,53 +3728,66 @@ pub fn transform_angular_file( let content_query_names: Vec = content_queries.iter().map(|q| q.property_name.to_string()).collect(); - // The selector of the standalone component or directive an - // `imports` entry refers to, when cross-file analysis can - // read it or the class is declared in this file, and whether - // `@defer` may lazy-load it (#313): its import is referenced - // nowhere else in this file or in the compiled output, so the - // static import can go. Watched files are only reported with - // `resolve_imported_values`, so it is required here too. + // ngc's `DeferredSymbolTracker.canDefer` (#313): `@defer` + // may replace an import declaration with dynamic `import()`s + // when no specifier in it is referenced in the compiled + // output or anywhere in this file except the `imports` + // entries the file's components mark as deferrable + // candidates. An explicit `deferredImports` entry is such a + // reference: OXC keeps its static import. #[cfg(feature = "cross_file_elision")] - let import_selector = cross_file_analyzer - .as_ref() - .filter(|_| options.resolve_imported_values) - .map(|analyzer| { - let import_map = &import_map; - let import_elision = &import_elision_mut; - let program = &parser_ret.program; - let string_consts = &string_consts; - move |local: &str| -> Option<(String, bool)> { - let Some(info) = import_map.get(&Ident::from(local)) else { - let selector = - super::cross_file_elision::local_standalone_class_selector( - program, - string_consts, - local, - implicit_standalone, - )?; - return Some((selector, false)); - }; - if !info.is_named_import || info.is_type_only { - return None; + let can_defer = |name: &str, marked: &[Ident<'_>]| -> bool { + let import_elision = &import_elision_mut; + import_elision.declaration_specifiers(name).iter().all(|specifier| { + let Some(references) = import_elision.value_reference_count(specifier) + else { + return false; + }; + if import_elision.is_kept_by_output(specifier) { + return false; + } + let marks_of = |marked: &[Ident<'_>]| { + marked.iter().filter(|m| m.as_str() == specifier.as_str()).count() + }; + let mut marks = marks_of(marked); + if marks < references + && references != usize::MAX + && let Some(import_selector) = &import_selector + { + let mut deferrable_marks = deferrable_marks.borrow_mut(); + for other in + parser_ret.program.body.iter().filter_map(statement_class) + { + if other.span == class.span { + continue; + } + marks += marks_of( + deferrable_marks.entry(other.span).or_insert_with(|| { + component_deferrable_marks( + allocator, + other, + path, + source, + options, + &import_map, + &string_consts, + resolved_resources, + import_selector, + ) + }), + ); } - let export_name = info.imported_name.as_deref().unwrap_or(local); - let selector = analyzer.standalone_directive_selector( - info.source_module.as_str(), - export_name, - std::path::Path::new(path), - implicit_standalone, - )?; - let deferrable = import_elision.value_reference_count(local) - == Some(1) - && !import_elision.is_kept_by_output(local); - Some((selector, deferrable)) } - }); + marks > 0 && marks >= references + }) + }; #[cfg(feature = "cross_file_elision")] - let import_selector = import_selector.as_ref().map(|f| f as ImportSelector<'_>); + let (import_selector, can_defer) = ( + import_selector.as_ref().map(|f| f as ImportSelector<'_>), + Some(&can_defer as CanDefer<'_>), + ); #[cfg(not(feature = "cross_file_elision"))] - let import_selector = None; + let (import_selector, can_defer) = (None, None); // 4. Compile the template and generate ɵcmp/ɵfac // Pass the shared pool index to ensure unique constant names @@ -3752,7 +3807,7 @@ pub fn transform_angular_file( &import_map, resolved_resources, import_selector, - Some(&import_elision_mut), + can_defer, false, ) { Ok(mut compilation_result) => { @@ -5616,9 +5671,163 @@ fn html_parse_error_diagnostic( } /// The selector of the standalone component or directive an `imports` -/// entry refers to, by local name, and whether `@defer` may lazy-load it. +/// entry refers to, by local name, and whether a named import brings it in, +/// which `@defer` may then replace with a dynamic `import()`. type ImportSelector<'f> = &'f dyn Fn(&str) -> Option<(String, bool)>; +/// ngc's `DeferredSymbolTracker.canDefer` for the import declaration that +/// binds a name, given the `imports` entries the component being compiled +/// marks as deferrable candidates. +type CanDefer<'f> = &'f dyn Fn(&str, &[Ident<'_>]) -> bool; + +/// What `@defer` may lazy-load among a component's `imports`, as ngc's +/// `resolveDeferBlocks` finds it. +struct DeferCandidates<'a> { + /// Each `imports` class with a known selector. + candidates: std::vec::Vec<(Ident<'a>, std::vec::Vec)>, + /// The candidates the template matches only inside `@defer` blocks. + block_only: std::vec::Vec>, + /// The `imports` entries naming a `block_only` candidate a named import + /// brings in, one per entry: the identifiers ngc's + /// `registerDeferrableCandidate` marks. + marked: std::vec::Vec>, +} + +/// The [`DeferCandidates`] of a component, or `None` when its template has +/// no `@defer` block or an `imports` entry can't be read. +fn implicit_defer_candidates<'a>( + metadata: &ComponentMetadata<'a>, + nodes: &[crate::ast::r3::R3Node<'a>], + import_selector: ImportSelector<'_>, +) -> Option> { + let Some(OutputExpression::LiteralArray(raw_imports)) = &metadata.raw_imports else { + return None; + }; + if raw_imports.entries.len() != metadata.import_entry_classes.len() + || !raw_imports + .entries + .iter() + .zip(&metadata.import_entry_classes) + .all(|(entry, class)| class.is_some() || matches!(entry, OutputExpression::ReadVar(_))) + || !super::defer_resolver::template_has_defer_block(nodes) + { + return None; + } + let mut imported = std::vec::Vec::new(); + let candidates: std::vec::Vec<_> = metadata + .import_entry_classes + .iter() + .flatten() + .map(|(name, _)| *name) + .filter_map(|name| { + let (selector, is_imported) = import_selector(name.as_str())?; + if is_imported { + imported.push(name); + } + Some((name, crate::pipeline::selector::CssSelector::parse(&selector))) + }) + .collect(); + let block_only = super::defer_resolver::implicitly_deferred_imports(nodes, &candidates); + let marked = metadata + .import_entry_classes + .iter() + .flatten() + .map(|(name, _)| *name) + .filter(|name| block_only.contains(name) && imported.contains(name)) + .collect(); + Some(DeferCandidates { candidates, block_only, marked }) +} + +/// The class a top-level statement declares, exported or not. +#[cfg(feature = "cross_file_elision")] +fn statement_class<'a>(stmt: &'a Statement<'a>) -> Option<&'a oxc_ast::ast::Class<'a>> { + match stmt { + Statement::ClassDeclaration(class) => Some(class), + Statement::ExportDefaultDeclaration(export) => match &export.declaration { + ExportDefaultDeclarationKind::ClassDeclaration(class) => Some(class), + _ => None, + }, + Statement::ExportDeclaration(export) => match &export.declaration { + Declaration::ClassDeclaration(class) => Some(class), + _ => None, + }, + _ => None, + } +} + +/// The `imports` entries another component of the file marks as deferrable +/// candidates ([`DeferCandidates::marked`]); empty for any other class. Its +/// template is parsed, as `compile_component_full` does, only when it may +/// hold a `@defer` block. One that fails to parse fails to compile too. +#[cfg(feature = "cross_file_elision")] +#[expect(clippy::too_many_arguments)] +fn component_deferrable_marks<'a>( + allocator: &'a Allocator, + class: &'a oxc_ast::ast::Class<'a>, + path: &str, + source: &'a str, + options: &TransformOptions, + import_map: &ImportMap<'a>, + string_consts: &crate::directive::StringConsts<'a>, + resolved_resources: Option<&ResolvedResources>, + import_selector: ImportSelector<'_>, +) -> std::vec::Vec> { + if find_jit_forced_decorator(class, string_consts).is_some() { + return std::vec::Vec::new(); + } + let Some(metadata) = extract_component_metadata( + allocator, + class, + options.implicit_standalone(), + import_map, + Some(source), + string_consts, + ) else { + return std::vec::Vec::new(); + }; + let (Some(template), _) = resolve_template(&metadata, resolved_resources) else { + return std::vec::Vec::new(); + }; + if metadata.import_entry_classes.is_empty() || !template.contains("@defer") { + return std::vec::Vec::new(); + } + let template = allocator.alloc_str(&template); + + let parse_options = ParseTemplateOptions { + preserve_whitespaces: metadata + .preserve_whitespaces + .or(options.preserve_whitespaces) + .unwrap_or(false), + enable_block_syntax: true, + enable_let_syntax: true, + tokenize_expansion_forms: true, + ..Default::default() + }; + let html_result = HtmlParser::with_options(allocator, template, path, &parse_options).parse(); + if !html_result.errors.is_empty() { + return std::vec::Vec::new(); + } + let untrimmed: &'a [crate::ast::html::HtmlNode<'a>] = allocator.alloc(html_result.nodes); + let nodes = if parse_options.preserve_whitespaces { + untrimmed + } else { + allocator.alloc(remove_whitespaces(allocator, untrimmed, true)) as &_ + }; + let r3_transform_options = R3TransformOptions { + collect_comment_nodes: parse_options.collect_comment_nodes, + angular_version: options.angular_version, + enable_i18n_legacy_message_id_format: options.enable_i18n_legacy_message_id_format, + }; + let r3_result = HtmlToR3Transform::new(allocator, template, r3_transform_options) + .with_untrimmed_nodes(untrimmed) + .transform(nodes); + if !r3_result.errors.is_empty() { + return std::vec::Vec::new(); + } + implicit_defer_candidates(&metadata, &r3_result.nodes, import_selector) + .map_or_else(std::vec::Vec::new, |candidates| candidates.marked) +} + fn compile_component_full<'a>( allocator: &'a Allocator, template: &'a str, @@ -5634,7 +5843,7 @@ fn compile_component_full<'a>( import_map: &ImportMap<'a>, resolved_resources: Option<&ResolvedResources>, import_selector: Option>, - import_elision: Option<&ImportElisionAnalyzer<'_>>, + can_defer: Option>, hmr_update: bool, ) -> Result, Vec> { use oxc_allocator::FromIn; @@ -5811,50 +6020,22 @@ fn compile_component_full<'a>( // dependency function can be built (see below). let mut defer_block_dependencies = None; if let Some(import_selector) = import_selector - && let Some(import_elision) = import_elision + && let Some(can_defer) = can_defer && !options.hmr - && let Some(OutputExpression::LiteralArray(raw_imports)) = &metadata.raw_imports - && raw_imports.entries.len() == metadata.import_entry_classes.len() - && raw_imports - .entries - .iter() - .zip(&metadata.import_entry_classes) - .all(|(entry, class)| class.is_some() || matches!(entry, OutputExpression::ReadVar(_))) - && super::defer_resolver::template_has_defer_block(&r3_result.nodes) + && let Some(DeferCandidates { candidates, block_only, marked }) = + implicit_defer_candidates(metadata, &r3_result.nodes, import_selector) { - let mut deferrable = Vec::new(); - let candidates: Vec<_> = metadata - .import_entry_classes - .iter() - .flatten() - .map(|(name, _)| *name) - .filter_map(|name| { - let (selector, can_defer) = import_selector(name.as_str())?; - if can_defer { - deferrable.push(name); - } - Some((name, crate::pipeline::selector::CssSelector::parse(&selector))) - }) - .collect(); - let block_only = - super::defer_resolver::implicitly_deferred_imports(&r3_result.nodes, &candidates); - let mut deferred = block_only.clone(); - deferred.retain(|name| deferrable.contains(name)); - // ngc's `DeferredSymbolTracker.canDefer`: an import declaration goes - // only when every non-`type` specifier in it is deferred, or the - // static import would stay and nothing would be split off. An - // explicit `deferredImports` entry doesn't count: OXC keeps its - // static import. - let mut deferred: std::vec::Vec<_> = deferred - .iter() - .copied() - .filter(|name| { - import_elision - .declaration_specifiers(name) - .iter() - .all(|specifier| deferred.contains(specifier)) - }) - .collect(); + // ngc's `DeferredSymbolTracker.canDefer`, over the whole file: the + // `imports` entries this component marks are handed over with the + // name, and the ones other components mark are counted there. A + // class two entries name is listed once (`seenDeps` in + // `resolveAllDeferredDependencies`). + let mut deferred = std::vec::Vec::new(); + for &name in &block_only { + if !deferred.contains(&name) && marked.contains(&name) && can_defer(&name, &marked) { + deferred.push(name); + } + } // ngc's full compilation gives each `@defer` block its own dependency // function (`DeferBlockDepsEmitMode.PerBlock`) listing what the // block's main content uses: a dynamic `import()` for a lazy diff --git a/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs index aeead3d56..a68985d96 100644 --- a/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs +++ b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs @@ -1404,7 +1404,7 @@ export class My_TemplateCmp { ); let code = &result.code; let compact = compact(code); - assert!(compact.contains("constFoo_Defer_1_DepsFn=()=>[LazyCmp];"), "{code}"); + assert!(compact.contains("constFoo_Defer_1_DepsFn=()=>[import(\"./lazy\")"), "{code}"); assert!(compact.contains("ɵɵdefer(1,0,Foo_Defer_1_DepsFn)"), "{code}"); assert!( compact.contains("constFoo_Template_Defer_1_DepsFn=()=>[import(\"./other\")"), @@ -1412,7 +1412,8 @@ export class My_TemplateCmp { ); assert!(compact.contains("ɵɵdefer(1,0,Foo_Template_Defer_1_DepsFn)"), "{code}"); assert!( - compact.contains("constMyCmp_Conditional_0_Template_Defer_1_DepsFn=()=>[LazyCmp];"), + compact + .contains("constMyCmp_Conditional_0_Template_Defer_1_DepsFn=()=>[import(\"./lazy\")"), "{code}" ); assert_eq!(code.matches("const Foo_Defer_1_DepsFn").count(), 1, "{code}"); @@ -1547,3 +1548,146 @@ export class C {} assert!(compact.contains("constC_Defer_2_DepsFn=()=>[Dep];"), "{code}"); assert!(compact.contains("ɵɵdefer(2,1,C_Defer_2_DepsFn);"), "{code}"); } + +// --- ngc's `DeferredSymbolTracker` over the whole file: the `imports` +// entries of every component that defers a class don't keep its import --- + +/// Two components listing `LazyCmp` in `imports`, the second after `app-b`'s +/// template. +fn two_parents(lazy_template_b: &str, rest: &str) -> String { + format!( + r#" +import {{ Component }} from '@angular/core'; +import {{ LazyCmp }} from './lazy'; + +@Component({{ selector: 'app-a', imports: [LazyCmp], template: '@defer {{ }}' }}) +export class ParentA {{}} + +@Component({{ selector: 'app-b', imports: [LazyCmp], template: '{lazy_template_b}' }}) +export class ParentB {{}} +{rest}"# + ) +} + +#[test] +fn import_deferred_by_two_components_is_lazy_loaded() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = + transform(&dir, &two_parents("@defer (on idle) { } @placeholder { p }", "")); + let code = &result.code; + let compact = compact(code); + assert!( + compact.contains("constParentA_Defer_1_DepsFn=()=>[import(\"./lazy\")") + && compact.contains("constParentB_Defer_2_DepsFn=()=>[import(\"./lazy\")"), + "Both components must load `LazyCmp` lazily. Output:\n{code}" + ); + assert!(!code.contains("import { LazyCmp }"), "{code}"); + assert!( + compact.contains("setClassMetadataAsync(ParentA,()=>[import(\"./lazy\")") + && compact.contains("setClassMetadataAsync(ParentB,()=>[import(\"./lazy\")"), + "{code}" + ); +} + +#[test] +fn import_used_eagerly_by_another_component_stays_eager() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = transform(&dir, &two_parents("", "")); + let code = &result.code; + assert_eager(code, "./lazy"); + assert!(code.contains("import { LazyCmp }"), "{code}"); +} + +#[test] +fn import_listed_by_another_component_that_does_not_use_it_stays_eager() { + // ngc marks an `imports` entry only for a component whose `@defer` + // blocks use the class. + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = transform(&dir, &two_parents("@defer { nothing }", "")); + let code = &result.code; + assert_eager(code, "./lazy"); + assert!(code.contains("import { LazyCmp }"), "{code}"); +} + +#[test] +fn import_deferred_by_two_components_and_referenced_elsewhere_stays_eager() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = + transform(&dir, &two_parents("@defer { }", "export const LAZY = LazyCmp;\n")); + let code = &result.code; + assert_eager(code, "./lazy"); + assert!(code.contains("import { LazyCmp }"), "{code}"); +} + +#[test] +fn import_deferred_by_two_components_and_listed_by_a_module_stays_eager() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = transform( + &dir, + &two_parents( + "@defer { }", + "import { NgModule } from '@angular/core';\n\ + @NgModule({ imports: [LazyCmp] })\nexport class LazyModule {}\n", + ), + ); + let code = &result.code; + assert_eager(code, "./lazy"); + assert!(code.contains("import { LazyCmp }"), "{code}"); +} + +#[test] +fn specifiers_deferred_by_different_components_remove_the_declaration() { + let dir = TempDir::new().unwrap(); + two_cmps(&dir); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { LazyCmp, EagerCmp } from './cmps'; + +@Component({ selector: 'app-a', imports: [LazyCmp], template: '@defer { }' }) +export class ParentA {} + +@Component({ selector: 'app-b', imports: [EagerCmp], template: '@defer { }' }) +export class ParentB {} +"#, + ); + let code = &result.code; + assert_lazy(code, "./cmps", "LazyCmp"); + assert_lazy(code, "./cmps", "EagerCmp"); + // A declaration kept whole is printed with its own quotes. + assert!(!code.contains("from './cmps'") && !code.contains("from \"./cmps\""), "{code}"); +} + +#[test] +fn class_two_imports_entries_name_is_loaded_once() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { LazyCmp } from './lazy'; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp, LazyCmp], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + let code = &result.code; + assert!(!code.contains("import { LazyCmp }"), "{code}"); + assert!( + compact(code).contains( + "setClassMetadataAsync(Parent,()=>[import(\"./lazy\").then((m)=>m.LazyCmp)],(LazyCmp)=>" + ), + "{code}" + ); +} From 8bcb1e07e689142dddea09692e49719489732c91 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 29 Oct 2026 12:22:11 +0800 Subject: [PATCH 07/11] fix(emitter): parenthesize a raw arrow factory when it is called A block-body arrow factory the converter keeps as raw source (e.g. one that declares a `const`) was called bare inside the `@Service` / `@Injectable` factory wrapper: `return () => {...}();` and `factory:() =>() => {...}()`, a SyntaxError. An async arrow kept its syntax but called its own body instead of the factory. ngtsc builds `meta.useFactory.callFn([])` over a WrappedNodeExpr, and `ts.factory.createCallExpression` runs the callee through `parenthesizeLeftSideOfAccess`, which wraps anything that is not a left-hand-side expression. Port that rule for `RawSource` callees in the emitter: arrows (and other non-LHS source) get parens; names, member access, function and class expressions stay bare, like ngtsc. Co-Authored-By: Claude Opus 5.5 --- .../src/output/emitter.rs | 109 +++++++++++++++++- .../tests/integration_test.rs | 33 ++++++ 2 files changed, 139 insertions(+), 3 deletions(-) diff --git a/crates/oxc_angular_compiler/src/output/emitter.rs b/crates/oxc_angular_compiler/src/output/emitter.rs index cb1e289c4..fa96a2fb8 100644 --- a/crates/oxc_angular_compiler/src/output/emitter.rs +++ b/crates/oxc_angular_compiler/src/output/emitter.rs @@ -843,9 +843,16 @@ impl JsEmitter { if e.pure { ctx.print("/*@__PURE__*/ "); } - // Wrap arrow functions in parens for IIFE - let should_parenthesize = - matches!(e.fn_expr.as_ref(), OutputExpression::ArrowFunction(_)); + // Wrap arrow functions in parens for IIFE. A `RawSource` callee + // is user code ngtsc emits verbatim (WrappedNodeExpr); TS's + // printer parenthesizes it unless it is a left-hand-side + // expression, so `() => {...}` becomes `(() => {...})()` + // while `makeIt` and `function () {...}` stay bare. + let should_parenthesize = match e.fn_expr.as_ref() { + OutputExpression::ArrowFunction(_) => true, + OutputExpression::RawSource(raw) => !is_left_hand_side_source(&raw.source), + _ => false, + }; if should_parenthesize { ctx.print("("); } @@ -1184,6 +1191,57 @@ fn emits_leading_brace(expr: &OutputExpression<'_>) -> bool { } } +/// Whether raw JavaScript source parses as a left-hand-side expression, one +/// that can be called without parentheses. Ported from TypeScript's +/// `parenthesizeLeftSideOfAccess` (via `isLeftHandSideExpression`), which +/// `ts.factory.createCallExpression` applies to the callee when ngtsc +/// translates `meta.useFactory.callFn([])` over a WrappedNodeExpr. A `new` +/// without an argument list is not one: `new X()` would call the class. +/// Source that does not parse is treated as needing parentheses, which is +/// always valid. +fn is_left_hand_side_source(source: &str) -> bool { + use oxc_ast::ast::Expression; + use oxc_span::GetSpan; + + let allocator = oxc_allocator::Allocator::default(); + let Ok(expr) = + oxc_parser::Parser::new(&allocator, source, oxc_span::SourceType::mjs()).parse_expression() + else { + return false; + }; + match &expr { + // `new X(a)` has an argument list after its callee; `new X` does not. + Expression::NewExpression(new_expr) => { + source[new_expr.callee.span().end as usize..].contains('(') + } + Expression::Identifier(_) + | Expression::BooleanLiteral(_) + | Expression::NullLiteral(_) + | Expression::NumericLiteral(_) + | Expression::BigIntLiteral(_) + | Expression::RegExpLiteral(_) + | Expression::StringLiteral(_) + | Expression::TemplateLiteral(_) + | Expression::ArrayExpression(_) + | Expression::ObjectExpression(_) + | Expression::FunctionExpression(_) + | Expression::ClassExpression(_) + | Expression::ThisExpression(_) + | Expression::Super(_) + | Expression::ParenthesizedExpression(_) + | Expression::CallExpression(_) + | Expression::ChainExpression(_) + | Expression::TaggedTemplateExpression(_) + | Expression::ImportMeta(_) + | Expression::NewTarget(_) + | Expression::ImportExpression(_) + | Expression::ComputedMemberExpression(_) + | Expression::StaticMemberExpression(_) + | Expression::PrivateFieldExpression(_) => true, + _ => false, + } +} + fn binary_operator_to_str(op: BinaryOperator) -> &'static str { match op { BinaryOperator::Equals => "==", @@ -1545,6 +1603,51 @@ mod tests { assert_eq!(emitter.emit_expression(&expr), "myVar"); } + #[test] + fn test_emit_raw_source_callee_parens() { + // A raw user expression called with no args, as in `useFactory`. + // Arrows (and other non-left-hand-side expressions) need parens, + // like ngtsc's TS printer adds; names, member access, function + // expressions and already parenthesized source stay as written. + let emitter = JsEmitter::new(); + let alloc = Allocator::default(); + let call = |source: &str| { + let raw = OutputExpression::RawSource(Box::new_in( + crate::output::ast::RawSourceExpr { + source: Ident::from(alloc.alloc_str(source)), + source_span: None, + }, + &&alloc, + )); + let expr = OutputExpression::InvokeFunction(Box::new_in( + crate::output::ast::InvokeFunctionExpr { + fn_expr: Box::new_in(raw, &&alloc), + args: oxc_allocator::Vec::new_in(&&alloc), + pure: false, + optional: false, + source_span: None, + }, + &&alloc, + )); + emitter.emit_expression(&expr) + }; + assert_eq!( + call("() => { const q = 1; return q; }"), + "(() => { const q = 1; return q; })()" + ); + assert_eq!(call("() => ({ v: 2 })"), "(() => ({ v: 2 }))()"); + assert_eq!(call("async () => { return 4; }"), "(async () => { return 4; })()"); + assert_eq!(call("x => x"), "(x => x)()"); + assert_eq!(call("a ? b : c"), "(a ? b : c)()"); + assert_eq!(call("new Foo"), "(new Foo)()"); + assert_eq!(call("function () { return 3; }"), "function () { return 3; }()"); + assert_eq!(call("(() => 5)"), "(() => 5)()"); + assert_eq!(call("makeIt"), "makeIt()"); + assert_eq!(call("Factories.make"), "Factories.make()"); + assert_eq!(call("new Foo(1)"), "new Foo(1)()"); + assert_eq!(call("class { v = 1; }"), "class { v = 1; }()"); + } + #[test] fn test_escape_string() { // Uses double quotes to match Angular's output style diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index b3c518913..169dfa033 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -16079,6 +16079,39 @@ export class CounterService {} insta::assert_snapshot!("aot_service_decorator_inline_arrow_factory", result.code); } +#[test] +fn test_raw_arrow_factory_is_parenthesized_when_called() { + // A block-body arrow the converter keeps as raw source (it declares a + // `const`) is called inside the factory wrapper. ngtsc emits + // `(() => {...})()`; a bare `() => {...}()` is a SyntaxError. + let code = compile_to_valid_js( + r" +import { Injectable, InjectionToken, Service } from '@angular/core'; +export const DEP = new InjectionToken('DEP'); +@Service({ factory: () => { const q = 1; return { v: q }; } }) +export class S1 {} +@Service({ factory: async () => ({ v: 2 }) }) +export class S2 {} +@Injectable({ providedIn: 'root', useFactory: () => { const q = 3; return { v: q }; } }) +export class I1 {} +@Injectable({ providedIn: 'root', useFactory: (d) => { const q = d + 4; return { v: q }; }, deps: [DEP] }) +export class I2 {} +@Injectable({ providedIn: 'root', useFactory: function () { const q = 5; return { v: q }; } }) +export class I3 {} +", + ); + for expected in [ + "return (() => { const q = 1; return { v: q }; })();", + "return (async () => ({ v: 2 }))();", + "factory:() =>(() => { const q = 3; return { v: q }; })()", + "((d) => { const q = d + 4; return { v: q }; })(i0.ɵɵinject(DEP))", + // ngtsc leaves a called function expression bare. + "factory:() =>function () { const q = 5; return { v: q }; }()", + ] { + assert!(code.contains(expected), "Missing `{expected}` in:\n{code}"); + } +} + #[test] fn test_aot_service_decorator_version_gated() { // Targeting Angular < 22 should surface a diagnostic and leave the From 1a577e69da69c152a7a72532b12cf93ce86b41fa Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 29 Oct 2026 12:24:58 +0800 Subject: [PATCH 08/11] fix(component): emit the selector attrs field only before Angular v22 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ngtsc 22 has no `attrs` on ɵcmp: angular/angular@d99ab0e040 ("fix(compiler): stop generating unused field") removed it from compileComponentFromMetadata in v22.0.0-next.4, and no runtime reads it. We still pooled the first selector's attributes, which also took a `_cN` index and shifted every later constant. Pool and emit it only when angularVersion is below 22.0.0-next.4 (19.x to 21.x still emit it), and pool it after the query predicates, as v21.2's compileComponentFromMetadata does after baseDirectiveFields. Co-Authored-By: Claude Opus 5.5 --- .../src/component/definition.rs | 5 +- .../src/component/metadata.rs | 37 ++++++++ .../src/component/transform.rs | 22 +++-- .../tests/const_pool_long_strings_test.rs | 20 +++- .../tests/integration_test.rs | 94 +++++++++++-------- ...n_test__selector_attrs_const_emission.snap | 5 +- 6 files changed, 128 insertions(+), 55 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/definition.rs b/crates/oxc_angular_compiler/src/component/definition.rs index e116351c5..56c023df1 100644 --- a/crates/oxc_angular_compiler/src/component/definition.rs +++ b/crates/oxc_angular_compiler/src/component/definition.rs @@ -56,7 +56,7 @@ pub struct ComponentDefinitions<'a> { /// * `job` - The compilation job with template compilation results /// * `template_fn` - The compiled template function /// * `host_binding_result` - Optional host binding compilation result (function, hostAttrs, hostVars) -/// * `attrs_ref` - Optional pre-pooled attrs constant reference (pooled before template compilation) +/// * `attrs_ref` - Optional pre-pooled attrs constant reference (pooled before template compilation, pre-v22 only) /// * `view_query_fn` - Optional view query function (with pre-pooled predicates) /// /// # Returns @@ -282,7 +282,8 @@ fn generate_cmp_definition<'a>( // 14. attrs: ["class", "..."] - only if first selector has attributes // This is optional and only included if the first selector specifies attributes. - // Ported from Angular compiler.ts lines 195-212. + // Ported from Angular compiler.ts (v21.2.x); v22.0.0-next.4 dropped the field + // (angular/angular@d99ab0e040), so attrs_ref is only set for older versions. // The attrs_ref is pre-pooled BEFORE template compilation to ensure correct constant ordering. // TypeScript Angular adds attrs to the pool BEFORE template ingestion/compilation. if let Some(attrs) = attrs_ref { diff --git a/crates/oxc_angular_compiler/src/component/metadata.rs b/crates/oxc_angular_compiler/src/component/metadata.rs index 46617b4f9..e828e4b01 100644 --- a/crates/oxc_angular_compiler/src/component/metadata.rs +++ b/crates/oxc_angular_compiler/src/component/metadata.rs @@ -219,6 +219,22 @@ impl AngularVersion { self.is_at_least(22, 0, 0) } + /// Check if this version emits the `attrs` field on `ɵcmp` (before + /// v22.0.0-next.4). + /// + /// Until then `compileComponentFromMetadata` pooled the attributes of the + /// component's first selector (`[bitBadge]` gives `["bitBadge", ""]`) and set + /// `attrs` to that constant. No runtime ever read it, and + /// `angular/angular@d99ab0e040` ("fix(compiler): stop generating unused + /// field") removed it in v22.0.0-next.4; it was never backported. The + /// constant still takes a `_cN` index on older versions. + pub fn emits_selector_attrs(&self) -> bool { + if self.major == 22 && self.minor == 0 && self.patch == 0 { + return matches!(self.prerelease, Some(prerelease) if prerelease < Prerelease::Next(4)); + } + self.major < 22 + } + /// Check if this version compiles component templates in DOM-only mode /// (v20.1.0-next.3+). /// @@ -1196,4 +1212,25 @@ mod tests { assert!(!version.uses_shadow_css_polyfills(), "{version:?}"); } } + + #[test] + fn selector_attrs_end_at_22_0_0_next_4() { + let parse = |s| AngularVersion::parse(s).unwrap(); + + for version in + [parse("19.2.20"), parse("20.3.34"), parse("21.2.25"), parse("22.0.0-next.3")] + { + assert!(version.emits_selector_attrs(), "{version:?}"); + } + + for version in [ + parse("22.0.0-next.4"), + parse("22.0.0-rc.0"), + parse("22.0.0"), + parse("22.2.2"), + AngularVersion::new(u32::MAX, 0, 0), + ] { + assert!(!version.emits_selector_attrs(), "{version:?}"); + } + } } diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 9da7837e5..9c32b3cd0 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -6275,10 +6275,16 @@ fn compile_component_full<'a>( job.pool.continue_from(index); } - // Pool attrs const if first selector has attributes, after the host - // bindings and before the template. - // See: packages/compiler/src/render3/view/compiler.ts lines 192-212 - let attrs_ref = pool_selector_attrs(allocator, &mut job, metadata); + // Pool the `attrs` const when the first selector has attributes, after + // the host bindings and before the template. Only Angular before + // v22.0.0-next.4 adds it, in `compileComponentFromMetadata` after + // `baseDirectiveFields`. + // See: packages/compiler/src/render3/view/compiler.ts (v21.2.x) + let attrs_ref = if options.angular_version.is_some_and(|v| v.emits_selector_attrs()) { + pool_selector_attrs(allocator, &mut job, metadata) + } else { + None + }; let compiled = compile_template(&mut job); @@ -7543,11 +7549,11 @@ fn convert_host_metadata_input_to_host_metadata<'a>( /// Pool selector attrs constant BEFORE template compilation. /// /// This matches TypeScript Angular's behavior where the attrs constant is added to -/// the constant pool BEFORE template ingestion and compilation. By pooling it first, -/// attrs gets the first available constant index (_c0), ensuring the correct constant -/// ordering in the output. +/// the constant pool after the host bindings' constants and BEFORE template ingestion and +/// compilation, ensuring the correct constant ordering in the output. Only Angular +/// before v22.0.0-next.4 emits it (see [`crate::AngularVersion::emits_selector_attrs`]). /// -/// See: packages/compiler/src/render3/view/compiler.ts lines 192-212 +/// See: packages/compiler/src/render3/view/compiler.ts (v21.2.x) fn pool_selector_attrs<'a>( allocator: &'a Allocator, job: &mut crate::pipeline::compilation::ComponentCompilationJob<'a>, diff --git a/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs b/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs index 8e8f2b318..246366a3f 100644 --- a/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs +++ b/crates/oxc_angular_compiler/tests/const_pool_long_strings_test.rs @@ -5,7 +5,8 @@ use oxc_allocator::Allocator; use oxc_angular_compiler::{ - ComponentHmrUpdateInput, TransformOptions, compile_component_hmr_update, transform_angular_file, + AngularVersion, ComponentHmrUpdateInput, TransformOptions, compile_component_hmr_update, + transform_angular_file, }; /// 76 chars, over the 50-char threshold. @@ -184,15 +185,26 @@ export class C { @ViewChild('x') q: any; @ViewChild('y') r: any; } #[test] fn literals_whose_strings_join_alike_stay_apart() { // `["a", "b,str:c,str:d"]` and `["a", "b", "c", "d"]` are different arrays, so B must not - // read A's `attrs` constant. - let code = squash(&transform( + // read A's `attrs` constant. Only Angular before v22.0.0-next.4 emits `attrs`. + let allocator = Allocator::default(); + let options = TransformOptions { + angular_version: Some(AngularVersion::new(21, 2, 0)), + ..Default::default() + }; + let result = transform_angular_file( + &allocator, + "test.ts", "import {Component} from '@angular/core'; @Component({selector: '[a=\"b,str:c,str:d\"]', template: 'x', standalone: true}) export class A {} @Component({selector: '[a=b][c=d]', template: 'y', standalone: true}) export class B {} ", - )); + Some(&options), + None, + ); + assert!(!result.has_errors(), "{:?}", result.diagnostics); + let code = squash(&result.code); assert!(code.contains(r#"const_c0=["a","b,str:c,str:d"];"#), "{code}"); assert!(code.contains(r#"const_c1=["a","b","c","d"];"#), "{code}"); assert!(code.contains("type:B,selectors:[[\"\",\"a\",\"b\",\"c\",\"d\"]],attrs:_c1"), "{code}"); diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 169dfa033..0752e749b 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -2870,23 +2870,19 @@ fn test_multiple_same_name_references_in_for() { insta::assert_snapshot!("multiple_same_name_references_in_for", js); } -/// Test that attrs const is emitted for selectors with attributes. +/// Test that no attrs const is emitted for selectors with attributes. /// -/// When a selector like `span[bitBadge]` is used, Angular extracts the attribute -/// selector into a constant array that is referenced in the component definition. -/// -/// Expected output: +/// Angular v22.0.0-next.4 stopped emitting the unused `attrs` field +/// (angular/angular@d99ab0e040), so `span[bitBadge]` takes no constant and +/// the ng-content selectors get `_c0`, as in ngtsc 22.2: /// ```javascript -/// const _c0 = ["bitBadge", ""]; // attrs const +/// const _c0 = ["*"]; /// BadgeComponent.ɵcmp = ɵɵdefineComponent({ -/// attrs: _c0, +/// selectors: [["span", "bitBadge", ""]], +/// ngContentSelectors: _c0, /// ... /// }) /// ``` -/// -/// This was a bug where the attrs const was added to the pool during -/// `generate_component_definitions` but never emitted because `compile_template` -/// had already drained the pool to `declarations`. #[test] fn test_selector_attrs_const_emission() { let allocator = Allocator::default(); @@ -2906,24 +2902,55 @@ export class BadgeComponent {} assert_eq!(result.component_count, 1); assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); - // The declarations_js should contain the attrs const - // The const should look like: const _cN = ["bitBadge",""] - // Note: no space after comma in the emitter output - assert!( - result.code.contains(r#"["bitBadge",""]"#), - "Should emit attrs const with selector attribute. Got:\n{}", - result.code - ); + let code: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + assert!(!code.contains("attrs:"), "Should not emit attrs. Got:\n{}", result.code); + assert!(!code.contains(r#"["bitBadge",""]"#), "Should not pool attrs. Got:\n{}", result.code); + assert!(code.contains(r#"const_c0=["*"];"#), "{}", result.code); + assert!(code.contains("ngContentSelectors:_c0,"), "{}", result.code); + + insta::assert_snapshot!("selector_attrs_const_emission", result.code); +} - // The ɵcmp definition should reference the attrs const - // Note: no space after colon in the emitter output +/// Angular before v22.0.0-next.4 emits `attrs` with the first selector's +/// attributes. `compileComponentFromMetadata` pools it after the query +/// predicates (`baseDirectiveFields`) and before the template, and shares it +/// with a later class of the file (`forceShared`). ngtsc 21.2.25 gives: +/// ```javascript +/// const _c0 = ["y"]; +/// const _c1 = ["x"]; +/// const _c2 = ["a", ""]; +/// const _c3 = ["*"]; +/// ``` +#[test] +fn test_selector_attrs_before_v22() { + let allocator = Allocator::default(); + let source = r" +import { Component, ContentChild, ViewChild } from '@angular/core'; + +@Component({ selector: '[a]', template: '', standalone: true }) +export class A { @ViewChild('x') q: any; @ContentChild('y') c: any; } + +@Component({ selector: 'b[a], c', template: '', standalone: true }) +export class B {} +"; + let options = ComponentTransformOptions { + angular_version: Some(AngularVersion::new(21, 2, 0)), + ..ComponentTransformOptions::default() + }; + + let result = transform_angular_file(&allocator, "a.ts", source, Some(&options), None); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + let code: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); assert!( - result.code.contains("attrs:_c"), - "Should reference attrs const in component definition. Got:\n{}", + code.contains(r#"const_c0=["y"];const_c1=["x"];const_c2=["a",""];const_c3=["*"];"#), + "{}", result.code ); - - insta::assert_snapshot!("selector_attrs_const_emission", result.code); + assert!(code.contains("ɵɵcontentQuery(dirIndex,_c0,5)"), "{}", result.code); + assert!(code.contains("ɵɵviewQuery(_c1,5)"), "{}", result.code); + assert!(code.contains("},attrs:_c2,ngContentSelectors:_c3,"), "{}", result.code); + assert!(code.contains(r#"selectors:[["b","a",""],["c"]],attrs:_c2,"#), "{}", result.code); } // ============================================================================ @@ -3584,13 +3611,8 @@ export class MatMiniFabButton {} ); } - // Both components should have attrs referencing _c constants (from selector) - let attrs_pattern = "attrs:_c"; - let attrs_count = code.matches(attrs_pattern).count(); - assert!( - attrs_count >= 2, - "Expected at least 2 attrs references (one per component), found {attrs_count}. Output:\n{code}" - ); + // Angular v22 emits no `attrs` for the attribute selectors. + assert!(!code.contains("attrs:"), "Should not emit attrs. Output:\n{code}"); } /// Test that multiple components sharing the same template in the same file @@ -3786,12 +3808,8 @@ export class MatMiniFabButton { "Duplicate const declarations found: {duplicates:?}. Full output:\n{code}" ); - // Each component should have its own constants for attrs and ngContentSelectors - let attrs_count = code.matches("attrs:_c").count(); - assert!( - attrs_count >= 2, - "Expected at least 2 attrs references (one per component), found {attrs_count}" - ); + // Angular v22 emits no `attrs` for the attribute selectors. + assert!(!code.contains("attrs:"), "Should not emit attrs. Output:\n{code}"); } /// Test three components in the same file to ensure constant pool index is correctly diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__selector_attrs_const_emission.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__selector_attrs_const_emission.snap index 34d3341b1..d66f4fd61 100644 --- a/crates/oxc_angular_compiler/tests/snapshots/integration_test__selector_attrs_const_emission.snap +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__selector_attrs_const_emission.snap @@ -6,15 +6,14 @@ expression: result.code import { Component } from '@angular/core'; import * as i0 from '@angular/core'; -const _c0 = ["bitBadge",""]; -const _c1 = ["*"]; +const _c0 = ["*"]; export class BadgeComponent { static ɵfac = function BadgeComponent_Factory(__ngFactoryType__) { return new (__ngFactoryType__ || BadgeComponent)(); }; static ɵcmp = /*@__PURE__*/ i0.ɵɵdefineComponent({type:BadgeComponent,selectors:[["span","bitBadge", - ""]],attrs:_c0,ngContentSelectors:_c1,decls:1,vars:0,template:function BadgeComponent_Template(rf, + ""]],ngContentSelectors:_c0,decls:1,vars:0,template:function BadgeComponent_Template(rf, ctx) { if ((rf & 1)) { i0.ɵɵprojectionDef(); From aa89ff3ff0280895c9319c640714ed1f09bcbb44 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 29 Oct 2026 13:26:54 +0800 Subject: [PATCH 09/11] test(defer): expect lazy DepsFns to be shared across the file's classes With an import every component of the file defers now loaded lazily, two classes whose blocks load the same thing build the same DepsFn, and the file-wide pool hands the second one the first's name. ngtsc 22.2.3 emits the same. Co-Authored-By: Claude Opus 5.5 --- .../tests/defer_implicit_imports_test.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs index a68985d96..d72508117 100644 --- a/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs +++ b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs @@ -1411,11 +1411,10 @@ export class My_TemplateCmp { "{code}" ); assert!(compact.contains("ɵɵdefer(1,0,Foo_Template_Defer_1_DepsFn)"), "{code}"); - assert!( - compact - .contains("constMyCmp_Conditional_0_Template_Defer_1_DepsFn=()=>[import(\"./lazy\")"), - "{code}" - ); + // `My_TemplateCmp`'s block loads the same thing as `Foo`'s, and ngc's + // file-wide pool hands it `Foo_Defer_1_DepsFn`. + assert_eq!(compact.matches("ɵɵdefer(1,0,Foo_Defer_1_DepsFn)").count(), 2, "{code}"); + assert!(!compact.contains("MyCmp_Conditional_0_Template_Defer_1_DepsFn"), "{code}"); assert_eq!(code.matches("const Foo_Defer_1_DepsFn").count(), 1, "{code}"); } @@ -1579,8 +1578,8 @@ fn import_deferred_by_two_components_is_lazy_loaded() { let compact = compact(code); assert!( compact.contains("constParentA_Defer_1_DepsFn=()=>[import(\"./lazy\")") - && compact.contains("constParentB_Defer_2_DepsFn=()=>[import(\"./lazy\")"), - "Both components must load `LazyCmp` lazily. Output:\n{code}" + && compact.contains("ɵɵdefer(2,0,ParentA_Defer_1_DepsFn,null,1);"), + "Both components must load `LazyCmp` lazily, through one shared function. Output:\n{code}" ); assert!(!code.contains("import { LazyCmp }"), "{code}"); assert!( From a9aecad3a2fea82c72fd737fc062b164e9dac2e7 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 29 Oct 2026 13:29:28 +0800 Subject: [PATCH 10/11] test(component): pin pre-v22 attrs after the host bindings' constants v21.2's compileComponentFromMetadata pools attrs after baseDirectiveFields, which compiles the host bindings. Co-Authored-By: Claude Opus 5.5 --- .../tests/integration_test.rs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 0752e749b..096d8530a 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -2953,6 +2953,40 @@ export class B {} assert!(code.contains(r#"selectors:[["b","a",""],["c"]],attrs:_c2,"#), "{}", result.code); } +/// v21's `compileComponentFromMetadata` pools `attrs` after `baseDirectiveFields`, +/// so after the host bindings' constants and before the template's. +#[test] +fn test_selector_attrs_before_v22_follow_host_constants() { + let allocator = Allocator::default(); + let source = r#" +import { Component } from '@angular/core'; + +@Component({ + selector: 'x[role=a]', + host: { '[class]': '{on: on, off: !on}' }, + template: '', +}) +export class X { on = true; } +"#; + let options = ComponentTransformOptions { + angular_version: Some(AngularVersion::new(21, 2, 0)), + ..ComponentTransformOptions::default() + }; + + let result = transform_angular_file(&allocator, "x.ts", source, Some(&options), None); + + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + let code: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + code.contains( + r#"const_c0=(a0,a1)=>({on:a0,off:a1});const_c1=["role","a"];const_c2=(a0,a1)=>({a:a0,b:a1});"# + ), + "{}", + result.code + ); + assert!(code.contains("},attrs:_c1,decls:1,"), "{}", result.code); +} + // ============================================================================ // Multi-Component File Tests // ============================================================================ From 6bb4adb58afcd0112d21127b5ce5b81e727a8ca9 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 29 Oct 2026 13:36:26 +0800 Subject: [PATCH 11/11] fix(pure-functions): escape strings in pure function keys Two bindings like [k, 'a","b'] and [k, 'a', 'b'] built the same key, so the second reused the first's pure function and got a two-entry array. With the pool now shared across classes this also reached from one component to the next. Quote strings with escaping, as expression_to_key already does. ngtsc's GenericKeyFn doesn't escape either and emits the same wrong sharing; this only changes output for such strings. Co-Authored-By: Claude Opus 5.5 --- .../phases/pure_function_extraction.rs | 22 ++++++++----- .../tests/const_pool_file_wide_test.rs | 31 +++++++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/pure_function_extraction.rs b/crates/oxc_angular_compiler/src/pipeline/phases/pure_function_extraction.rs index 9d1d24391..c35946104 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/pure_function_extraction.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/pure_function_extraction.rs @@ -178,8 +178,7 @@ fn generate_expression_key(expr: &IrExpression<'_>) -> String { if *is_spread { format!("...{}", value_key) } else { - let key_str = - if *quoted { format!("\"{}\"", key) } else { key.to_string() }; + let key_str = if *quoted { quote_key(key) } else { key.to_string() }; format!("{}:{}", key_str, value_key) } }) @@ -240,6 +239,13 @@ fn generate_expression_key(expr: &IrExpression<'_>) -> String { } } +/// `s` in quotes, escaped, so no string can read as several entries in a +/// key (`'a","b'` against `'a', 'b'`). Angular's `GenericKeyFn` doesn't +/// escape and shares one pure function between such bindings. +fn quote_key(s: &str) -> String { + format!("{s:?}") +} + /// Generate a key for an Angular AST expression. /// /// This matches the key format from Angular's `GenericKeyFn.keyOf` in `constant_pool.ts`. @@ -247,9 +253,9 @@ fn generate_angular_expression_key(expr: &crate::ast::expression::AngularExpress use crate::ast::expression::{AngularExpression, LiteralValue}; match expr { - // LiteralExpr with string value -> `"${value}"` + // LiteralExpr with string value -> `"${value}"`, escaped (see `quote_key`) AngularExpression::LiteralPrimitive(lit) => match &lit.value { - LiteralValue::String(s) => format!("\"{}\"", s), + LiteralValue::String(s) => quote_key(s), LiteralValue::Number(n) => { // Match JavaScript's String() behavior for numbers if n.is_nan() { @@ -297,7 +303,7 @@ fn generate_angular_expression_key(expr: &crate::ast::expression::AngularExpress .filter_map(|(key, value)| { if let LiteralMapKey::Property(prop) = key { let key_str = if prop.quoted { - format!("\"{}\"", prop.key) + quote_key(prop.key.as_str()) } else { prop.key.to_string() }; @@ -402,7 +408,7 @@ fn generate_angular_expression_key(expr: &crate::ast::expression::AngularExpress } AngularExpression::Interpolation(interp) => { - let strings: Vec<_> = interp.strings.iter().map(|s| format!("\"{}\"", s)).collect(); + let strings: Vec<_> = interp.strings.iter().map(|s| quote_key(s)).collect(); let exprs: Vec<_> = interp.expressions.iter().map(generate_angular_expression_key).collect(); format!("interp([{}],[{}])", strings.join(","), exprs.join(",")) @@ -415,7 +421,7 @@ fn generate_angular_expression_key(expr: &crate::ast::expression::AngularExpress } AngularExpression::TemplateLiteral(tl) => { - let elements: Vec<_> = tl.elements.iter().map(|e| format!("\"{}\"", e.text)).collect(); + let elements: Vec<_> = tl.elements.iter().map(|e| quote_key(e.text.as_str())).collect(); let exprs: Vec<_> = tl.expressions.iter().map(generate_angular_expression_key).collect(); format!("template([{}],[{}])", elements.join(","), exprs.join(",")) @@ -424,7 +430,7 @@ fn generate_angular_expression_key(expr: &crate::ast::expression::AngularExpress AngularExpression::TaggedTemplateLiteral(ttl) => { let tag = generate_angular_expression_key(&ttl.tag); let elements: Vec<_> = - ttl.template.elements.iter().map(|e| format!("\"{}\"", e.text)).collect(); + ttl.template.elements.iter().map(|e| quote_key(e.text.as_str())).collect(); let exprs: Vec<_> = ttl.template.expressions.iter().map(generate_angular_expression_key).collect(); format!("tagged({},template([{}],[{}]))", tag, elements.join(","), exprs.join(",")) diff --git a/crates/oxc_angular_compiler/tests/const_pool_file_wide_test.rs b/crates/oxc_angular_compiler/tests/const_pool_file_wide_test.rs index f25468a7a..95f4d5df0 100644 --- a/crates/oxc_angular_compiler/tests/const_pool_file_wide_test.rs +++ b/crates/oxc_angular_compiler/tests/const_pool_file_wide_test.rs @@ -240,3 +240,34 @@ export class Cmp { w = 1; h = 2; f(x: any) { return ''; } } assert_eq!(squashed.matches(",_c0,ctx.w,ctx.h)").count(), 2, "{}", linked.code); assert_eq!(squashed.matches(",arrowFn0,ctx)").count(), 2, "{}", linked.code); } + +#[test] +fn pure_functions_whose_strings_join_alike_stay_apart() { + // `'a","b'` must not read as the two entries `'a', 'b'`. Angular's + // `GenericKeyFn` doesn't escape strings and gives both bindings one + // function, so `B` would get a two-entry array; Oxc keeps them apart, in + // one class as across classes. + let code = transform( + r#" +import { Component } from '@angular/core'; + +@Component({ selector: 'a-a', template: '' }) +export class A { k = 1; } + +@Component({ + selector: 'b-b', + template: '', +}) +export class B { k = 1; } +"#, + ); + assert_eq!(constants(&code), ["_c0", "_c1"], "{code}"); + let squashed = squash(&code); + assert_eq!(value(&squashed, "_c0"), r#"(a0)=>[a0,"a\",\"b"]"#, "{code}"); + assert_eq!(value(&squashed, "_c1"), r#"(a0)=>[a0,"a","b"]"#, "{code}"); + assert!( + squashed.contains(r#"ɵɵdomProperty("title",i0.ɵɵpureFunction1(2,_c1,ctx.k))("id""#), + "{code}" + ); + assert_eq!(squashed.matches(",_c0,ctx.k)").count(), 2, "{code}"); +}