refactor(core): separate host directive inputs from selector-matched ones (angular#60036)

Currently `TNode.inputs`/`TNode.outputs` store all of the available bindings on that node, no matter if they came from a directive that the user applied directly or from a host directive. This has a couple of drawbacks:
1. We need to store more information that necessary. For example, the only reason we have strings in the arrays is to facilitate host directive aliasing.
2. It doesn't allow us to distinguish which host directives belong to which selector-matched directives.

These changes are a step towards resolving both issues by storing the host directive binding information in separate data structures.

PR Close angular#60036

Author: crisbeto

packages/core/src/render3/component_ref.ts

@@ -42,13 +42,7 @@ import {
 } from './instructions/shared';
 import {ComponentDef, DirectiveDef} from './interfaces/definition';
 import {InputFlags} from './interfaces/input_flags';
-import {
-  NodeInputBindings,
-  TContainerNode,
-  TElementContainerNode,
-  TElementNode,
-  TNode,
-} from './interfaces/node';
+import {TContainerNode, TElementContainerNode, TElementNode, TNode} from './interfaces/node';
 import {RElement, RNode} from './interfaces/renderer_dom';
 import {
   CONTEXT,
@@ -386,31 +380,28 @@ export class ComponentRef<T> extends AbstractComponentRef<T> {
   }
 
   override setInput(name: string, value: unknown): void {
-    const inputData = this._tNode.inputs;
-    let dataValue: NodeInputBindings[typeof name] | undefined;
-    if (inputData !== null && (dataValue = inputData[name])) {
-      this.previousInputValues ??= new Map();
-      // Do not set the input if it is the same as the last value
-      // This behavior matches `bindingUpdated` when binding inputs in templates.
-      if (
-        this.previousInputValues.has(name) &&
-        Object.is(this.previousInputValues.get(name), value)
-      ) {
-        return;
-      }
+    const tNode = this._tNode;
+    this.previousInputValues ??= new Map();
+    // Do not set the input if it is the same as the last value
+    // This behavior matches `bindingUpdated` when binding inputs in templates.
+    if (
+      this.previousInputValues.has(name) &&
+      Object.is(this.previousInputValues.get(name), value)
+    ) {
+      return;
+    }
 
-      const lView = this._rootLView;
-      setInputsForProperty(lView[TVIEW], lView, dataValue, name, value);
-      this.previousInputValues.set(name, value);
-      const childComponentLView = getComponentLViewByIndex(this._tNode.index, lView);
-      markViewDirty(childComponentLView, NotificationSource.SetInput);
-    } else {
-      if (ngDevMode) {
-        const cmpNameForError = stringifyForError(this.componentType);
-        let message = `Can't set value of the '${name}' input on the '${cmpNameForError}' component. `;
-        message += `Make sure that the '${name}' property is annotated with @Input() or a mapped @Input('${name}') exists.`;
-        reportUnknownPropertyError(message);
-      }
+    const lView = this._rootLView;
+    const hasSetInput = setInputsForProperty(tNode, lView[TVIEW], lView, name, value);
+    this.previousInputValues.set(name, value);
+    const childComponentLView = getComponentLViewByIndex(tNode.index, lView);
+    markViewDirty(childComponentLView, NotificationSource.SetInput);
+
+    if (ngDevMode && !hasSetInput) {
+      const cmpNameForError = stringifyForError(this.componentType);
+      let message = `Can't set value of the '${name}' input on the '${cmpNameForError}' component. `;
+      message += `Make sure that the '${name}' property is annotated with @Input() or a mapped @Input('${name}') exists.`;
+      reportUnknownPropertyError(message);
     }
   }
 

packages/core/src/render3/instructions/listener.ts

@@ -29,6 +29,7 @@ import {
 
 import {markViewDirty} from './mark_view_dirty';
 import {handleError, loadComponentRenderer} from './shared';
+import {DirectiveDef} from '../interfaces/definition';
 
 /**
  * Contains a reference to a function that disables event replay feature
@@ -159,7 +160,7 @@ export function listenerInternal(
 ): void {
   const isTNodeDirectiveHost = isDirectiveHost(tNode);
   const firstCreatePass = tView.firstCreatePass;
-  const tCleanup: false | any[] = firstCreatePass && getOrCreateTViewCleanup(tView);
+  const tCleanup = firstCreatePass ? getOrCreateTViewCleanup(tView) : null;
   const context = lView[CONTEXT];
 
   // When the ɵɵlistener instruction was generated and is executed we know that there is either a
@@ -230,34 +231,55 @@ export function listenerInternal(
     listenerFn = wrapListener(tNode, lView, context, listenerFn);
   }
 
-  // subscribe to directive outputs
-  const outputs = tNode.outputs;
-  let props: NodeOutputBindings[keyof NodeOutputBindings] | undefined;
-  if (processOutputs && outputs !== null && (props = outputs[eventName])) {
-    const propsLength = props.length;
-    if (propsLength) {
-      for (let i = 0; i < propsLength; i += 2) {
-        const index = props[i] as number;
-        ngDevMode && assertIndexInRange(lView, index);
-        const minifiedName = props[i + 1];
-        const directiveInstance = lView[index];
-        const output = directiveInstance[minifiedName];
+  if (processOutputs) {
+    const outputConfig = tNode.outputs?.[eventName];
+    const hostDirectiveOutputConfig = tNode.hostDirectiveOutputs?.[eventName];
 
-        if (ngDevMode && !isOutputSubscribable(output)) {
-          throw new Error(
-            `@Output ${minifiedName} not initialized in '${directiveInstance.constructor.name}'.`,
-          );
-        }
+    if (hostDirectiveOutputConfig && hostDirectiveOutputConfig.length) {
+      for (let i = 0; i < hostDirectiveOutputConfig.length; i += 2) {
+        const index = hostDirectiveOutputConfig[i] as number;
+        ngDevMode && assertIndexInRange(lView, index);
+        const instance = lView[index];
+        const lookupName = hostDirectiveOutputConfig[i + 1] as string;
+        const def = tView.data[index] as DirectiveDef<unknown>;
+        const minifiedName = def.outputs[lookupName];
+        listenToOutput(tNode, instance, eventName, minifiedName, listenerFn, lCleanup, tCleanup);
+      }
+    }
 
-        const subscription = (output as SubscribableOutput<unknown>).subscribe(listenerFn);
-        const idx = lCleanup.length;
-        lCleanup.push(listenerFn, subscription);
-        tCleanup && tCleanup.push(eventName, tNode.index, idx, -(idx + 1));
+    if (outputConfig && outputConfig.length) {
+      for (let i = 0; i < outputConfig.length; i += 2) {
+        const index = outputConfig[i] as number;
+        ngDevMode && assertIndexInRange(lView, index);
+        const instance = lView[index];
+        const minifiedName = outputConfig[i + 1] as string;
+        listenToOutput(tNode, instance, eventName, minifiedName, listenerFn, lCleanup, tCleanup);
       }
     }
   }
 }
 
+function listenToOutput(
+  tNode: TNode,
+  instance: any,
+  eventName: string,
+  propertyName: string,
+  listenerFn: (e?: any) => any,
+  lCleanup: any[],
+  tCleanup: any[] | null,
+) {
+  const output = instance[propertyName];
+
+  if (ngDevMode && !isOutputSubscribable(output)) {
+    throw new Error(`@Output ${propertyName} not initialized in '${instance.constructor.name}'.`);
+  }
+
+  const subscription = (output as SubscribableOutput<unknown>).subscribe(listenerFn);
+  const idx = lCleanup.length;
+  lCleanup.push(listenerFn, subscription);
+  tCleanup && tCleanup.push(eventName, tNode.index, idx, -(idx + 1));
+}
+
 function executeListenerWithErrorHandling(
   lView: LView,
   context: {} | null,

packages/core/src/render3/instructions/property.ts

@@ -71,8 +71,6 @@ export function setDirectiveInputsWhichShadowsStyling(
   value: any,
   isClassBased: boolean,
 ) {
-  const inputs = tNode.inputs!;
-  const property = isClassBased ? 'class' : 'style';
   // We support both 'class' and `className` hence the fallback.
-  setInputsForProperty(tView, lView, inputs[property], property, value);
+  setInputsForProperty(tNode, tView, lView, isClassBased ? 'class' : 'style', value);
 }

packages/core/src/render3/instructions/shared.ts

@@ -249,15 +249,18 @@ export function elementPropertyInternal<T>(
   nativeOnly: boolean,
 ): void {
   ngDevMode && assertNotSame(value, NO_CHANGE as any, 'Incoming value should never be NO_CHANGE.');
-  let inputData = tNode.inputs;
-  let dataValue: NodeInputBindings[typeof propName] | undefined;
-  if (!nativeOnly && inputData != null && (dataValue = inputData[propName])) {
-    setInputsForProperty(tView, lView, dataValue, propName, value);
-    if (isComponentHost(tNode)) markDirtyIfOnPush(lView, tNode.index);
-    if (ngDevMode) {
-      setNgReflectProperties(lView, tView, tNode, dataValue, value);
+
+  if (!nativeOnly) {
+    const hasSetInput = setInputsForProperty(tNode, tView, lView, propName, value);
+
+    if (hasSetInput) {
+      isComponentHost(tNode) && markDirtyIfOnPush(lView, tNode.index);
+      ngDevMode && setNgReflectProperties(lView, tView, tNode, propName, value);
+      return; // Stop propcessing if we've matched at least one input.
     }
-  } else if (tNode.type & TNodeType.AnyRNode) {
+  }
+
+  if (tNode.type & TNodeType.AnyRNode) {
     const element = getNativeByTNode(tNode, lView) as RElement | RComment;
     propName = mapPropName(propName);
 
@@ -310,15 +313,33 @@ function setNgReflectProperty(lView: LView, tNode: TNode, attrName: string, valu
   }
 }
 
-export function setNgReflectProperties(
+function setNgReflectProperties(
   lView: LView,
   tView: TView,
   tNode: TNode,
-  inputConfig: NodeInputBindings[string],
+  publicName: string,
   value: any,
 ) {
-  if (tNode.type & (TNodeType.AnyRNode | TNodeType.Container)) {
-    // Note: we set the private name of the input as the reflected property, not the public one.
+  if (!(tNode.type & (TNodeType.AnyRNode | TNodeType.Container))) {
+    return;
+  }
+
+  // TODO: this is identical to the block below, but will diverge in a future refactor.
+  // Figure out if we still can't consolidate them somehow.
+  const inputConfig = tNode.inputs?.[publicName];
+  const hostInputConfig = tNode.hostDirectiveInputs?.[publicName];
+
+  if (hostInputConfig) {
+    for (let i = 0; i < hostInputConfig.length; i += 2) {
+      const index = hostInputConfig[i] as number;
+      const publicName = hostInputConfig[i + 1] as string;
+      const def = tView.data[index] as DirectiveDef<unknown>;
+      setNgReflectProperty(lView, tNode, def.inputs[publicName][0], value);
+    }
+  }
+
+  // Note: we set the private name of the input as the reflected property, not the public one.
+  if (inputConfig) {
     for (let i = 0; i < inputConfig.length; i += 2) {
       const index = inputConfig[i] as number;
       const lookupName = inputConfig[i + 1] as string;
@@ -555,7 +576,7 @@ export function storePropertyBindingMetadata(
   // Since we don't have a concept of the "first update pass" we need to check for presence of the
   // binding meta-data to decide if one should be stored (or if was stored already).
   if (tData[bindingIndex] === null) {
-    if (tNode.inputs == null || !tNode.inputs[propertyName]) {
+    if (!tNode.inputs?.[propertyName] && !tNode.hostDirectiveInputs?.[propertyName]) {
       const propBindingIdxs = tNode.propertyBindings || (tNode.propertyBindings = []);
       propBindingIdxs.push(bindingIndex);
       let bindingMetadata = propertyName;
@@ -599,25 +620,46 @@ export function handleError(lView: LView, error: any): void {
 /**
  * Set the inputs of directives at the current node to corresponding value.
  *
+ * @param tNode TNode on which the input is being set.
  * @param tView The current TView
  * @param lView the `LView` which contains the directives.
- * @param inputs mapping between the public "input" name and privately-known,
- *        possibly minified, property names to write to.
  * @param value Value to set.
  */
 export function setInputsForProperty(
+  tNode: TNode,
   tView: TView,
   lView: LView,
-  inputs: NodeInputBindings[typeof publicName],
   publicName: string,
   value: unknown,
-): void {
-  for (let i = 0; i < inputs.length; i += 2) {
-    const index = inputs[i] as number;
-    ngDevMode && assertIndexInRange(lView, index);
-    const privateName = inputs[i + 1] as string;
-    const instance = lView[index];
-    const def = tView.data[index] as DirectiveDef<any>;
-    writeToDirectiveInput(def, instance, privateName, value);
+): boolean {
+  const inputs = tNode.inputs?.[publicName];
+  const hostDirectiveInputs = tNode.hostDirectiveInputs?.[publicName];
+  let hasMatch = false;
+
+  // TODO: this is identical to the block below, but will diverge in a future refactor.
+  // Figure out if we still can't consolidate them somehow.
+  if (hostDirectiveInputs) {
+    for (let i = 0; i < hostDirectiveInputs.length; i += 2) {
+      const index = hostDirectiveInputs[i] as number;
+      ngDevMode && assertIndexInRange(lView, index);
+      const publicName = hostDirectiveInputs[i + 1] as string;
+      const def = tView.data[index] as DirectiveDef<unknown>;
+      writeToDirectiveInput(def, lView[index], publicName, value);
+      hasMatch = true;
+    }
   }
+
+  if (inputs) {
+    for (let i = 0; i < inputs.length; i += 2) {
+      const index = inputs[i] as number;
+      ngDevMode && assertIndexInRange(lView, index);
+      const privateName = inputs[i + 1] as string;
+      const instance = lView[index];
+      const def = tView.data[index] as DirectiveDef<any>;
+      writeToDirectiveInput(def, instance, privateName, value);
+      hasMatch = true;
+    }
+  }
+
+  return hasMatch;
 }

packages/core/src/render3/interfaces/node.ts

@@ -425,12 +425,22 @@ export interface TNode {
    */
   inputs: NodeInputBindings | null;
 
+  /**
+   * Input data for host directives applied to the node.
+   */
+  hostDirectiveInputs: HostDirectiveInputs | null;
+
   /**
    * Output data for all directives on this node. `null` means that there are no directives with
    * outputs on this node.
    */
   outputs: NodeOutputBindings | null;
 
+  /**
+   * Input data for host directives applied to the node.
+   */
+  hostDirectiveOutputs: HostDirectiveOutputs | null;
+
   /**
    * The TView attached to this node.
    *
@@ -835,6 +845,26 @@ export type InitialInputData = (InitialInputs | null)[];
  */
 export type InitialInputs = string[];
 
+/**
+ * Represents inputs coming from a host directive and exposed on a TNode.
+ *
+ * - The key is the public name of an input as it is exposed on the specific node.
+ * - The value is an array where:
+ *   - i+0: Index of the host directive that should be written to.
+ *   - i+1: Public name of the input as it was defined on the host directive before aliasing.
+ */
+export type HostDirectiveInputs = Record<string, (number | string)[]>;
+
+/**
+ * Represents outputs coming from a host directive and exposed on a TNode.
+ *
+ * - The key is the public name of an output as it is exposed on the specific node.
+ * - The value is an array where:
+ *   - i+0: Index of the host directive on which the output is defined..
+ *   - i+1: Public name of the output as it was defined on the host directive before aliasing.
+ */
+export type HostDirectiveOutputs = Record<string, (number | string)[]>;
+
 /**
  * Type representing a set of TNodes that can have local refs (`#foo`) placed on them.
  */

packages/core/src/render3/tnode_manipulation.ts

@@ -293,7 +293,9 @@ export function createTNode(
     localNames: null,
     initialInputs: null,
     inputs: null,
+    hostDirectiveInputs: null,
     outputs: null,
+    hostDirectiveOutputs: null,
     tView: null,
     next: null,
     prev: null,