From 536e6f23dc855fc19553d20cb3349d46d87679ae Mon Sep 17 00:00:00 2001 From: Gabriel Silva Date: Tue, 20 Oct 2026 21:32:22 -0300 Subject: [PATCH 1/5] fix(defer): lazy-load imports used only inside @defer blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A standalone component or directive listed in `@Component.imports` and matched only inside the main content of `@defer` blocks was emitted as an eager dependency, so `@defer` did no code-splitting unless the symbol was moved to `deferredImports`. ngc's full compilation defers it implicitly. With cross-file analysis (`resolve_imported_values`, which the Vite plugin sets), the selector is read from the file the symbol is imported from and matched against the template. A symbol matched only inside `@defer` joins the per-component resolver, leaves `ɵɵgetComponentDepsFactory`, and its static import is removed. Placeholder, loading and error content count as eager. The import stays eager when the selector can't be read, the symbol is referenced elsewhere in the file, or under HMR. Pipes stay eager. Closes #313 --- .../src/component/cross_file_elision.rs | 127 +++++- .../src/component/defer_resolver.rs | 179 +++++++- .../src/component/import_elision.rs | 43 +- .../src/component/transform.rs | 93 +++- .../src/pipeline/selector.rs | 81 ++++ .../tests/defer_implicit_imports_test.rs | 413 ++++++++++++++++++ .../tests/integration_test.rs | 11 +- 7 files changed, 928 insertions(+), 19 deletions(-) create mode 100644 crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs diff --git a/crates/oxc_angular_compiler/src/component/cross_file_elision.rs b/crates/oxc_angular_compiler/src/component/cross_file_elision.rs index dd2b54c9a..da939ff48 100644 --- a/crates/oxc_angular_compiler/src/component/cross_file_elision.rs +++ b/crates/oxc_angular_compiler/src/component/cross_file_elision.rs @@ -19,8 +19,8 @@ use std::rc::Rc; use oxc_allocator::Allocator; use oxc_ast::ast::{ - BindingPattern, Declaration, ExportDefaultDeclarationKind, Expression, Statement, - TSNamespaceDeclarationBody, + BindingPattern, Class, Declaration, ExportDefaultDeclarationKind, Expression, + ObjectPropertyKind, Statement, TSNamespaceDeclarationBody, }; use oxc_parser::Parser; use oxc_resolver::{ @@ -115,6 +115,63 @@ impl ImportValueResolver for CrossFileResolver { } } +/// The selector of the class `local` declares in `file_path` (`"default"` for +/// `export default class`), when it is a standalone `@Component` or +/// `@Directive` whose `selector` and `standalone` are literals. +fn standalone_class_selector( + file_path: &str, + local: &str, + implicit_standalone: bool, +) -> Option { + let source = std::fs::read_to_string(file_path).ok()?; + let allocator = Allocator::default(); + let source_type = SourceType::from_path(file_path).unwrap_or_default(); + let parser_ret = Parser::new(&allocator, &source, source_type).parse(); + let program = &parser_ret.program; + + let is_local = |class: &Class| class.id.as_ref().is_some_and(|id| id.name == local); + let class = program.body.iter().find_map(|stmt| match stmt { + Statement::ClassDeclaration(class) if is_local(class) => Some(&**class), + Statement::ExportDeclaration(decl) => match &decl.declaration { + Declaration::ClassDeclaration(class) if is_local(class) => Some(&**class), + _ => None, + }, + Statement::ExportDefaultDeclaration(decl) if local == "default" => { + match &decl.declaration { + ExportDefaultDeclarationKind::ClassDeclaration(class) => Some(&**class), + _ => None, + } + } + _ => None, + })?; + + let consts = crate::directive::collect_string_consts(&allocator, program); + let decorator = ["Component", "Directive"].iter().find_map(|name| { + crate::directive::find_angular_class_decorator(&class.decorators, name, &consts) + })?; + let Expression::CallExpression(call) = &decorator.expression else { return None }; + let Some(Expression::ObjectExpression(object)) = + call.arguments.first().and_then(|arg| arg.as_expression()) + else { + return None; + }; + + let mut selector = None; + let mut standalone = implicit_standalone; + for property in &object.properties { + let ObjectPropertyKind::ObjectProperty(property) = property else { return None }; + let Some(key) = property.key.static_name() else { continue }; + if key == "selector" { + let Expression::StringLiteral(value) = &property.value else { return None }; + selector = Some(value.value.as_str()?.to_string()); + } else if key == "standalone" { + let Expression::BooleanLiteral(value) = &property.value else { return None }; + standalone = value.value; + } + } + selector.filter(|selector| standalone && !selector.trim().is_empty()) +} + impl CrossFileAnalyzer { /// Create a new cross-file analyzer. /// @@ -194,6 +251,31 @@ impl CrossFileAnalyzer { self.inner.resolve_export_value(import_source, import_name, from_file) } + /// The selector of the standalone `@Component` or `@Directive` class that + /// `import_name` is exported as from `import_source`, as `import_source` + /// is written in `from_file`, following named re-exports. Used to tell + /// whether a template matches the class, for `@defer` (#313). + /// + /// `None` when the import can't be followed to a class declared in a + /// source file (packages, `export *`, unresolvable paths), the class isn't + /// a standalone component or directive, or its `selector` or `standalone` + /// isn't a literal: the caller keeps the import eager. Files read are + /// recorded in [`Self::take_value_dependencies`]. + pub(crate) fn standalone_directive_selector( + &self, + import_source: &str, + import_name: &str, + from_file: &Path, + implicit_standalone: bool, + ) -> Option { + self.inner.standalone_directive_selector( + import_source, + import_name, + from_file, + implicit_standalone, + ) + } + /// The resolver the decorator evaluator reads `from_file`'s imported /// bindings through (see `TransformOptions::resolve_imported_values`). pub(crate) fn value_resolver(&self, from_file: PathBuf) -> CrossFileResolver { @@ -348,6 +430,47 @@ impl Inner { self.export_value_at(&resolved, import_name, 0) } + fn standalone_directive_selector( + &self, + import_source: &str, + import_name: &str, + from_file: &Path, + implicit_standalone: bool, + ) -> Option { + let mut file = self.resolve_module_spec(from_file.parent()?, import_source)?; + let mut name = import_name.to_string(); + for _ in 0..MAX_EXPORT_CHAIN { + if Path::new(&file).components().any(|c| c.as_os_str() == "node_modules") { + return None; + } + self.value_dependencies.borrow_mut().insert(file.clone()); + if !self.cache.borrow().contains_key(&file) { + if self.analyzing.borrow().contains(&file) { + return None; // Circular import + } + self.analyze_file(&file); + } + let info = self.cache.borrow().get(&file)?.get(&name).cloned()?; + match info.re_export_source { + None => { + return standalone_class_selector( + &file, + info.value_local.as_deref()?, + implicit_standalone, + ); + } + Some((source, original)) => { + if original == "*" { + return None; + } + file = self.resolve_module_spec(Path::new(&file).parent()?, &source)?; + name = original; + } + } + } + None + } + /// Every value `import_source` exports, for `{ ...ns }` with /// `import * as ns` (ngtsc's `ResolvedModule.getExports()`): each name /// with the value it resolves to, or `None` for one that can't be read diff --git a/crates/oxc_angular_compiler/src/component/defer_resolver.rs b/crates/oxc_angular_compiler/src/component/defer_resolver.rs index d78e2c8c2..54552d820 100644 --- a/crates/oxc_angular_compiler/src/component/defer_resolver.rs +++ b/crates/oxc_angular_compiler/src/component/defer_resolver.rs @@ -22,25 +22,32 @@ //! import map. It is consumed by the main compile path in //! [`super::transform::compile_component_full`]. //! -//! ## Why `deferredImports`, not `imports`? +//! ## `deferredImports` and implicitly deferred `imports` //! //! Angular's full compilation mode also derives a deferrable set from //! `@Component.imports` by matching template selectors to imported //! directives/pipes/components (see `resolveAllDeferredDependencies` in //! `packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts`). -//! That requires cross-file metadata OXC doesn't have — selectors live on -//! the imported class, not on the import declaration. In local compilation -//! (which OXC is — single file in, single file out) Angular falls back to -//! the `deferredImports` field as the only safe source of deferrable -//! symbols, and OXC does the same. +//! That requires cross-file metadata — selectors live on the imported class, +//! not on the import declaration. In local compilation Angular falls back to +//! the `deferredImports` field as the only source of deferrable symbols. +//! OXC does the same unless cross-file analysis can read the imported +//! class's selector: then [`implicitly_deferred_imports`] picks the +//! standalone components and directives the template matches only inside +//! `@defer` blocks, and they join `deferredImports` (#313). use oxc_allocator::Allocator; -use crate::ast::r3::{R3DeferredBlock, R3Node, R3Visitor, visit_all}; +use crate::ast::expression::BindingType; +use crate::ast::r3::{ + R3BoundAttribute, R3BoundEvent, R3Component, R3DeferredBlock, R3Directive, R3Element, R3Node, + R3Template, R3TemplateAttr, R3TextAttribute, R3Visitor, visit_all, +}; use crate::class_metadata::{ R3DeferPerComponentDependency, compile_component_metadata_async_resolver, }; use crate::output::ast::OutputExpression; +use crate::pipeline::selector::CssSelector; use oxc_str::Ident; use super::metadata::ComponentMetadata; @@ -165,3 +172,161 @@ pub fn build_defer_resolver_expression<'a>( Some(compile_component_metadata_async_resolver(allocator, &deps)) } + +/// The `candidates` matched only inside the main content of `@defer` blocks +/// (`resolveDeferBlocks` in ngc's component `handler.ts`). `@placeholder`, +/// `@loading` and `@error` count as eager. Selectorless templates return none. +pub(super) fn implicitly_deferred_imports<'a>( + nodes: &[R3Node<'a>], + candidates: &[(Ident<'a>, std::vec::Vec)], +) -> std::vec::Vec> { + struct Matcher<'c, 'a> { + candidates: &'c [(Ident<'a>, std::vec::Vec)], + in_defer: bool, + deferred: std::vec::Vec, + eager: std::vec::Vec, + selectorless: bool, + } + + impl Matcher<'_, '_> { + fn record(&mut self, element: &CssSelector) { + for (index, (_, selectors)) in self.candidates.iter().enumerate() { + if selectors.iter().any(|selector| selector.matches(element)) { + if self.in_defer { + self.deferred[index] = true; + } else { + self.eager[index] = true; + } + } + } + } + } + + impl<'a> R3Visitor<'a> for Matcher<'_, 'a> { + fn visit_element(&mut self, element: &R3Element<'a>) { + if !element.directives.is_empty() { + self.selectorless = true; + } + self.record(&element_selector( + &element.name, + &element.attributes, + &element.inputs, + &element.outputs, + )); + self.visit_element_children(element); + } + + fn visit_template(&mut self, template: &R3Template<'a>) { + if !template.directives.is_empty() { + self.selectorless = true; + } + self.record(&template_selector(template)); + self.visit_template_children(template); + } + + fn visit_deferred_block(&mut self, block: &R3DeferredBlock<'a>) { + let outer = self.in_defer; + self.in_defer = true; + for child in &block.children { + child.visit(self); + } + self.in_defer = false; + if let Some(placeholder) = &block.placeholder { + self.visit_deferred_block_placeholder(placeholder); + } + if let Some(loading) = &block.loading { + self.visit_deferred_block_loading(loading); + } + if let Some(error) = &block.error { + self.visit_deferred_block_error(error); + } + self.in_defer = outer; + } + + fn visit_component(&mut self, _component: &R3Component<'a>) { + self.selectorless = true; + } + + fn visit_directive(&mut self, _directive: &R3Directive<'a>) { + self.selectorless = true; + } + } + + let mut matcher = Matcher { + candidates, + in_defer: false, + deferred: vec![false; candidates.len()], + eager: vec![false; candidates.len()], + selectorless: false, + }; + visit_all(&mut matcher, nodes); + if matcher.selectorless { + return std::vec::Vec::new(); + } + candidates + .iter() + .enumerate() + .filter(|(index, _)| matcher.deferred[*index] && !matcher.eager[*index]) + .map(|(_, (name, _))| name.clone()) + .collect() +} + +/// An element as Angular matches directives against it +/// (`getAttrsForDirectiveMatching` in `packages/compiler/src/render3/view/util.ts`): +/// its static attributes with their values, the names of its property and +/// two-way bindings and of its outputs, and its `class` attribute as classes. +fn element_selector( + name: &str, + attributes: &[R3TextAttribute<'_>], + inputs: &[R3BoundAttribute<'_>], + outputs: &[R3BoundEvent<'_>], +) -> CssSelector { + let mut selector = CssSelector::new(); + selector.set_element(name); + for attribute in attributes { + if attribute.name.starts_with("i18n") { + continue; + } + selector.add_attribute(&attribute.name, Some(&attribute.value)); + if attribute.name == "class" { + for class in attribute.value.split_whitespace() { + selector.add_class_name(class); + } + } + } + for input in inputs { + if matches!(input.binding_type, BindingType::Property | BindingType::TwoWay) { + selector.add_attribute(&input.name, None); + } + } + for output in outputs { + selector.add_attribute(&output.name, None); + } + selector +} + +/// A template as Angular matches directives against it: an `` +/// like an element, a structural directive's template (`*ngIf`) by the names +/// of its template attributes. +fn template_selector(template: &R3Template<'_>) -> CssSelector { + match &template.tag_name { + Some(tag_name) if tag_name.as_str() == "ng-template" => element_selector( + "ng-template", + &template.attributes, + &template.inputs, + &template.outputs, + ), + _ => { + let mut selector = CssSelector::new(); + selector.set_element("ng-template"); + for attribute in &template.template_attrs { + let name = match attribute { + R3TemplateAttr::Bound(bound) => &bound.name, + R3TemplateAttr::Text(text) => &text.name, + }; + selector.add_attribute(name, None); + } + selector + } + } +} diff --git a/crates/oxc_angular_compiler/src/component/import_elision.rs b/crates/oxc_angular_compiler/src/component/import_elision.rs index 76731155b..40956b667 100644 --- a/crates/oxc_angular_compiler/src/component/import_elision.rs +++ b/crates/oxc_angular_compiler/src/component/import_elision.rs @@ -47,7 +47,7 @@ use oxc_ast::ast::{ }; use oxc_semantic::{Semantic, SemanticBuilder, SymbolFlags}; use oxc_str::Ident; -use rustc_hash::FxHashSet; +use rustc_hash::{FxHashMap, FxHashSet}; use crate::optimizer::Edit; @@ -61,6 +61,9 @@ const PARAM_DECORATORS: &[&str] = &["Inject", "Optional", "Self", "SkipSelf", "H pub struct ImportElisionAnalyzer<'a> { /// Set of import specifier local names that should be removed (type-only). type_only_specifiers: FxHashSet>, + /// Number of value (non-type) references to each named or default import + /// specifier, by local name. + value_reference_counts: FxHashMap, usize>, } impl<'a> ImportElisionAnalyzer<'a> { @@ -82,6 +85,7 @@ impl<'a> ImportElisionAnalyzer<'a> { let semantic = &semantic_ret.semantic; let mut type_only_specifiers = FxHashSet::default(); + let mut value_reference_counts = FxHashMap::default(); // First, collect all symbols that are used ONLY in constructor parameter decorators. // These should be elided because Angular removes these decorators during compilation. @@ -127,6 +131,10 @@ impl<'a> ImportElisionAnalyzer<'a> { } let name: Ident<'a> = spec.local.name.clone().into(); + value_reference_counts.insert( + name.clone(), + Self::value_reference_count_of(&spec.local, semantic), + ); // Check if this import has only type references if Self::is_type_only_import(&spec.local, semantic) { @@ -139,6 +147,10 @@ impl<'a> ImportElisionAnalyzer<'a> { } ImportDeclarationSpecifier::ImportDefaultSpecifier(spec) => { let name: Ident<'a> = spec.local.name.clone().into(); + value_reference_counts.insert( + name.clone(), + Self::value_reference_count_of(&spec.local, semantic), + ); if Self::is_type_only_import(&spec.local, semantic) { type_only_specifiers.insert(name.clone()); @@ -178,7 +190,7 @@ impl<'a> ImportElisionAnalyzer<'a> { } } - Self { type_only_specifiers } + Self { type_only_specifiers, value_reference_counts } } /// Local names of **default imports** that a decorated class's constructor @@ -848,6 +860,33 @@ impl<'a> ImportElisionAnalyzer<'a> { !has_value_reference } + /// The number of value references to an import binding, or `usize::MAX` + /// when a value redeclaration shadows it (a reference can't be told apart). + fn value_reference_count_of(id: &BindingIdentifier<'a>, semantic: &Semantic<'a>) -> usize { + let symbol_id = id.symbol_id(); + let symbol_flags = semantic.scoping().symbol_flags(symbol_id); + if (symbol_flags - SymbolFlags::Import).is_value() { + return usize::MAX; + } + semantic + .scoping() + .get_resolved_references(symbol_id) + .filter(|reference| !reference.is_type()) + .count() + } + + /// How many value (non-type) references the source makes to the named or + /// default import `name`, or `None` for a name no such import binds. + pub fn value_reference_count(&self, name: &str) -> Option { + self.value_reference_counts.get(&Ident::from(name)).copied() + } + + /// Remove `name`'s import: the compiled output no longer references it, + /// such as a component that `@defer` now loads with a dynamic `import()`. + pub fn elide(&mut self, name: Ident<'a>) { + self.type_only_specifiers.insert(name); + } + /// Check if a specifier name should be elided (removed). pub fn should_elide(&self, name: &str) -> bool { self.type_only_specifiers.contains(&Ident::from(name)) diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index f9ad30d81..c66e2738e 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -3514,6 +3514,42 @@ 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 `@defer` may lazy-load it + // (#313): its import is referenced nowhere else in this file, + // so the static import can go, and cross-file analysis can + // read the selector. Watched files are only reported with + // `resolve_imported_values`, so it is required here too. + #[cfg(feature = "cross_file_elision")] + let deferrable_selector = cross_file_analyzer + .as_ref() + .filter(|_| options.resolve_imported_values) + .map(|analyzer| { + let import_map = &import_map; + let import_elision = &import_elision_mut; + move |local: &str| -> Option { + if import_elision.value_reference_count(local)? != 1 { + return None; + } + let info = import_map.get(&Ident::from(local))?; + if !info.is_named_import || info.is_type_only { + return None; + } + let export_name = info.imported_name.as_deref().unwrap_or(local); + analyzer.standalone_directive_selector( + info.source_module.as_str(), + export_name, + std::path::Path::new(path), + implicit_standalone, + ) + } + }); + #[cfg(feature = "cross_file_elision")] + let deferrable_selector = + deferrable_selector.as_ref().map(|f| f as DeferrableSelector<'_>); + #[cfg(not(feature = "cross_file_elision"))] + let deferrable_selector = None; + // 4. Compile the template and generate ɵcmp/ɵfac // Pass the shared pool index to ensure unique constant names // Pass the file-level namespace registry to ensure consistent namespace assignments @@ -3530,11 +3566,17 @@ pub fn transform_angular_file( &mut file_namespace_registry, &import_map, resolved_resources, + deferrable_selector, ) { Ok(compilation_result) => { // Update the shared pool index for the next component shared_pool_index = compilation_result.next_pool_index; + // `@defer` loads these with a dynamic `import()` now. + for name in &compilation_result.implicitly_deferred_imports { + import_elision_mut.elide(Ident::from_in(name.as_str(), allocator)); + } + // Stylesheet ``s in the template are watched // files like decorator `styleUrls`, and a missing // one is the same non-fatal diagnostic. @@ -4704,6 +4746,11 @@ struct FullCompilationResult { /// `import(...)` calls in the output. has_defer_block: bool, + /// Local names of `@Component.imports` entries moved to the `@defer` + /// resolver because the template matches them only inside `@defer` + /// blocks (#313). The caller removes their static imports. + implicitly_deferred_imports: Vec, + /// Resolvable-URL stylesheet ``s the template's preparser collected /// (upstream `_extractTemplateStyleUrls`). The caller records them as /// watch dependencies like decorator `styleUrls`. @@ -4778,6 +4825,7 @@ fn compile_component_partial<'a>( next_pool_index: pool_starting_index, ng_content_selectors: Vec::new(), has_defer_block, + implicitly_deferred_imports: Vec::new(), // Partial mode never parses the template; the linker collects link // URLs at consumer build time. template_style_urls: Vec::new(), @@ -4865,6 +4913,10 @@ fn html_parse_error_diagnostic( } } +/// The selector of the standalone component or directive an `imports` +/// entry refers to, by local name, when `@defer` may lazy-load it. +type DeferrableSelector<'f> = &'f dyn Fn(&str) -> Option; + fn compile_component_full<'a>( allocator: &'a Allocator, template: &'a str, @@ -4878,6 +4930,7 @@ fn compile_component_full<'a>( namespace_registry: &mut NamespaceRegistry<'a>, import_map: &ImportMap<'a>, resolved_resources: Option<&ResolvedResources>, + deferrable_selector: Option>, ) -> Result> { use oxc_allocator::FromIn; @@ -5016,6 +5069,39 @@ fn compile_component_full<'a>( } } + // `imports` entries matched only inside `@defer` are lazy-loaded, as in + // ngc's full compilation (#313). Needs cross-file analysis for the + // selectors; skipped under HMR, like ngc. + let mut implicitly_deferred_imports = Vec::new(); + if let Some(deferrable_selector) = deferrable_selector + && !options.hmr + && let Some(OutputExpression::LiteralArray(raw_imports)) = &metadata.raw_imports + && raw_imports.entries.iter().all(|entry| matches!(entry, OutputExpression::ReadVar(_))) + && super::defer_resolver::template_has_defer_block(&r3_result.nodes) + { + let candidates: Vec<_> = metadata + .imports + .iter() + .filter_map(|name| { + let selector = deferrable_selector(name.as_str())?; + Some((name.clone(), crate::pipeline::selector::CssSelector::parse(&selector))) + }) + .collect(); + let deferred = + super::defer_resolver::implicitly_deferred_imports(&r3_result.nodes, &candidates); + if !deferred.is_empty() { + let is_deferred = |name: &str| deferred.iter().any(|d| d.as_str() == name); + metadata.imports.retain(|name| !is_deferred(name.as_str())); + if let Some(OutputExpression::LiteralArray(raw_imports)) = &mut metadata.raw_imports { + raw_imports.entries.retain(|entry| { + !matches!(entry, OutputExpression::ReadVar(var) if is_deferred(var.name.as_str())) + }); + } + implicitly_deferred_imports = deferred.iter().map(ToString::to_string).collect(); + metadata.deferred_imports.extend(deferred); + } + } + // Build the deferrable-dependencies resolver from // `@Component.deferredImports`. // @@ -5032,9 +5118,9 @@ fn compile_component_full<'a>( // // Note: Angular's full compilation mode also derives deferrable symbols // from `imports: [...]` by matching template selectors against imported - // directive classes (`resolveAllDeferredDependencies`). That needs - // cross-file metadata OXC doesn't have, so we follow Angular's local - // path and require explicit `deferredImports`. + // directive classes (`resolveAllDeferredDependencies`). OXC does that + // above when cross-file analysis is available (#313) and otherwise + // follows Angular's local path, which requires explicit `deferredImports`. // Computed once and reused below: the resolver decision and the eventual // `setClassMetadataAsync` gate both depend on whether the template uses // `@defer`. @@ -5318,6 +5404,7 @@ fn compile_component_full<'a>( next_pool_index, ng_content_selectors, has_defer_block, + implicitly_deferred_imports, template_style_urls, missing_link_urls, }) diff --git a/crates/oxc_angular_compiler/src/pipeline/selector.rs b/crates/oxc_angular_compiler/src/pipeline/selector.rs index a58031825..7e1484dc1 100644 --- a/crates/oxc_angular_compiler/src/pipeline/selector.rs +++ b/crates/oxc_angular_compiler/src/pipeline/selector.rs @@ -85,6 +85,40 @@ impl CssSelector { self.attrs.push(value.map(|v| v.to_lowercase()).unwrap_or_default()); } + /// Whether this selector matches `element`, a selector describing a + /// template element the way Angular builds one for directive matching: + /// its tag name, its attributes as name/value pairs, and its `class` + /// attribute as class names. + /// + /// The element name must be equal when the selector has one, every class + /// and attribute the selector names must be present, an attribute value in + /// the selector must be equal (an empty one matches any value), and none + /// of the `:not(...)` selectors may match. + /// + /// Ported from the matching half of Angular's `SelectorMatcher` in + /// `directive_matching.ts`. + pub fn matches(&self, element: &CssSelector) -> bool { + if let Some(name) = &self.element + && element.element.as_deref() != Some(name.as_str()) + { + return false; + } + if !self.class_names.iter().all(|class| element.class_names.contains(class)) { + return false; + } + let has_attribute = |name: &str, value: &str| { + element.attrs.chunks(2).any(|pair| { + pair[0] == name + && (value.is_empty() || pair.get(1).map(String::as_str) == Some(value)) + }) + }; + if !self.attrs.chunks(2).all(|pair| has_attribute(&pair[0], pair.get(1).map_or("", |v| v))) + { + return false; + } + !self.not_selectors.iter().any(|not| not.matches(element)) + } + /// Parse a CSS selector string into a list of CssSelectors. /// /// Handles comma-separated selectors, element names, classes, attributes, and :not(). @@ -594,4 +628,51 @@ mod tests { assert_eq!(selectors[0].element, Some("span".to_string())); assert_eq!(selectors[0].attrs, vec!["bitBadge", ""]); } + + fn element(name: &str, attrs: &[(&str, &str)], classes: &[&str]) -> CssSelector { + let mut element = CssSelector::new(); + element.set_element(name); + for (attr, value) in attrs { + element.add_attribute(attr, Some(value)); + } + for class in classes { + element.add_class_name(class); + } + element + } + + fn matches(selector: &str, element: &CssSelector) -> bool { + CssSelector::parse(selector).iter().any(|s| s.matches(element)) + } + + #[test] + fn test_matches_element_name() { + assert!(matches("app-lazy", &element("app-lazy", &[], &[]))); + assert!(!matches("app-lazy", &element("app-other", &[], &[]))); + } + + #[test] + fn test_matches_attribute_with_and_without_value() { + let button = element("button", &[("appTooltip", ""), ("type", "Submit")], &[]); + assert!(matches("[appTooltip]", &button)); + assert!(matches("button[type=submit]", &button)); + assert!(!matches("button[type=reset]", &button)); + assert!(!matches("[appOther]", &button)); + } + + #[test] + fn test_matches_classes() { + let div = element("div", &[], &["card", "wide"]); + assert!(matches(".card", &div)); + assert!(matches("div.card.wide", &div)); + assert!(!matches(".narrow", &div)); + } + + #[test] + fn test_matches_not_and_selector_lists() { + let input = element("input", &[("type", "checkbox")], &[]); + assert!(!matches("input:not([type=checkbox])", &input)); + assert!(matches("input:not([type=radio])", &input)); + assert!(matches("textarea, input[type=checkbox]", &input)); + } } diff --git a/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs new file mode 100644 index 000000000..ba74ccc0e --- /dev/null +++ b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs @@ -0,0 +1,413 @@ +//! Implicit `@defer` dependencies (#313): a standalone component or directive +//! listed in `@Component.imports` and matched only inside `@defer` blocks is +//! lazy-loaded through a dynamic `import()`, the way ngc's full compilation +//! does, when cross-file analysis can read its selector +//! (`TransformOptions::resolve_imported_values`). + +#![cfg(feature = "cross_file_elision")] + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{TransformOptions, TransformResult, transform_angular_file}; +use tempfile::TempDir; + +fn create_test_file(dir: &std::path::Path, name: &str, content: &str) -> std::path::PathBuf { + let path = dir.join(name); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(&path, content).unwrap(); + path +} + +fn options() -> TransformOptions { + TransformOptions { resolve_imported_values: true, ..Default::default() } +} + +fn transform_with(dir: &TempDir, source: &str, options: &TransformOptions) -> TransformResult { + let path = create_test_file(dir.path(), "app/parent.component.ts", source); + let allocator = Allocator::default(); + let result = + transform_angular_file(&allocator, path.to_str().unwrap(), source, Some(options), None); + assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + result +} + +fn transform(dir: &TempDir, source: &str) -> TransformResult { + transform_with(dir, source, &options()) +} + +fn compact(code: &str) -> String { + code.chars().filter(|c| !c.is_whitespace()).collect() +} + +fn lazy_cmp(dir: &TempDir) { + create_test_file( + dir.path(), + "app/lazy.ts", + r#" +import { Component } from '@angular/core'; + +@Component({ selector: 'app-lazy', template: 'lazy' }) +export class LazyCmp {} +"#, + ); +} + +fn assert_lazy(code: &str, module: &str, member: &str) { + let compact = compact(code); + assert!( + compact.contains(&format!("import(\"{module}\").then((m)=>m.{member})")) + || compact.contains(&format!("import(\"{module}\").then(m=>m.{member})")), + "Expected `import('{module}').then(m => m.{member})`. Output:\n{code}" + ); +} + +fn assert_eager(code: &str, module: &str) { + assert!( + !code.contains(&format!("import(\"{module}\")")), + "`{module}` must stay an eager dependency. Output:\n{code}" + ); +} + +#[test] +fn component_used_only_inside_defer_is_lazy_loaded() { + 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], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + let code = &result.code; + let compact = compact(code); + + assert_lazy(code, "./lazy", "LazyCmp"); + assert!( + compact.contains("ɵɵdefer(1,0,()=>[import(\"./lazy\")"), + "The resolver must be wired into `ɵɵdefer`. Output:\n{code}" + ); + assert!( + compact.contains("ɵɵgetComponentDepsFactory(Parent,[])"), + "The lazy component must leave the eager dependencies. Output:\n{code}" + ); + assert!( + !code.contains("import { LazyCmp }"), + "The static import of the lazy component must be removed. Output:\n{code}" + ); + assert!(code.contains("setClassMetadataAsync"), "Output:\n{code}"); +} + +#[test] +fn component_also_used_outside_defer_stays_eager() { + 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], + template: ' @defer { }', +}) +export class Parent {} +"#, + ); + let code = &result.code; + assert_eager(code, "./lazy"); + assert!(compact(code).contains("ɵɵgetComponentDepsFactory(Parent,[LazyCmp])"), "{code}"); + assert!(code.contains("import { LazyCmp }"), "{code}"); +} + +#[test] +fn component_used_in_a_placeholder_stays_eager() { + 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], + template: '@defer { } @placeholder { }', +}) +export class Parent {} +"#, + ); + assert_eager(&result.code, "./lazy"); +} + +#[test] +fn component_used_in_nested_control_flow_inside_defer_is_lazy_loaded() { + 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], + template: '@defer { @switch (mode) { @case (1) {
} } }', +}) +export class Parent { mode = 1; } +"#, + ); + assert_lazy(&result.code, "./lazy", "LazyCmp"); +} + +#[test] +fn aliased_import_reads_the_exported_name() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { LazyCmp as Lazy } from './lazy'; + +@Component({ + selector: 'app-parent', + imports: [Lazy], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + assert_lazy(&result.code, "./lazy", "LazyCmp"); +} + +#[test] +fn default_import_reads_default() { + let dir = TempDir::new().unwrap(); + create_test_file( + dir.path(), + "app/lazy.ts", + r#" +import { Component } from '@angular/core'; + +@Component({ selector: 'app-lazy', template: 'lazy' }) +export default class LazyCmp {} +"#, + ); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import LazyCmp from './lazy'; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + assert_lazy(&result.code, "./lazy", "default"); +} + +#[test] +fn import_referenced_elsewhere_in_the_file_stays_eager() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { LazyCmp } from './lazy'; + +export const EXPOSED = LazyCmp; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + assert_eager(&result.code, "./lazy"); +} + +#[test] +fn attribute_directive_used_only_inside_defer_is_lazy_loaded() { + let dir = TempDir::new().unwrap(); + create_test_file( + dir.path(), + "app/tooltip.ts", + r#" +import { Directive } from '@angular/core'; + +@Directive({ selector: '[appTooltip]' }) +export class Tooltip {} +"#, + ); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { Tooltip } from './tooltip'; + +@Component({ + selector: 'app-parent', + imports: [Tooltip], + template: ' @defer { }', +}) +export class Parent {} +"#, + ); + assert_lazy(&result.code, "./tooltip", "Tooltip"); +} + +#[test] +fn bound_input_matches_an_attribute_selector() { + let dir = TempDir::new().unwrap(); + create_test_file( + dir.path(), + "app/tooltip.ts", + r#" +import { Directive, input } from '@angular/core'; + +@Directive({ selector: '[appTooltip]' }) +export class Tooltip { appTooltip = input(''); } +"#, + ); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { Tooltip } from './tooltip'; + +@Component({ + selector: 'app-parent', + imports: [Tooltip], + template: ' @defer { }', +}) +export class Parent { text = ''; } +"#, + ); + assert_eager(&result.code, "./tooltip"); +} + +#[test] +fn not_standalone_component_stays_eager() { + let dir = TempDir::new().unwrap(); + create_test_file( + dir.path(), + "app/lazy.ts", + r#" +import { Component } from '@angular/core'; + +@Component({ selector: 'app-lazy', template: 'lazy', standalone: false }) +export class LazyCmp {} +"#, + ); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { LazyCmp } from './lazy'; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + assert_eager(&result.code, "./lazy"); +} + +#[test] +fn pipe_stays_eager() { + let dir = TempDir::new().unwrap(); + create_test_file( + dir.path(), + "app/shout.ts", + r#" +import { Pipe } from '@angular/core'; + +@Pipe({ name: 'shout' }) +export class ShoutPipe { transform(v: string) { return v; } } +"#, + ); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { ShoutPipe } from './shout'; + +@Component({ + selector: 'app-parent', + imports: [ShoutPipe], + template: '@defer { {{ name | shout }} }', +}) +export class Parent { name = ''; } +"#, + ); + assert_eager(&result.code, "./shout"); +} + +#[test] +fn implicit_and_explicit_deferred_imports_share_the_resolver() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { LazyCmp } from './lazy'; +import { Other } from './other'; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + deferredImports: [Other], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + assert_lazy(&result.code, "./lazy", "LazyCmp"); + assert_lazy(&result.code, "./other", "Other"); +} + +#[test] +fn hmr_keeps_imports_eager() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = transform_with( + &dir, + r#" +import { Component } from '@angular/core'; +import { LazyCmp } from './lazy'; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: '@defer { }', +}) +export class Parent {} +"#, + &TransformOptions { hmr: true, ..options() }, + ); + assert_eager(&result.code, "./lazy"); +} diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 88cc64509..837ba7251 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -798,11 +798,12 @@ export class Parent {} ); } -/// When the user lists a symbol only in `@Component.imports`, OXC must NOT -/// auto-detect it as deferrable (that's the full-compilation behavior, which -/// needs cross-file selector info OXC doesn't have). Issue #289's original -/// repro form (`imports: [LazyCmp]` with `@defer`) therefore emits no -/// resolver — the user must move the symbol to `deferredImports`. +/// Without cross-file analysis, a symbol listed only in `@Component.imports` +/// must NOT be auto-detected as deferrable: that full-compilation behavior +/// needs the imported class's selector (see `defer_implicit_imports_test.rs` +/// for the cross-file case, #313). Issue #289's original repro form +/// (`imports: [LazyCmp]` with `@defer`) therefore emits no resolver here — +/// the user must move the symbol to `deferredImports`. #[test] fn test_defer_with_only_imports_does_not_auto_detect() { let allocator = Allocator::default(); From 207ff03f3c6445439c4221d82bfa55140e5be13c Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 21 Oct 2026 12:15:43 +0800 Subject: [PATCH 2/5] fix(defer): match elements against selectors the way ngc does Build each element's selector like createCssSelectorFromNode and getAttrsForDirectiveMatching: drop namespace prefixes (`:svg:`, `:xlink:`) from element and attribute names, skip only `i18n` and `i18n-*`, compare `class` case-insensitively, and let a binding replace a static attribute of the same name. In CssSelector::matches, the `*` element of a `:not(...)`-only selector matches any element, and an empty selector (`*`, a trailing comma) matches nothing, as in SelectorMatcher. The placeholder, loading and error blocks of a nested `@defer` inherit the outer block's state, as in t2_binder's visitDeferredBlock. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/component/defer_resolver.rs | 58 ++++--- .../src/pipeline/selector.rs | 35 +++- .../tests/defer_implicit_imports_test.rs | 158 ++++++++++++++++++ 3 files changed, 228 insertions(+), 23 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/defer_resolver.rs b/crates/oxc_angular_compiler/src/component/defer_resolver.rs index 54552d820..73a988b6f 100644 --- a/crates/oxc_angular_compiler/src/component/defer_resolver.rs +++ b/crates/oxc_angular_compiler/src/component/defer_resolver.rs @@ -46,7 +46,9 @@ use crate::ast::r3::{ use crate::class_metadata::{ R3DeferPerComponentDependency, compile_component_metadata_async_resolver, }; +use crate::i18n::serializer::is_i18n_attribute; use crate::output::ast::OutputExpression; +use crate::parser::html::split_ns_name; use crate::pipeline::selector::CssSelector; use oxc_str::Ident; @@ -230,7 +232,10 @@ pub(super) fn implicitly_deferred_imports<'a>( for child in &block.children { child.visit(self); } - self.in_defer = false; + // `@placeholder`, `@loading` and `@error` render before the chunk + // loads, so they are eager unless an outer `@defer` holds them + // (`visitDeferredBlock` in t2_binder's `DirectiveBinder`). + self.in_defer = outer; if let Some(placeholder) = &block.placeholder { self.visit_deferred_block_placeholder(placeholder); } @@ -240,7 +245,6 @@ pub(super) fn implicitly_deferred_imports<'a>( if let Some(error) = &block.error { self.visit_deferred_block_error(error); } - self.in_defer = outer; } fn visit_component(&mut self, _component: &R3Component<'a>) { @@ -267,44 +271,60 @@ pub(super) fn implicitly_deferred_imports<'a>( .iter() .enumerate() .filter(|(index, _)| matcher.deferred[*index] && !matcher.eager[*index]) - .map(|(_, (name, _))| name.clone()) + .map(|(_, (name, _))| *name) .collect() } /// An element as Angular matches directives against it -/// (`getAttrsForDirectiveMatching` in `packages/compiler/src/render3/view/util.ts`): -/// its static attributes with their values, the names of its property and -/// two-way bindings and of its outputs, and its `class` attribute as classes. +/// (`createCssSelectorFromNode` and `getAttrsForDirectiveMatching` in +/// `packages/compiler/src/render3/view/util.ts`): its static attributes with +/// their values, the names of its property and two-way bindings and of its +/// outputs, and its `class` attribute as classes. A binding replaces the value +/// of a static attribute with the same name, and namespace prefixes +/// (`:svg:`, `:xlink:`) are dropped. fn element_selector( name: &str, attributes: &[R3TextAttribute<'_>], inputs: &[R3BoundAttribute<'_>], outputs: &[R3BoundEvent<'_>], ) -> CssSelector { - let mut selector = CssSelector::new(); - selector.set_element(name); + let mut attrs = std::vec::Vec::new(); for attribute in attributes { - if attribute.name.starts_with("i18n") { - continue; - } - selector.add_attribute(&attribute.name, Some(&attribute.value)); - if attribute.name == "class" { - for class in attribute.value.split_whitespace() { - selector.add_class_name(class); - } + if !is_i18n_attribute(&attribute.name) { + set_attribute(&mut attrs, &attribute.name, &attribute.value); } } for input in inputs { if matches!(input.binding_type, BindingType::Property | BindingType::TwoWay) { - selector.add_attribute(&input.name, None); + set_attribute(&mut attrs, &input.name, ""); } } for output in outputs { - selector.add_attribute(&output.name, None); + set_attribute(&mut attrs, &output.name, ""); + } + + let mut selector = CssSelector::new(); + selector.set_element(split_ns_name(name).1); + for (name, value) in attrs { + selector.add_attribute(split_ns_name(name).1, Some(value)); + if name.eq_ignore_ascii_case("class") { + for class in value.split_whitespace() { + selector.add_class_name(class); + } + } } selector } +/// Sets `name` to `value` in `attrs`, a name/value map kept in insertion order +/// like the object `getAttrsForDirectiveMatching` builds. +fn set_attribute<'s>(attrs: &mut std::vec::Vec<(&'s str, &'s str)>, name: &'s str, value: &'s str) { + match attrs.iter_mut().find(|(existing, _)| *existing == name) { + Some(entry) => entry.1 = value, + None => attrs.push((name, value)), + } +} + /// A template as Angular matches directives against it: an `` /// like an element, a structural directive's template (`*ngIf`) by the names /// of its template attributes. @@ -324,7 +344,7 @@ fn template_selector(template: &R3Template<'_>) -> CssSelector { R3TemplateAttr::Bound(bound) => &bound.name, R3TemplateAttr::Text(text) => &text.name, }; - selector.add_attribute(name, None); + selector.add_attribute(split_ns_name(name).1, None); } selector } diff --git a/crates/oxc_angular_compiler/src/pipeline/selector.rs b/crates/oxc_angular_compiler/src/pipeline/selector.rs index 7e1484dc1..df9a40588 100644 --- a/crates/oxc_angular_compiler/src/pipeline/selector.rs +++ b/crates/oxc_angular_compiler/src/pipeline/selector.rs @@ -90,15 +90,26 @@ impl CssSelector { /// its tag name, its attributes as name/value pairs, and its `class` /// attribute as class names. /// - /// The element name must be equal when the selector has one, every class - /// and attribute the selector names must be present, an attribute value in - /// the selector must be equal (an empty one matches any value), and none - /// of the `:not(...)` selectors may match. + /// The element name must be equal when the selector has one (`*`, which a + /// selector made only of `:not(...)` gets, matches any element), every + /// class and attribute the selector names must be present, an attribute + /// value in the selector must be equal (an empty one matches any value), + /// and none of the `:not(...)` selectors may match. An empty selector (`*`, + /// or the part after a trailing comma) matches nothing, because + /// `_addSelectable` registers nothing for it. /// /// Ported from the matching half of Angular's `SelectorMatcher` in /// `directive_matching.ts`. pub fn matches(&self, element: &CssSelector) -> bool { + if self.element.is_none() + && self.class_names.is_empty() + && self.attrs.is_empty() + && self.not_selectors.is_empty() + { + return false; + } if let Some(name) = &self.element + && name != "*" && element.element.as_deref() != Some(name.as_str()) { return false; @@ -675,4 +686,20 @@ mod tests { assert!(matches("input:not([type=radio])", &input)); assert!(matches("textarea, input[type=checkbox]", &input)); } + + #[test] + fn test_matches_not_only_selector_matches_any_element() { + assert!(matches(":not(.plain)", &element("span", &[], &[]))); + assert!(!matches(":not(.plain)", &element("span", &[], &["plain"]))); + assert!(matches("*:not([appDep])", &element("div", &[], &[]))); + } + + #[test] + fn test_matches_empty_selector_matches_nothing() { + let span = element("span", &[("appDep", "")], &["card"]); + assert!(!matches("", &span)); + assert!(!matches("*", &span)); + assert!(!matches("[appOther],", &span)); + assert!(matches("[appDep],", &span)); + } } 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 ba74ccc0e..aff0b16f3 100644 --- a/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs +++ b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs @@ -411,3 +411,161 @@ export class Parent {} ); assert_eager(&result.code, "./lazy"); } + +// --------------------------------------------------------------------------- +// Selector matching (`pipeline/selector.rs`, `component/defer_resolver.rs`): +// elements are matched the way ngc's `createCssSelectorFromNode` and +// `SelectorMatcher` match them. +// --------------------------------------------------------------------------- + +/// Writes `app/dep.ts`, a standalone directive `Dep` with `selector`. +fn selector_matching_dep(dir: &TempDir, selector: &str) { + create_test_file( + dir.path(), + "app/dep.ts", + &format!( + r#" +import {{ Directive }} from '@angular/core'; + +@Directive({{ selector: '{selector}' }}) +export class Dep {{}} +"# + ), + ); +} + +/// Compiles a parent component that imports `Dep` and uses `template`. +fn selector_matching_transform(dir: &TempDir, template: &str) -> TransformResult { + transform( + dir, + &format!( + r#" +import {{ Component }} from '@angular/core'; +import {{ Dep }} from './dep'; + +@Component({{ + selector: 'app-parent', + imports: [Dep], + template: `{template}`, +}}) +export class Parent {{ x = ''; }} +"# + ), + ) +} + +#[test] +fn selector_matching_svg_element_used_eagerly_stays_eager() { + let dir = TempDir::new().unwrap(); + selector_matching_dep(&dir, "svg[appIcon], i[appIcon]"); + let result = + selector_matching_transform(&dir, " @defer { }"); + assert_eager(&result.code, "./dep"); +} + +#[test] +fn selector_matching_element_inside_svg_used_eagerly_stays_eager() { + let dir = TempDir::new().unwrap(); + selector_matching_dep(&dir, "a[appLink]"); + let result = + selector_matching_transform(&dir, " @defer { }"); + assert_eager(&result.code, "./dep"); +} + +#[test] +fn selector_matching_element_inside_svg_used_only_inside_defer_is_lazy_loaded() { + let dir = TempDir::new().unwrap(); + selector_matching_dep(&dir, "g[appDep]"); + let result = selector_matching_transform(&dir, "@defer { }"); + assert_lazy(&result.code, "./dep", "Dep"); +} + +#[test] +fn selector_matching_namespaced_attribute_used_eagerly_stays_eager() { + let dir = TempDir::new().unwrap(); + selector_matching_dep(&dir, "[href]"); + let result = selector_matching_transform( + &dir, + r#" @defer { }"#, + ); + assert_eager(&result.code, "./dep"); +} + +#[test] +fn selector_matching_only_i18n_attributes_are_skipped() { + let dir = TempDir::new().unwrap(); + selector_matching_dep(&dir, "[i18nTip]"); + let result = + selector_matching_transform(&dir, r#" @defer { }"#); + assert_eager(&result.code, "./dep"); +} + +#[test] +fn selector_matching_class_attribute_name_is_case_insensitive() { + let dir = TempDir::new().unwrap(); + selector_matching_dep(&dir, ".foo"); + let result = + selector_matching_transform(&dir, r#" @defer { }"#); + assert_eager(&result.code, "./dep"); +} + +#[test] +fn selector_matching_not_only_selector_matches_any_element() { + let dir = TempDir::new().unwrap(); + selector_matching_dep(&dir, ":not(.plain)"); + let result = selector_matching_transform(&dir, "@defer { }"); + assert_lazy(&result.code, "./dep", "Dep"); + + let dir = TempDir::new().unwrap(); + selector_matching_dep(&dir, "[appDep], :not([appDep]):not(span)"); + let result = selector_matching_transform(&dir, "
@defer { }"); + assert_eager(&result.code, "./dep"); +} + +#[test] +fn selector_matching_empty_selector_in_a_list_matches_nothing() { + let dir = TempDir::new().unwrap(); + selector_matching_dep(&dir, "[appDep],"); + let result = selector_matching_transform(&dir, "
@defer { }"); + assert_lazy(&result.code, "./dep", "Dep"); +} + +#[test] +fn selector_matching_binding_overrides_static_attribute_value() { + let dir = TempDir::new().unwrap(); + selector_matching_dep(&dir, "[type=a]"); + let result = selector_matching_transform( + &dir, + r#" @defer { }"#, + ); + assert_lazy(&result.code, "./dep", "Dep"); + + let dir = TempDir::new().unwrap(); + selector_matching_dep(&dir, ".foo"); + let result = selector_matching_transform( + &dir, + r#"
@defer {
}"#, + ); + assert_lazy(&result.code, "./dep", "Dep"); +} + +#[test] +fn selector_matching_placeholder_of_a_nested_defer_is_lazy_loaded() { + 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], + template: '@defer { @defer {

x

} @placeholder { } }', +}) +export class Parent {} +"#, + ); + assert_lazy(&result.code, "./lazy", "LazyCmp"); +} From ecdd2eaf06f7fb7225b1d25e752b8bd3d40f176a Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 21 Oct 2026 12:20:18 +0800 Subject: [PATCH 3/5] fix(defer): keep imports other emitted code needs, defer whole declarations An implicitly deferred import could be removed while other output still named it: a `jit: true` sibling's `ctorParameters` (lost only when that class came first in the file), a default import in a sibling's `setClassMetadata` constructor types, or a computed type key. Those names are now collected before any class compiles and are never deferred. Like ngc's `DeferredSymbolTracker.canDefer`, a specifier is deferred only when every other non-`type` specifier in its import declaration is deferred too, so the static import really goes away. `value_reference_count` and `elide` are now `pub(crate)`. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/component/import_elision.rs | 61 +++- .../src/component/transform.rs | 109 ++++-- .../tests/defer_implicit_imports_test.rs | 318 ++++++++++++++++++ 3 files changed, 442 insertions(+), 46 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/import_elision.rs b/crates/oxc_angular_compiler/src/component/import_elision.rs index 40956b667..b46175196 100644 --- a/crates/oxc_angular_compiler/src/component/import_elision.rs +++ b/crates/oxc_angular_compiler/src/component/import_elision.rs @@ -64,6 +64,14 @@ pub struct ImportElisionAnalyzer<'a> { /// Number of value (non-type) references to each named or default import /// specifier, by local name. value_reference_counts: FxHashMap, usize>, + /// Imports the compiled output references as values although the source + /// doesn't (see [`Self::preserve`] and the post-passes in + /// [`Self::analyze`]), so `@defer` can't remove them. + kept_by_output: FxHashSet>, + /// The non-`type` specifiers of each import declaration, by local name. + declaration_specifiers: Vec>>, + /// Index into `declaration_specifiers` of each non-`type` specifier. + declaration_of: FxHashMap, usize>, } impl<'a> ImportElisionAnalyzer<'a> { @@ -86,6 +94,8 @@ impl<'a> ImportElisionAnalyzer<'a> { let mut type_only_specifiers = FxHashSet::default(); let mut value_reference_counts = FxHashMap::default(); + let mut declaration_specifiers = Vec::new(); + let mut declaration_of = FxHashMap::default(); // First, collect all symbols that are used ONLY in constructor parameter decorators. // These should be elided because Angular removes these decorators during compilation. @@ -121,6 +131,8 @@ impl<'a> ImportElisionAnalyzer<'a> { continue; }; + let declaration = declaration_specifiers.len(); + declaration_specifiers.push(Vec::new()); for specifier in specifiers { match specifier { ImportDeclarationSpecifier::ImportSpecifier(spec) => { @@ -131,10 +143,8 @@ impl<'a> ImportElisionAnalyzer<'a> { } let name: Ident<'a> = spec.local.name.clone().into(); - value_reference_counts.insert( - name.clone(), - Self::value_reference_count_of(&spec.local, semantic), - ); + value_reference_counts + .insert(name, Self::value_reference_count_of(&spec.local, semantic)); // Check if this import has only type references if Self::is_type_only_import(&spec.local, semantic) { @@ -147,10 +157,8 @@ impl<'a> ImportElisionAnalyzer<'a> { } ImportDeclarationSpecifier::ImportDefaultSpecifier(spec) => { let name: Ident<'a> = spec.local.name.clone().into(); - value_reference_counts.insert( - name.clone(), - Self::value_reference_count_of(&spec.local, semantic), - ); + value_reference_counts + .insert(name, Self::value_reference_count_of(&spec.local, semantic)); if Self::is_type_only_import(&spec.local, semantic) { type_only_specifiers.insert(name.clone()); @@ -165,6 +173,9 @@ impl<'a> ImportElisionAnalyzer<'a> { } } } + let name = specifier.local().name; + declaration_specifiers[declaration].push(name); + declaration_of.insert(name, declaration); } } @@ -176,6 +187,7 @@ impl<'a> ImportElisionAnalyzer<'a> { for name in &computed_key_idents { type_only_specifiers.remove(name); } + let mut kept_by_output = computed_key_idents; // Post-pass: when class metadata is emitted, a default import used as a // bare ctor-param type reference is emitted into the metadata's @@ -188,9 +200,16 @@ impl<'a> ImportElisionAnalyzer<'a> { for name in &ctor_type_names { type_only_specifiers.remove(name); } + kept_by_output.extend(ctor_type_names); } - Self { type_only_specifiers, value_reference_counts } + Self { + type_only_specifiers, + value_reference_counts, + kept_by_output, + declaration_specifiers, + declaration_of, + } } /// Local names of **default imports** that a decorated class's constructor @@ -877,14 +896,31 @@ impl<'a> ImportElisionAnalyzer<'a> { /// How many value (non-type) references the source makes to the named or /// default import `name`, or `None` for a name no such import binds. - pub fn value_reference_count(&self, name: &str) -> Option { + pub(crate) fn value_reference_count(&self, name: &str) -> Option { self.value_reference_counts.get(&Ident::from(name)).copied() } + /// Whether the compiled output references `name` as a value although the + /// source doesn't, like a `jit: true` class's `ctorParameters`. + pub(crate) fn is_kept_by_output(&self, name: &str) -> bool { + self.kept_by_output.contains(&Ident::from(name)) + } + + /// The local names of every non-`type` specifier in the import + /// declaration that binds `name`, `name` included (empty if none does). + pub(crate) fn declaration_specifiers(&self, name: &str) -> &[Ident<'a>] { + self.declaration_of + .get(&Ident::from(name)) + .map_or(&[], |&declaration| &self.declaration_specifiers[declaration]) + } + /// Remove `name`'s import: the compiled output no longer references it, /// such as a component that `@defer` now loads with a dynamic `import()`. - pub fn elide(&mut self, name: Ident<'a>) { - self.type_only_specifiers.insert(name); + /// A name the output still needs (see [`Self::is_kept_by_output`]) stays. + pub(crate) fn elide(&mut self, name: Ident<'a>) { + if !self.kept_by_output.contains(&name) { + self.type_only_specifiers.insert(name); + } } /// Check if a specifier name should be elided (removed). @@ -903,6 +939,7 @@ impl<'a> ImportElisionAnalyzer<'a> { /// transforms, on the rewritten AST. pub fn preserve(&mut self, name: &'a str) { self.type_only_specifiers.remove(&Ident::from(name)); + self.kept_by_output.insert(Ident::from(name)); } /// Get the set of type-only specifier names. diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index c66e2738e..f8d536ae5 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -3242,6 +3242,55 @@ pub fn transform_angular_file( } } + // The imports a `jit: true` class's downleveled output keeps live, found + // before any class compiles so `@defer` leaves them alone whichever class + // comes first (#313). + for stmt in &parser_ret.program.body { + let class = match stmt { + Statement::ClassDeclaration(class) => class.as_ref(), + Statement::ExportDefaultDeclaration(export) => match &export.declaration { + ExportDefaultDeclarationKind::ClassDeclaration(class) => class.as_ref(), + _ => continue, + }, + Statement::ExportDeclaration(export) => match &export.declaration { + Declaration::ClassDeclaration(class) => class.as_ref(), + _ => continue, + }, + _ => continue, + }; + if find_jit_forced_decorator(class, &string_consts).is_none() { + continue; + } + + // Identifiers used only in decorator positions (member, + // `declare` member, and param decorators) survive in the + // emitted __decorate / propDecorators calls, so their imports + // must be kept — the same result TypeScript's elision + // produces on ngtsc's rewritten AST. + for name in collect_decorator_identifiers(class) { + import_elision_mut.preserve(name); + } + + // Identifiers used only in constructor parameters (types, + // param decorators) survive in the emitted ctorParameters / + // __param, so their imports must be kept — the same result + // TypeScript's elision produces on ngtsc's rewritten AST. + let mut param_idents = JitCtorParamIdentifiers::default(); + for element in &class.body.body { + if let oxc_ast::ast::ClassElement::MethodDefinition(method) = element + && method.kind == oxc_ast::ast::MethodDefinitionKind::Constructor + { + oxc_ast_visit::Visit::visit_formal_parameters( + &mut param_idents, + &method.value.params, + ); + } + } + for name in param_idents.0 { + import_elision_mut.preserve(name); + } + } + // 2. Walk AST to find @Component decorated classes and extract metadata for stmt in &parser_ret.program.body { let (class, stmt_start) = match stmt { @@ -3398,34 +3447,6 @@ pub fn transform_angular_file( &mut jit_edits, ); jit_classes.push(info); - - // Identifiers used only in decorator positions (member, - // `declare` member, and param decorators) survive in the - // emitted __decorate / propDecorators calls, so their imports - // must be kept — the same result TypeScript's elision - // produces on ngtsc's rewritten AST. - for name in collect_decorator_identifiers(class) { - import_elision_mut.preserve(name); - } - - // Identifiers used only in constructor parameters (types, - // param decorators) survive in the emitted ctorParameters / - // __param, so their imports must be kept — the same result - // TypeScript's elision produces on ngtsc's rewritten AST. - let mut param_idents = JitCtorParamIdentifiers::default(); - for element in &class.body.body { - if let oxc_ast::ast::ClassElement::MethodDefinition(method) = element - && method.kind == oxc_ast::ast::MethodDefinitionKind::Constructor - { - oxc_ast_visit::Visit::visit_formal_parameters( - &mut param_idents, - &method.value.params, - ); - } - } - for name in param_idents.0 { - import_elision_mut.preserve(&name); - } continue; } @@ -3516,10 +3537,11 @@ pub fn transform_angular_file( // The selector of the standalone component or directive an // `imports` entry refers to, when `@defer` may lazy-load it - // (#313): its import is referenced nowhere else in this file, - // so the static import can go, and cross-file analysis can - // read the selector. Watched files are only reported with - // `resolve_imported_values`, so it is required here too. + // (#313): its import is referenced nowhere else in this file + // or in the compiled output, so the static import can go, and + // cross-file analysis can read the selector. Watched files + // are only reported with `resolve_imported_values`, so it is + // required here too. #[cfg(feature = "cross_file_elision")] let deferrable_selector = cross_file_analyzer .as_ref() @@ -3528,7 +3550,9 @@ pub fn transform_angular_file( let import_map = &import_map; let import_elision = &import_elision_mut; move |local: &str| -> Option { - if import_elision.value_reference_count(local)? != 1 { + if import_elision.value_reference_count(local)? != 1 + || import_elision.is_kept_by_output(local) + { return None; } let info = import_map.get(&Ident::from(local))?; @@ -3567,6 +3591,7 @@ pub fn transform_angular_file( &import_map, resolved_resources, deferrable_selector, + &import_elision_mut, ) { Ok(compilation_result) => { // Update the shared pool index for the next component @@ -4931,6 +4956,7 @@ fn compile_component_full<'a>( import_map: &ImportMap<'a>, resolved_resources: Option<&ResolvedResources>, deferrable_selector: Option>, + import_elision: &ImportElisionAnalyzer<'_>, ) -> Result> { use oxc_allocator::FromIn; @@ -5084,11 +5110,26 @@ fn compile_component_full<'a>( .iter() .filter_map(|name| { let selector = deferrable_selector(name.as_str())?; - Some((name.clone(), crate::pipeline::selector::CssSelector::parse(&selector))) + Some((*name, crate::pipeline::selector::CssSelector::parse(&selector))) }) .collect(); let deferred = super::defer_resolver::implicitly_deferred_imports(&r3_result.nodes, &candidates); + // 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 deferred: std::vec::Vec<_> = deferred + .iter() + .copied() + .filter(|name| { + import_elision + .declaration_specifiers(name) + .iter() + .all(|specifier| deferred.contains(specifier)) + }) + .collect(); if !deferred.is_empty() { let is_deferred = |name: &str| deferred.iter().any(|d| d.as_str() == name); metadata.imports.retain(|name| !is_deferred(name.as_str())); 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 aff0b16f3..d8506dce8 100644 --- a/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs +++ b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs @@ -569,3 +569,321 @@ export class Parent {} ); assert_lazy(&result.code, "./lazy", "LazyCmp"); } + +// --- Import removal: names other emitted code needs, and ngc's +// per-declaration `canDefer` rule --- + +/// `app/cmps.ts` with two standalone components, `app-lazy` and `app-eager`, +/// a `TOKEN` value and a `CmpOptions` interface. +fn two_cmps(dir: &TempDir) { + create_test_file( + dir.path(), + "app/cmps.ts", + r#" +import { Component, InjectionToken } from '@angular/core'; + +@Component({ selector: 'app-lazy', template: 'lazy' }) +export class LazyCmp {} + +@Component({ selector: 'app-eager', template: 'eager' }) +export class EagerCmp {} + +export const TOKEN = new InjectionToken('token'); + +export interface CmpOptions {} +"#, + ); +} + +/// A parent deferring `LazyCmp` next to a `jit: true` class that injects it. +fn jit_sibling_source(jit_first: bool) -> String { + let jit = r#" +@Component({ selector: 'app-jit', template: '', jit: true }) +export class JitCmp { + constructor(lazy: LazyCmp) {} +} +"#; + let parent = r#" +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: '@defer { }', +}) +export class Parent {} +"#; + let (first, second) = if jit_first { (jit, parent) } else { (parent, jit) }; + format!( + "import {{ Component }} from '@angular/core';\nimport {{ LazyCmp }} from './lazy';\n{first}{second}" + ) +} + +#[test] +fn jit_sibling_ctor_parameter_keeps_import_eager_when_declared_first() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = transform(&dir, &jit_sibling_source(true)); + let code = &result.code; + assert!(compact(code).contains("{type:LazyCmp}"), "{code}"); + assert!(code.contains("import { LazyCmp }"), "{code}"); + assert_eager(code, "./lazy"); +} + +#[test] +fn jit_sibling_ctor_parameter_keeps_import_eager_when_declared_last() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = transform(&dir, &jit_sibling_source(false)); + let code = &result.code; + assert!(compact(code).contains("{type:LazyCmp}"), "{code}"); + assert!(code.contains("import { LazyCmp }"), "{code}"); + assert_eager(code, "./lazy"); +} + +#[test] +fn default_import_in_sibling_ctor_metadata_stays_eager() { + let dir = TempDir::new().unwrap(); + create_test_file( + dir.path(), + "app/lazy.ts", + r#" +import { Component } from '@angular/core'; + +@Component({ selector: 'app-lazy', template: 'lazy' }) +export default class LazyCmp {} +"#, + ); + let result = transform( + &dir, + r#" +import { Component, Injectable } from '@angular/core'; +import LazyCmp from './lazy'; + +@Injectable() +export class Svc { + constructor(lazy: LazyCmp) {} +} + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + let code = &result.code; + assert!(compact(code).contains("{type:LazyCmp}"), "{code}"); + assert!(code.contains("import LazyCmp from"), "{code}"); + assert_eager(code, "./lazy"); +} + +#[test] +fn deferred_component_injected_by_a_sibling_uses_a_namespace_import() { + let dir = TempDir::new().unwrap(); + lazy_cmp(&dir); + let result = transform( + &dir, + r#" +import { Component, Injectable } from '@angular/core'; +import { LazyCmp } from './lazy'; + +@Injectable() +export class Svc { + constructor(lazy: LazyCmp) {} +} + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + let code = &result.code; + // Like ngc: the token goes through a namespace import, so the removed + // named import leaves no dangling reference. + assert_lazy(code, "./lazy", "LazyCmp"); + assert!(!code.contains("import { LazyCmp }"), "{code}"); + assert!(code.contains("import * as i1 from './lazy'"), "{code}"); + assert!(compact(code).contains("ɵɵinject(i1.LazyCmp)"), "{code}"); +} + +#[test] +fn sibling_specifier_used_eagerly_keeps_the_declaration_eager() { + 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-parent', + imports: [LazyCmp, EagerCmp], + template: ' @defer { }', +}) +export class Parent {} +"#, + ); + let code = &result.code; + assert_eager(code, "./cmps"); + assert!(code.contains("import { LazyCmp, EagerCmp }"), "{code}"); + assert!( + compact(code).contains("ɵɵgetComponentDepsFactory(Parent,[LazyCmp,EagerCmp])"), + "{code}" + ); +} + +#[test] +fn sibling_value_specifier_keeps_the_declaration_eager() { + let dir = TempDir::new().unwrap(); + two_cmps(&dir); + let result = transform( + &dir, + r#" +import { Component, inject } from '@angular/core'; +import { LazyCmp, TOKEN } from './cmps'; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: '@defer { }', +}) +export class Parent { + token = inject(TOKEN); +} +"#, + ); + let code = &result.code; + assert_eager(code, "./cmps"); + assert!(code.contains("import { LazyCmp, TOKEN }"), "{code}"); +} + +#[test] +fn inline_type_specifier_does_not_block_deferral() { + let dir = TempDir::new().unwrap(); + two_cmps(&dir); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { type EagerCmp, LazyCmp } from './cmps'; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: '@defer { }', +}) +export class Parent { + other?: EagerCmp; +} +"#, + ); + let code = &result.code; + assert_lazy(code, "./cmps", "LazyCmp"); + assert!(!code.contains("from \"./cmps\""), "{code}"); +} + +#[test] +fn all_specifiers_deferred_removes_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-parent', + imports: [LazyCmp, EagerCmp], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + let code = &result.code; + assert_lazy(code, "./cmps", "LazyCmp"); + assert_lazy(code, "./cmps", "EagerCmp"); + assert!(compact(code).contains("ɵɵgetComponentDepsFactory(Parent,[])"), "{code}"); + assert!(!code.contains("from \"./cmps\""), "{code}"); +} + +#[test] +fn sibling_specifier_in_deferred_imports_keeps_the_declaration_eager() { + // The explicit entry's static import stays, so the declaration does too. + 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-parent', + imports: [LazyCmp], + deferredImports: [EagerCmp], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + let code = &result.code; + assert!(code.contains("import { LazyCmp, EagerCmp }"), "{code}"); + assert!(compact(code).contains("ɵɵgetComponentDepsFactory(Parent,[LazyCmp])"), "{code}"); + assert!(!code.contains("m.LazyCmp"), "{code}"); +} + +#[test] +fn sibling_interface_without_type_keeps_the_declaration_eager() { + // Like ngc, which can't tell an interface from a value here (`AssumeEager`). + let dir = TempDir::new().unwrap(); + two_cmps(&dir); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { LazyCmp, CmpOptions } from './cmps'; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: '@defer { }', +}) +export class Parent { + options?: CmpOptions; +} +"#, + ); + let code = &result.code; + assert_eager(code, "./cmps"); + assert!(compact(code).contains("ɵɵgetComponentDepsFactory(Parent,[LazyCmp])"), "{code}"); +} + +#[test] +fn computed_type_key_keeps_import_eager() { + 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], + template: '@defer { }', +}) +export class Parent { + keyed?: { [LazyCmp]: string }; +} +"#, + ); + let code = &result.code; + assert!(code.contains("import { LazyCmp }"), "{code}"); + assert_eager(code, "./lazy"); +} From be73eb9d5bf402f0c5b888b5555b66f7c523dacb Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 21 Oct 2026 12:15:58 +0800 Subject: [PATCH 4/5] fix(defer): harden the cross-file selector lookup - Read a plain top-level class only when a local `export { Local }` names it. A namespace member in the export map could point the lookup at an unexported class with the same name. - Give up on a computed key in the decorator object, since it can override `selector` or `standalone`. - Reuse the source text analyze_file kept and memoize the selector per (file, local), cleared with the other caches. Files are still reported as dependencies on a cache hit. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/component/cross_file_elision.rs | 86 +++++++++++-- .../tests/defer_implicit_imports_test.rs | 119 ++++++++++++++++++ 2 files changed, 197 insertions(+), 8 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/cross_file_elision.rs b/crates/oxc_angular_compiler/src/component/cross_file_elision.rs index da939ff48..c0ef2e396 100644 --- a/crates/oxc_angular_compiler/src/component/cross_file_elision.rs +++ b/crates/oxc_angular_compiler/src/component/cross_file_elision.rs @@ -80,6 +80,10 @@ struct Inner { /// Memoized per-export value evaluations: `(file, local_name)` -> value. /// `None` means evaluated-but-opaque. value_memo: RefCell>>, + /// Memoized standalone class selectors: `(file, local_name, + /// implicit_standalone)` -> selector (see + /// [`Self::standalone_directive_selector`]). + selector_memo: RefCell>>, /// Exports whose evaluation is in progress (circular `const` chains — /// `a.A = b.B; b.B = a.A` — resolve the cycle edge to opaque, like /// `analyzing` does for files). @@ -115,23 +119,34 @@ impl ImportValueResolver for CrossFileResolver { } } -/// The selector of the class `local` declares in `file_path` (`"default"` for -/// `export default class`), when it is a standalone `@Component` or -/// `@Directive` whose `selector` and `standalone` are literals. +/// The selector of the class `local` declares in `source` (the text of +/// `file_path`; `"default"` for `export default class`), when it is a +/// standalone `@Component` or `@Directive` whose `selector` and `standalone` +/// are literals. fn standalone_class_selector( file_path: &str, + source: &str, local: &str, implicit_standalone: bool, ) -> Option { - let source = std::fs::read_to_string(file_path).ok()?; let allocator = Allocator::default(); let source_type = SourceType::from_path(file_path).unwrap_or_default(); - let parser_ret = Parser::new(&allocator, &source, source_type).parse(); + let parser_ret = Parser::new(&allocator, source, source_type).parse(); let program = &parser_ret.program; + // A plain `class Local` must be exported by a local `export { Local }`: + // the export map also records namespace members, which can share the name + // of a top-level class the file doesn't export. + let exported_locally = program.body.iter().any(|stmt| match stmt { + Statement::ExportNamedDeclaration(decl) if !decl.export_kind.is_type() => decl + .specifiers + .iter() + .any(|spec| !spec.export_kind.is_type() && spec.local.name() == local), + _ => false, + }); let is_local = |class: &Class| class.id.as_ref().is_some_and(|id| id.name == local); let class = program.body.iter().find_map(|stmt| match stmt { - Statement::ClassDeclaration(class) if is_local(class) => Some(&**class), + Statement::ClassDeclaration(class) if exported_locally && is_local(class) => Some(&**class), Statement::ExportDeclaration(decl) => match &decl.declaration { Declaration::ClassDeclaration(class) if is_local(class) => Some(&**class), _ => None, @@ -160,7 +175,8 @@ fn standalone_class_selector( let mut standalone = implicit_standalone; for property in &object.properties { let ObjectPropertyKind::ObjectProperty(property) = property else { return None }; - let Some(key) = property.key.static_name() else { continue }; + // A computed key can override `selector` or `standalone`. + let key = property.key.static_name()?; if key == "selector" { let Expression::StringLiteral(value) = &property.value else { return None }; selector = Some(value.value.as_str()?.to_string()); @@ -200,6 +216,7 @@ impl CrossFileAnalyzer { analyzing: RefCell::new(FxHashSet::default()), sources: RefCell::new(FxHashMap::default()), value_memo: RefCell::new(FxHashMap::default()), + selector_memo: RefCell::new(FxHashMap::default()), evaluating_values: RefCell::new(FxHashSet::default()), value_dependencies: RefCell::new(FxHashSet::default()), values_enabled, @@ -453,7 +470,7 @@ impl Inner { let info = self.cache.borrow().get(&file)?.get(&name).cloned()?; match info.re_export_source { None => { - return standalone_class_selector( + return self.class_selector( &file, info.value_local.as_deref()?, implicit_standalone, @@ -471,6 +488,33 @@ impl Inner { None } + /// [`standalone_class_selector`] memoized per `(file, local)`, reading the + /// source `analyze_file` kept instead of the file again. + fn class_selector( + &self, + file_path: &str, + local: &str, + implicit_standalone: bool, + ) -> Option { + let key = (file_path.to_string(), local.to_string(), implicit_standalone); + if let Some(cached) = self.selector_memo.borrow().get(&key) { + return cached.clone(); + } + // `sources` is populated when `values_enabled`; read the file + // directly as a fallback, like `eval_export_uncached`. + let source = self + .sources + .borrow() + .get(file_path) + .cloned() + .or_else(|| std::fs::read_to_string(file_path).ok()); + let selector = source.and_then(|source| { + standalone_class_selector(file_path, &source, local, implicit_standalone) + }); + self.selector_memo.borrow_mut().insert(key, selector.clone()); + selector + } + /// Every value `import_source` exports, for `{ ...ns }` with /// `import * as ns` (ngtsc's `ResolvedModule.getExports()`): each name /// with the value it resolves to, or `None` for one that can't be read @@ -1149,6 +1193,7 @@ impl Inner { self.cache.borrow_mut().clear(); self.sources.borrow_mut().clear(); self.value_memo.borrow_mut().clear(); + self.selector_memo.borrow_mut().clear(); } /// Get the number of cached files. @@ -1866,4 +1911,29 @@ export { doSomething } from './utils'; "{normalized:?} should contain the star-exported file" ); } + + #[test] + fn test_standalone_directive_selector_is_memoized() { + // The selector is read from the source `analyze_file` kept and + // memoized, not re-read from disk on every lookup, until the caches + // are cleared. The file is still reported as a dependency each time. + let dir = TempDir::new().unwrap(); + let component = "import { Component } from '@angular/core';\n@Component({ selector: 'app-a' })\nexport class LazyCmp {}"; + let lazy = create_test_file(dir.path(), "lazy.ts", component); + let main_file = dir.path().join("main.ts"); + let mut analyzer = CrossFileAnalyzer::new(dir.path(), None, true); + let selector = |analyzer: &CrossFileAnalyzer| { + analyzer.standalone_directive_selector("./lazy", "LazyCmp", &main_file, true) + }; + + assert_eq!(selector(&analyzer).as_deref(), Some("app-a")); + assert_eq!(analyzer.take_value_dependencies().len(), 1); + + fs::write(&lazy, component.replace("app-a", "app-b")).unwrap(); + assert_eq!(selector(&analyzer).as_deref(), Some("app-a")); + assert_eq!(analyzer.take_value_dependencies().len(), 1); + + analyzer.clear_cache(); + assert_eq!(selector(&analyzer).as_deref(), Some("app-b")); + } } 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 d8506dce8..b7a8bdeba 100644 --- a/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs +++ b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs @@ -887,3 +887,122 @@ export class Parent { assert!(code.contains("import { LazyCmp }"), "{code}"); assert_eager(code, "./lazy"); } + +// --------------------------------------------------------------------------- +// Cross-file selector lookup (`cross_file_elision.rs`) +// --------------------------------------------------------------------------- + +#[test] +fn namespace_member_does_not_select_an_unexported_class() { + // `NS.LazyCmp` is recorded as the barrel's `LazyCmp` export, but the + // top-level `LazyCmp` class is not exported. The `LazyCmp` the barrel + // exports comes from `./real` (`app-real`), which is used eagerly. + let dir = TempDir::new().unwrap(); + create_test_file( + dir.path(), + "app/real.ts", + r#" +import { Component } from '@angular/core'; + +@Component({ selector: 'app-real', template: 'real' }) +export class LazyCmp {} +"#, + ); + create_test_file( + dir.path(), + "app/barrel.ts", + r#" +import { Component } from '@angular/core'; + +export * from './real'; + +namespace NS { + export class LazyCmp {} +} + +@Component({ selector: 'app-lazy', template: 'lazy' }) +class LazyCmp {} +"#, + ); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { LazyCmp } from './barrel'; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: ' @defer { }', +}) +export class Parent {} +"#, + ); + assert_eager(&result.code, "./barrel"); +} + +#[test] +fn class_exported_by_a_local_export_list_is_lazy_loaded() { + let dir = TempDir::new().unwrap(); + create_test_file( + dir.path(), + "app/lazy.ts", + r#" +import { Component } from '@angular/core'; + +@Component({ selector: 'app-lazy', template: 'lazy' }) +class Impl {} + +export { Impl as LazyCmp }; +"#, + ); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { LazyCmp } from './lazy'; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: '@defer { }', +}) +export class Parent {} +"#, + ); + assert_lazy(&result.code, "./lazy", "LazyCmp"); +} + +#[test] +fn computed_selector_key_stays_eager() { + // The computed `[S]` key overrides `selector` with `app-b`, which is + // used eagerly. + let dir = TempDir::new().unwrap(); + create_test_file( + dir.path(), + "app/lazy.ts", + r#" +import { Component } from '@angular/core'; + +const S = 'selector'; + +@Component({ selector: 'app-a', [S]: 'app-b', template: 'lazy' }) +export class LazyCmp {} +"#, + ); + let result = transform( + &dir, + r#" +import { Component } from '@angular/core'; +import { LazyCmp } from './lazy'; + +@Component({ + selector: 'app-parent', + imports: [LazyCmp], + template: ' @defer { }', +}) +export class Parent {} +"#, + ); + assert_eager(&result.code, "./lazy"); +} From 9fa794f4a76630c686c883a4c61a3073ae07859c Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 21 Oct 2026 12:32:41 +0800 Subject: [PATCH 5/5] fix(defer): match the exported name when reading a plain class selector A top-level class exported under another name could still pass the namespace-member guard, so a namespace member named like the import made the lookup read the wrong class's selector. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/component/cross_file_elision.rs | 40 +++++++------ .../tests/defer_implicit_imports_test.rs | 57 +++++++++++-------- 2 files changed, 56 insertions(+), 41 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/cross_file_elision.rs b/crates/oxc_angular_compiler/src/component/cross_file_elision.rs index c0ef2e396..6c33bbbad 100644 --- a/crates/oxc_angular_compiler/src/component/cross_file_elision.rs +++ b/crates/oxc_angular_compiler/src/component/cross_file_elision.rs @@ -80,10 +80,10 @@ struct Inner { /// Memoized per-export value evaluations: `(file, local_name)` -> value. /// `None` means evaluated-but-opaque. value_memo: RefCell>>, - /// Memoized standalone class selectors: `(file, local_name, + /// Memoized standalone class selectors: `(file, exported_name, local_name, /// implicit_standalone)` -> selector (see /// [`Self::standalone_directive_selector`]). - selector_memo: RefCell>>, + selector_memo: RefCell>>, /// Exports whose evaluation is in progress (circular `const` chains — /// `a.A = b.B; b.B = a.A` — resolve the cycle edge to opaque, like /// `analyzing` does for files). @@ -120,12 +120,13 @@ impl ImportValueResolver for CrossFileResolver { } /// The selector of the class `local` declares in `source` (the text of -/// `file_path`; `"default"` for `export default class`), when it is a -/// standalone `@Component` or `@Directive` whose `selector` and `standalone` -/// are literals. +/// `file_path`; `"default"` for `export default class`) and exports as +/// `exported`, when it is a standalone `@Component` or `@Directive` whose +/// `selector` and `standalone` are literals. fn standalone_class_selector( file_path: &str, source: &str, + exported: &str, local: &str, implicit_standalone: bool, ) -> Option { @@ -134,14 +135,18 @@ fn standalone_class_selector( let parser_ret = Parser::new(&allocator, source, source_type).parse(); let program = &parser_ret.program; - // A plain `class Local` must be exported by a local `export { Local }`: - // the export map also records namespace members, which can share the name - // of a top-level class the file doesn't export. + // A plain `class Local` must be exported as `exported` by a local + // `export { Local as exported }`: the export map also records namespace + // members, which can share the name of a top-level class the file doesn't + // export (or exports under another name). let exported_locally = program.body.iter().any(|stmt| match stmt { - Statement::ExportNamedDeclaration(decl) if !decl.export_kind.is_type() => decl - .specifiers - .iter() - .any(|spec| !spec.export_kind.is_type() && spec.local.name() == local), + Statement::ExportNamedDeclaration(decl) if !decl.export_kind.is_type() => { + decl.specifiers.iter().any(|spec| { + !spec.export_kind.is_type() + && spec.local.name() == local + && spec.exported.name() == exported + }) + } _ => false, }); let is_local = |class: &Class| class.id.as_ref().is_some_and(|id| id.name == local); @@ -472,6 +477,7 @@ impl Inner { None => { return self.class_selector( &file, + &name, info.value_local.as_deref()?, implicit_standalone, ); @@ -488,15 +494,17 @@ impl Inner { None } - /// [`standalone_class_selector`] memoized per `(file, local)`, reading the - /// source `analyze_file` kept instead of the file again. + /// [`standalone_class_selector`] memoized per `(file, exported, local)`, + /// reading the source `analyze_file` kept instead of the file again. fn class_selector( &self, file_path: &str, + exported: &str, local: &str, implicit_standalone: bool, ) -> Option { - let key = (file_path.to_string(), local.to_string(), implicit_standalone); + let key = + (file_path.to_string(), exported.to_string(), local.to_string(), implicit_standalone); if let Some(cached) = self.selector_memo.borrow().get(&key) { return cached.clone(); } @@ -509,7 +517,7 @@ impl Inner { .cloned() .or_else(|| std::fs::read_to_string(file_path).ok()); let selector = source.and_then(|source| { - standalone_class_selector(file_path, &source, local, implicit_standalone) + standalone_class_selector(file_path, &source, exported, local, implicit_standalone) }); self.selector_memo.borrow_mut().insert(key, selector.clone()); selector 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 b7a8bdeba..e66f92da6 100644 --- a/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs +++ b/crates/oxc_angular_compiler/tests/defer_implicit_imports_test.rs @@ -895,38 +895,44 @@ export class Parent { #[test] fn namespace_member_does_not_select_an_unexported_class() { // `NS.LazyCmp` is recorded as the barrel's `LazyCmp` export, but the - // top-level `LazyCmp` class is not exported. The `LazyCmp` the barrel - // exports comes from `./real` (`app-real`), which is used eagerly. - let dir = TempDir::new().unwrap(); - create_test_file( - dir.path(), - "app/real.ts", - r#" + // top-level `LazyCmp` class is not exported as `LazyCmp` (at most under + // another name). The `LazyCmp` the barrel exports comes from `./real` + // (`app-real`), which is used eagerly. + for export_list in ["", "export { LazyCmp as Alias };"] { + let dir = TempDir::new().unwrap(); + create_test_file( + dir.path(), + "app/real.ts", + r#" import { Component } from '@angular/core'; @Component({ selector: 'app-real', template: 'real' }) export class LazyCmp {} "#, - ); - create_test_file( - dir.path(), - "app/barrel.ts", - r#" -import { Component } from '@angular/core'; + ); + create_test_file( + dir.path(), + "app/barrel.ts", + &format!( + r#" +import {{ Component }} from '@angular/core'; export * from './real'; -namespace NS { - export class LazyCmp {} -} +namespace NS {{ + export class LazyCmp {{}} +}} -@Component({ selector: 'app-lazy', template: 'lazy' }) -class LazyCmp {} -"#, - ); - let result = transform( - &dir, - r#" +@Component({{ selector: 'app-lazy', template: 'lazy' }}) +class LazyCmp {{}} + +{export_list} +"# + ), + ); + let result = transform( + &dir, + r#" import { Component } from '@angular/core'; import { LazyCmp } from './barrel'; @@ -937,8 +943,9 @@ import { LazyCmp } from './barrel'; }) export class Parent {} "#, - ); - assert_eager(&result.code, "./barrel"); + ); + assert_eager(&result.code, "./barrel"); + } } #[test]