refactor(core): simplify attributes extraction logic for ComponentRef (#59678) (#59735)

pkozlowski-opensource · pkozlowski-opensource · commit caaa7ca6b8c5 · 2025-01-27T16:49:51.000+01:00
Make extractAttrsAndClassesFromSelector to return TAttributes directly to simplify the overall logic and remove unecessary code. PR Close #59678 PR Close #59735
diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts
@@ -52,7 +52,6 @@ import {ComponentDef, DirectiveDef, HostDirectiveDefs} from './interfaces/defini
 import {InputFlags} from './interfaces/input_flags';
 import {
   NodeInputBindings,
-  TAttributes,
   TContainerNode,
   TElementContainerNode,
   TElementNode,
@@ -73,9 +72,7 @@ import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
 
 import {ChainedInjector} from './chained_injector';
 import {createElementNode, setupStaticAttributes} from './dom_node_manipulation';
-import {AttributeMarker} from './interfaces/attribute_marker';
 import {unregisterLView} from './interfaces/lview_tracking';
-import {CssSelector} from './interfaces/projection';
 import {
   extractAttrsAndClassesFromSelector,
   stringifyCSSSelectorList,
@@ -157,18 +154,6 @@ function getNamespace(elementName: string): string | null {
   return name === 'svg' ? SVG_NAMESPACE : name === 'math' ? MATH_ML_NAMESPACE : null;
 }
 
-// TODO(pk): change the extractAttrsAndClassesFromSelector so it returns TAttributes already?
-function getRootTAttributesFromSelector(selector: CssSelector) {
-  const {attrs, classes} = extractAttrsAndClassesFromSelector(selector);
-
-  const tAtts: TAttributes = attrs;
-  if (classes.length) {
-    tAtts.push(AttributeMarker.Classes, ...classes);
-  }
-
-  return tAtts;
-}
-
 /**
  * ComponentFactory interface implementation.
  */
@@ -214,9 +199,7 @@ export class ComponentFactory<T> extends AbstractComponentFactory<T> {
     super();
     this.componentType = componentDef.type;
     this.selector = stringifyCSSSelectorList(componentDef.selectors);
-    this.ngContentSelectors = componentDef.ngContentSelectors
-      ? componentDef.ngContentSelectors
-      : [];
+    this.ngContentSelectors = componentDef.ngContentSelectors ?? [];
     this.isBoundToModule = !!ngModule;
   }
 
@@ -369,7 +352,7 @@ export class ComponentFactory<T> extends AbstractComponentFactory<T> {
         const tAttributes = rootSelectorOrNode
           ? ['ng-version', '0.0.0-PLACEHOLDER']
           : // Extract attributes and classes from the first selector only to match VE behavior.
-            getRootTAttributesFromSelector(this.componentDef.selectors[0]);
+            extractAttrsAndClassesFromSelector(this.componentDef.selectors[0]);
 
         // TODO: this logic is shared with the element instruction first create pass
         const hostTNode = getOrCreateTNode(
diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts
@@ -1279,7 +1279,7 @@ export function getInitialLViewFlagsFromDef(def: ComponentDef<unknown>): LViewFl
   return flags;
 }
 
-export function createComponentLView<T>(
+function createComponentLView<T>(
   lView: LView,
   hostTNode: TElementNode,
   def: ComponentDef<T>,
diff --git a/packages/core/src/render3/node_selector_matcher.ts b/packages/core/src/render3/node_selector_matcher.ts
@@ -440,11 +440,8 @@ export function stringifyCSSSelectorList(selectorList: CssSelectorList): string
  * @param selector CSS selector in parsed form (in a form of array)
  * @returns object with `attrs` and `classes` fields that contain extracted information
  */
-export function extractAttrsAndClassesFromSelector(selector: CssSelector): {
-  attrs: string[];
-  classes: string[];
-} {
-  const attrs: string[] = [];
+export function extractAttrsAndClassesFromSelector(selector: CssSelector): TAttributes {
+  const attrs: TAttributes = [];
   const classes: string[] = [];
   let i = 1;
   let mode = SelectorFlags.ATTRIBUTE;
@@ -467,5 +464,9 @@ export function extractAttrsAndClassesFromSelector(selector: CssSelector): {
     }
     i++;
   }
-  return {attrs, classes};
+  if (classes.length) {
+    attrs.push(AttributeMarker.Classes, ...classes);
+  }
+
+  return attrs;
 }
diff --git a/packages/core/test/render3/node_selector_matcher_spec.ts b/packages/core/test/render3/node_selector_matcher_spec.ts
@@ -741,8 +741,13 @@ describe('extractAttrsAndClassesFromSelector', () => {
   cases.forEach(([selector, attrs, classes]) => {
     it(`should process ${JSON.stringify(selector)} selector`, () => {
       const extracted = extractAttrsAndClassesFromSelector(selector);
-      expect(extracted.attrs).toEqual(attrs as string[]);
-      expect(extracted.classes).toEqual(classes as string[]);
+      const cssClassMarker = extracted.indexOf(AttributeMarker.Classes);
+
+      const extractedAttrs = cssClassMarker > -1 ? extracted.slice(0, cssClassMarker) : extracted;
+      const extractedClasses = cssClassMarker > -1 ? extracted.slice(cssClassMarker + 1) : [];
+
+      expect(extractedAttrs).toEqual(attrs as string[]);
+      expect(extractedClasses).toEqual(classes as string[]);
     });
   });
 });

Original file line number	Diff line number	Diff line change
`@@ -1279,7 +1279,7 @@ export function getInitialLViewFlagsFromDef(def: ComponentDef<unknown>): LViewFl`
`1279`	`1279`	`return flags;`
`1280`	`1280`	`}`
`1281`	`1281`
`1282`		`-export function createComponentLView<T>(`
	`1282`	`+function createComponentLView<T>(`
`1283`	`1283`	`lView: LView,`
`1284`	`1284`	`hostTNode: TElementNode,`
`1285`	`1285`	`def: ComponentDef<T>,`