diff --git a/packages/core/src/render3/definition.ts b/packages/core/src/render3/definition.ts index 4c892398e0c..2904ad7381b 100644 --- a/packages/core/src/render3/definition.ts +++ b/packages/core/src/render3/definition.ts @@ -645,7 +645,7 @@ function getNgDirectiveDef(directiveDefinition: DirectiveDefinition): 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), diff --git a/packages/core/src/render3/features/host_directives_feature.ts b/packages/core/src/render3/features/host_directives_feature.ts index 3319dd07808..5a9f3f32bbe 100644 --- a/packages/core/src/render3/features/host_directives_feature.ts +++ b/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[]): HostDirectiveResolution { + const allDirectiveDefs: DirectiveDef[] = []; + 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, matchedDefs: DirectiveDef[], diff --git a/packages/core/src/render3/interfaces/definition.ts b/packages/core/src/render3/interfaces/definition.ts index 5db253a9136..072b7dd9e1b 100644 --- a/packages/core/src/render3/interfaces/definition.ts +++ b/packages/core/src/render3/interfaces/definition.ts @@ -237,21 +237,11 @@ export interface DirectiveDef { 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, - matchedDefs: DirectiveDef[], - hostDirectiveDefs: HostDirectiveDefs, - ) => void) - | null; + resolveHostDirectives: ((matches: DirectiveDef[]) => HostDirectiveResolution) | null; /** * Additional directives to be applied whenever the directive has been matched. @@ -260,8 +250,8 @@ export interface DirectiveDef { * 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[], + 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, [start: number, end: number]>; + /** Runtime information used to configure a host directive. */ export interface HostDirectiveDef { /** Class representing the host directive. */ diff --git a/packages/core/src/render3/util/discovery_utils.ts b/packages/core/src/render3/util/discovery_utils.ts index 031bf1bf7c6..ab638cc3aaa 100644 --- a/packages/core/src/render3/util/discovery_utils.ts +++ b/packages/core/src/render3/util/discovery_utils.ts @@ -441,7 +441,7 @@ function isDirectiveDefHack(obj: any): obj is DirectiveDef { return ( obj.type !== undefined && obj.declaredInputs !== undefined && - obj.findHostDirectiveDefs !== undefined + obj.resolveHostDirectives !== undefined ); } diff --git a/packages/core/src/render3/view/directives.ts b/packages/core/src/render3/view/directives.ts index eac0c7d92f8..064480180f2 100644 --- a/packages/core/src/render3/view/directives.ts +++ b/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[] | null; -/** Data produced after host directives are resolved for a node. */ -type HostDirectiveResolution = [ - matches: DirectiveDef[], - 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, [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[]; + 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[]): HostDirectiveResolution | null { - let componentDef: ComponentDef | 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[] = []; - 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, - allDirectiveDefs: DirectiveDef[], - 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. diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json index 1642ede0991..4ce3b5e3258 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -443,7 +443,6 @@ "requiresRefreshOrTraversal", "resetPreOrderHookFlags", "resolveForwardRef", - "resolveHostDirectivesForDef", "resolveTiming", "resolveTimingValue", "roundOffset", diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index 6a03346c2b2..94bdfef9e3c 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -469,7 +469,6 @@ "requiresRefreshOrTraversal", "resetPreOrderHookFlags", "resolveForwardRef", - "resolveHostDirectivesForDef", "resolveTiming", "resolveTimingValue", "roundOffset", diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index 4beed7c5061..abb7e6dbd28 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -380,7 +380,6 @@ "requiresRefreshOrTraversal", "resetPreOrderHookFlags", "resolveForwardRef", - "resolveHostDirectivesForDef", "runEffectsInView", "runInInjectionContext", "saveNameToExportMap", diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index 88fabe10c08..55dd9a78b01 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -847,7 +847,6 @@ "resetPreOrderHookFlags", "resolveDirectives", "resolveForwardRef", - "resolveHostDirectivesForDef", "retrieveHydrationInfo", "runEffectsInView", "runInInjectionContext", diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index 536bb29f488..df61ba2e912 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -570,7 +570,6 @@ "resetPreOrderHookFlags", "resolveDirectives", "resolveForwardRef", - "resolveHostDirectivesForDef", "resolveProvider", "runEffectsInView", "runInInjectionContext", diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index 5c3419bbb2a..dd564b728c8 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -561,7 +561,6 @@ "resetPreOrderHookFlags", "resolveDirectives", "resolveForwardRef", - "resolveHostDirectivesForDef", "resolveProvider", "resolvedPromise", "resolvedPromise2", diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index 4a072d397c4..14c74ae79a9 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -307,7 +307,6 @@ "requiresRefreshOrTraversal", "resetPreOrderHookFlags", "resolveForwardRef", - "resolveHostDirectivesForDef", "runEffectsInView", "runInInjectionContext", "saveNameToExportMap", diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index ee6ff0b5533..0e7a716009d 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -409,7 +409,6 @@ "requiresRefreshOrTraversal", "resetPreOrderHookFlags", "resolveForwardRef", - "resolveHostDirectivesForDef", "retrieveHydrationInfo", "retrieveHydrationInfoImpl", "runEffectsInView", diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index b9252e47cd7..78f110f17ae 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -651,7 +651,6 @@ "requiresRefreshOrTraversal", "resetPreOrderHookFlags", "resolveForwardRef", - "resolveHostDirectivesForDef", "rootRoute", "routes", "runEffectsInView", diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index 843734ae37e..b6fb1cd5638 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -340,7 +340,6 @@ "requiresRefreshOrTraversal", "resetPreOrderHookFlags", "resolveForwardRef", - "resolveHostDirectivesForDef", "runEffectsInView", "runInInjectionContext", "saveNameToExportMap", diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index a6c4e5837d4..c9a0d6ce79f 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -454,7 +454,6 @@ "resetPreOrderHookFlags", "resolveDirectives", "resolveForwardRef", - "resolveHostDirectivesForDef", "runEffectsInView", "runInInjectionContext", "saveNameToExportMap",