refactor(core): move more host directive matching logic into feature (#60452)

Moves the logic that tracks host directives into the `ɵɵHostDirectivesFeature` so it can be tree shaken.

PR Close #60452

packages/core/src/render3/definition.ts

@@ -645,7 +645,7 @@ function getNgDirectiveDef<T>(directiveDefinition: DirectiveDefinition<T>): Dire
     viewQuery: directiveDefinition.viewQuery || null,
     features: directiveDefinition.features || null,
     setInput: null,
-    findHostDirectiveDefs: null,
+    resolveHostDirectives: null,
     hostDirectives: null,
     inputs: parseAndConvertInputsForDefinition(directiveDefinition.inputs, declaredInputs),
     outputs: parseAndConvertOutputsForDefinition(directiveDefinition.outputs),

packages/core/src/render3/features/host_directives_feature.ts

@@ -10,6 +10,7 @@ import {RuntimeError, RuntimeErrorCode} from '../../errors';
 import {assertEqual} from '../../util/assert';
 import {EMPTY_OBJ} from '../../util/empty';
 import {getComponentDef, getDirectiveDef} from '../def_getters';
+import {isComponentDef} from '../interfaces/type_checks';
 import type {
   DirectiveDef,
   DirectiveDefFeature,
@@ -17,6 +18,8 @@ import type {
   HostDirectiveConfig,
   HostDirectiveDef,
   HostDirectiveDefs,
+  HostDirectiveRanges,
+  HostDirectiveResolution,
 } from '../interfaces/definition';
 
 /**
@@ -46,7 +49,7 @@ export function ɵɵHostDirectivesFeature(
     const isEager = Array.isArray(rawHostDirectives);
 
     if (definition.hostDirectives === null) {
-      definition.findHostDirectiveDefs = findHostDirectiveDefs;
+      definition.resolveHostDirectives = resolveHostDirectives;
       definition.hostDirectives = isEager
         ? rawHostDirectives.map(createHostDirectiveDef)
         : [rawHostDirectives];
@@ -60,6 +63,59 @@ export function ɵɵHostDirectivesFeature(
   return feature;
 }
 
+/**
+ * Function that will be patched onto a definition to enable host directives. It is intended to
+ * be called once during directive matching and is the same for all definitions.
+ * @param matches Directives resolved through selector matching.
+ */
+function resolveHostDirectives(matches: DirectiveDef<unknown>[]): HostDirectiveResolution {
+  const allDirectiveDefs: DirectiveDef<unknown>[] = [];
+  let hasComponent = false;
+  let hostDirectiveDefs: HostDirectiveDefs | null = null;
+  let hostDirectiveRanges: HostDirectiveRanges | null = null;
+
+  // Components are inserted at the front of the matches array so that their lifecycle
+  // hooks run before any directive lifecycle hooks. This appears to be for ViewEngine
+  // compatibility. This logic doesn't make sense with host directives, because it
+  // would allow the host directives to undo any overrides the host may have made.
+  // To handle this case, the host directives of components are inserted at the beginning
+  // of the array, followed by the component. As such, the insertion order is as follows:
+  // 1. Host directives belonging to the selector-matched component.
+  // 2. Selector-matched component.
+  // 3. Host directives belonging to selector-matched directives.
+  // 4. Selector-matched dir
+  for (let i = 0; i < matches.length; i++) {
+    const def = matches[i];
+
+    if (def.hostDirectives !== null) {
+      const start = allDirectiveDefs.length;
+
+      hostDirectiveDefs ??= new Map();
+      hostDirectiveRanges ??= new Map();
+
+      // TODO(pk): probably could return matches instead of taking in an array to fill in?
+      findHostDirectiveDefs(def, allDirectiveDefs, hostDirectiveDefs);
+
+      // Note that these indexes are within the offset by `directiveStart`. We can't do the
+      // offsetting here, because `directiveStart` hasn't been initialized on the TNode yet.
+      hostDirectiveRanges.set(def, [start, allDirectiveDefs.length - 1]);
+    }
+
+    // Component definition is always first and needs to be
+    // pushed early to maintain the correct ordering.
+    if (i === 0 && isComponentDef(def)) {
+      hasComponent = true;
+      allDirectiveDefs.push(def);
+    }
+  }
+
+  for (let i = hasComponent ? 1 : 0; i < matches.length; i++) {
+    allDirectiveDefs.push(matches[i]);
+  }
+
+  return [allDirectiveDefs, hostDirectiveDefs, hostDirectiveRanges];
+}
+
 function findHostDirectiveDefs(
   currentDef: DirectiveDef<unknown>,
   matchedDefs: DirectiveDef<unknown>[],

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

@@ -237,21 +237,11 @@ export interface DirectiveDef<T> {
   debugInfo: ClassDebugInfo | null;
 
   /**
-   * Function that will add the host directives to the list of matches during directive matching.
-   * Patched onto the definition by the `HostDirectivesFeature`.
-   * @param currentDef Definition that has been matched.
-   * @param matchedDefs List of all matches for a specified node. Will be mutated to include the
-   * host directives.
-   * @param hostDirectiveDefs Mapping of directive definitions to their host directive
-   * configuration. Host directives will be added to the map as they're being matched to the node.
+   * Function inteded to be called after template selector matching is done
+   * in order to resolve information about their host directives. Patched
+   * onto the definition by the `ɵɵHostDirectivesFeature`.
    */
-  findHostDirectiveDefs:
-    | ((
-        currentDef: DirectiveDef<unknown>,
-        matchedDefs: DirectiveDef<unknown>[],
-        hostDirectiveDefs: HostDirectiveDefs,
-      ) => void)
-    | null;
+  resolveHostDirectives: ((matches: DirectiveDef<unknown>[]) => HostDirectiveResolution) | null;
 
   /**
    * Additional directives to be applied whenever the directive has been matched.
@@ -260,8 +250,8 @@ export interface DirectiveDef<T> {
    * already pre-processed when the definition was created. A function needs to be resolved lazily
    * during directive matching, because it's a forward reference.
    *
-   * **Note:** we can't `HostDirectiveConfig` in the array, because there's no way to distinguish if
-   * a function in the array is a `Type` or a `() => HostDirectiveConfig[]`.
+   * **Note:** we can't use `HostDirectiveConfig` in the array, because there's no way to
+   * distinguish if a function in the array is a `Type` or a `() => HostDirectiveConfig[]`.
    */
   hostDirectives: (HostDirectiveDef | (() => HostDirectiveConfig[]))[] | null;
 
@@ -472,6 +462,20 @@ export interface DirectiveDefFeature {
   ngInherit?: true;
 }
 
+/** Data produced after host directives are resolved for a node. */
+export type HostDirectiveResolution = [
+  matches: DirectiveDef<unknown>[],
+  hostDirectiveDefs: HostDirectiveDefs | null,
+  hostDirectiveRanges: HostDirectiveRanges | null,
+];
+
+/**
+ * Map that tracks a selector-matched directive to the range within which its host directives
+ * are declared. Host directives for a specific directive are always contiguous within the runtime.
+ * Note that both the start and end are inclusive and they're both **after** `tNode.directiveStart`.
+ */
+export type HostDirectiveRanges = Map<DirectiveDef<unknown>, [start: number, end: number]>;
+
 /** Runtime information used to configure a host directive. */
 export interface HostDirectiveDef<T = unknown> {
   /** Class representing the host directive. */

packages/core/src/render3/util/discovery_utils.ts

@@ -441,7 +441,7 @@ function isDirectiveDefHack(obj: any): obj is DirectiveDef<any> {
   return (
     obj.type !== undefined &&
     obj.declaredInputs !== undefined &&
-    obj.findHostDirectiveDefs !== undefined
+    obj.resolveHostDirectives !== undefined
   );
 }
 

packages/core/src/render3/view/directives.ts

@@ -10,7 +10,6 @@ import {RuntimeError, RuntimeErrorCode} from '../../errors';
 import {Writable} from '../../interface/type';
 import {DoCheck, OnChanges, OnInit} from '../../interface/lifecycle_hooks';
 import {
-  assertDefined,
   assertGreaterThan,
   assertGreaterThanOrEqual,
   assertNotEqual,
@@ -26,6 +25,7 @@ import type {
   DirectiveDef,
   HostDirectiveDef,
   HostDirectiveDefs,
+  HostDirectiveRanges,
 } from '../interfaces/definition';
 import {NodeInjectorFactory} from '../interfaces/injector';
 import {
@@ -52,20 +52,6 @@ export type DirectiveMatcherStrategy = (
   tNode: TElementNode | TContainerNode | TElementContainerNode,
 ) => DirectiveDef<unknown>[] | null;
 
-/** Data produced after host directives are resolved for a node. */
-type HostDirectiveResolution = [
-  matches: DirectiveDef<unknown>[],
-  hostDirectiveDefs: HostDirectiveDefs | null,
-  hostDirectiveRanges: HostDirectiveRanges | null,
-];
-
-/**
- * Map that tracks a selector-matched directive to the range within which its host directives
- * are declared. Host directives for a specific directive are always contiguous within the runtime.
- * Note that both the start and end are inclusive and they're both **after** `tNode.directiveStart`.
- */
-type HostDirectiveRanges = Map<DirectiveDef<unknown>, [start: number, end: number]>;
-
 /**
  * Resolve the matched directives on a node.
  */
@@ -83,15 +69,16 @@ export function resolveDirectives(
   const matchedDirectiveDefs = directiveMatcher(tView, tNode);
 
   if (matchedDirectiveDefs !== null) {
-    let directiveDefs: DirectiveDef<unknown>[];
+    let directiveDefs = matchedDirectiveDefs;
     let hostDirectiveDefs: HostDirectiveDefs | null = null;
     let hostDirectiveRanges: HostDirectiveRanges | null = null;
-    const hostDirectiveResolution = resolveHostDirectives(matchedDirectiveDefs);
 
-    if (hostDirectiveResolution === null) {
-      directiveDefs = matchedDirectiveDefs;
-    } else {
-      [directiveDefs, hostDirectiveDefs, hostDirectiveRanges] = hostDirectiveResolution;
+    for (const def of matchedDirectiveDefs) {
+      if (def.resolveHostDirectives !== null) {
+        [directiveDefs, hostDirectiveDefs, hostDirectiveRanges] =
+          def.resolveHostDirectives(matchedDirectiveDefs);
+        break;
+      }
     }
 
     ngDevMode && assertNoDuplicateDirectives(directiveDefs);
@@ -133,83 +120,6 @@ function cacheMatchingLocalNames(
   }
 }
 
-function resolveHostDirectives(matches: DirectiveDef<unknown>[]): HostDirectiveResolution | null {
-  let componentDef: ComponentDef<unknown> | null = null;
-  let hasHostDirectives = false;
-
-  // Having host directives is the less common scenario. Make an initial
-  // validation pass so we don't allocate memory unnecessarily.
-  for (let i = 0; i < matches.length; i++) {
-    const def = matches[i];
-
-    // Given that we may need this further down, we can resolve it already while validating.
-    if (i === 0 && isComponentDef(def)) {
-      componentDef = def;
-    }
-
-    if (def.findHostDirectiveDefs !== null) {
-      hasHostDirectives = true;
-      break;
-    }
-  }
-
-  // If there's at least one def with host directive, we can't bail out of this function.
-  if (!hasHostDirectives) {
-    return null;
-  }
-
-  const allDirectiveDefs: DirectiveDef<unknown>[] = [];
-  let hostDirectiveDefs: HostDirectiveDefs | null = null;
-  let hostDirectiveRanges: HostDirectiveRanges | null = null;
-
-  // Components are inserted at the front of the matches array so that their lifecycle
-  // hooks run before any directive lifecycle hooks. This appears to be for ViewEngine
-  // compatibility. This logic doesn't make sense with host directives, because it
-  // would allow the host directives to undo any overrides the host may have made.
-  // To handle this case, the host directives of components are inserted at the beginning
-  // of the array, followed by the component. As such, the insertion order is as follows:
-  // 1. Host directives belonging to the selector-matched component.
-  // 2. Selector-matched component.
-  // 3. Host directives belonging to selector-matched directives.
-  // 4. Selector-matched dir
-  for (const def of matches) {
-    if (def.findHostDirectiveDefs !== null) {
-      hostDirectiveDefs ??= new Map();
-      hostDirectiveRanges ??= new Map();
-      resolveHostDirectivesForDef(def, allDirectiveDefs, hostDirectiveRanges, hostDirectiveDefs);
-    }
-
-    // Component definition needs to be pushed early to maintain the correct ordering.
-    if (def === componentDef) {
-      allDirectiveDefs.push(def);
-    }
-  }
-
-  if (componentDef === null) {
-    allDirectiveDefs.push(...matches);
-  } else {
-    allDirectiveDefs.push(...matches.slice(1));
-  }
-
-  return [allDirectiveDefs, hostDirectiveDefs, hostDirectiveRanges];
-}
-
-function resolveHostDirectivesForDef(
-  def: DirectiveDef<unknown>,
-  allDirectiveDefs: DirectiveDef<unknown>[],
-  hostDirectiveRanges: HostDirectiveRanges,
-  hostDirectiveDefs: HostDirectiveDefs,
-) {
-  ngDevMode && assertDefined(def.findHostDirectiveDefs, 'Expected host directive resolve function');
-  const start = allDirectiveDefs.length;
-  // TODO(pk): probably could return matches instead of taking in an array to fill in?
-  def.findHostDirectiveDefs!(def, allDirectiveDefs, hostDirectiveDefs);
-
-  // Note that these indexes are within the offset by `directiveStart`. We can't do the
-  // offsetting here, because `directiveStart` hasn't been initialized on the TNode yet.
-  hostDirectiveRanges.set(def, [start, allDirectiveDefs.length - 1]);
-}
-
 /**
  * Marks a given TNode as a component's host. This consists of:
  * - setting the component offset on the TNode.

packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json

@@ -443,7 +443,6 @@
   "requiresRefreshOrTraversal",
   "resetPreOrderHookFlags",
   "resolveForwardRef",
-  "resolveHostDirectivesForDef",
   "resolveTiming",
   "resolveTimingValue",
   "roundOffset",

packages/core/test/bundling/animations/bundle.golden_symbols.json

@@ -469,7 +469,6 @@
   "requiresRefreshOrTraversal",
   "resetPreOrderHookFlags",
   "resolveForwardRef",
-  "resolveHostDirectivesForDef",
   "resolveTiming",
   "resolveTimingValue",
   "roundOffset",

packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json

@@ -380,7 +380,6 @@
   "requiresRefreshOrTraversal",
   "resetPreOrderHookFlags",
   "resolveForwardRef",
-  "resolveHostDirectivesForDef",
   "runEffectsInView",
   "runInInjectionContext",
   "saveNameToExportMap",

packages/core/test/bundling/defer/bundle.golden_symbols.json

@@ -847,7 +847,6 @@
   "resetPreOrderHookFlags",
   "resolveDirectives",
   "resolveForwardRef",
-  "resolveHostDirectivesForDef",
   "retrieveHydrationInfo",
   "runEffectsInView",
   "runInInjectionContext",

packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json

@@ -570,7 +570,6 @@
   "resetPreOrderHookFlags",
   "resolveDirectives",
   "resolveForwardRef",
-  "resolveHostDirectivesForDef",
   "resolveProvider",
   "runEffectsInView",
   "runInInjectionContext",

packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json

@@ -561,7 +561,6 @@
   "resetPreOrderHookFlags",
   "resolveDirectives",
   "resolveForwardRef",
-  "resolveHostDirectivesForDef",
   "resolveProvider",
   "resolvedPromise",
   "resolvedPromise2",

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

@@ -307,7 +307,6 @@
   "requiresRefreshOrTraversal",
   "resetPreOrderHookFlags",
   "resolveForwardRef",
-  "resolveHostDirectivesForDef",
   "runEffectsInView",
   "runInInjectionContext",
   "saveNameToExportMap",

packages/core/test/bundling/hydration/bundle.golden_symbols.json

@@ -409,7 +409,6 @@
   "requiresRefreshOrTraversal",
   "resetPreOrderHookFlags",
   "resolveForwardRef",
-  "resolveHostDirectivesForDef",
   "retrieveHydrationInfo",
   "retrieveHydrationInfoImpl",
   "runEffectsInView",

packages/core/test/bundling/router/bundle.golden_symbols.json

@@ -651,7 +651,6 @@
   "requiresRefreshOrTraversal",
   "resetPreOrderHookFlags",
   "resolveForwardRef",
-  "resolveHostDirectivesForDef",
   "rootRoute",
   "routes",
   "runEffectsInView",

packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json

@@ -340,7 +340,6 @@
   "requiresRefreshOrTraversal",
   "resetPreOrderHookFlags",
   "resolveForwardRef",
-  "resolveHostDirectivesForDef",
   "runEffectsInView",
   "runInInjectionContext",
   "saveNameToExportMap",

packages/core/test/bundling/todo/bundle.golden_symbols.json

@@ -454,7 +454,6 @@
   "resetPreOrderHookFlags",
   "resolveDirectives",
   "resolveForwardRef",
-  "resolveHostDirectivesForDef",
   "runEffectsInView",
   "runInInjectionContext",
   "saveNameToExportMap",