diff --git a/packages/core/src/render3/features/host_directives_feature.ts b/packages/core/src/render3/features/host_directives_feature.ts index 868b59086678..3319dd078083 100644 --- a/packages/core/src/render3/features/host_directives_feature.ts +++ b/packages/core/src/render3/features/host_directives_feature.ts @@ -7,27 +7,18 @@ */ import {resolveForwardRef} from '../../di'; import {RuntimeError, RuntimeErrorCode} from '../../errors'; -import {Type} from '../../interface/type'; import {assertEqual} from '../../util/assert'; import {EMPTY_OBJ} from '../../util/empty'; import {getComponentDef, getDirectiveDef} from '../def_getters'; -import { +import type { DirectiveDef, DirectiveDefFeature, HostDirectiveBindingMap, + HostDirectiveConfig, HostDirectiveDef, HostDirectiveDefs, } from '../interfaces/definition'; -/** Values that can be used to define a host directive through the `HostDirectivesFeature`. */ -type HostDirectiveConfig = - | Type - | { - directive: Type; - inputs?: string[]; - outputs?: string[]; - }; - /** * This feature adds the host directives behavior to a directive definition by patching a * function onto it. The expectation is that the runtime will invoke the function during @@ -52,22 +43,17 @@ export function ɵɵHostDirectivesFeature( rawHostDirectives: HostDirectiveConfig[] | (() => HostDirectiveConfig[]), ) { const feature: DirectiveDefFeature = (definition: DirectiveDef) => { - const resolved = ( - Array.isArray(rawHostDirectives) ? rawHostDirectives : rawHostDirectives() - ).map((dir) => { - return typeof dir === 'function' - ? {directive: resolveForwardRef(dir), inputs: EMPTY_OBJ, outputs: EMPTY_OBJ} - : { - directive: resolveForwardRef(dir.directive), - inputs: bindingArrayToMap(dir.inputs), - outputs: bindingArrayToMap(dir.outputs), - }; - }); + const isEager = Array.isArray(rawHostDirectives); + if (definition.hostDirectives === null) { definition.findHostDirectiveDefs = findHostDirectiveDefs; - definition.hostDirectives = resolved; + definition.hostDirectives = isEager + ? rawHostDirectives.map(createHostDirectiveDef) + : [rawHostDirectives]; + } else if (isEager) { + definition.hostDirectives.unshift(...rawHostDirectives.map(createHostDirectiveDef)); } else { - definition.hostDirectives.unshift(...resolved); + definition.hostDirectives.unshift(rawHostDirectives); } }; feature.ngInherit = true; @@ -80,23 +66,50 @@ function findHostDirectiveDefs( hostDirectiveDefs: HostDirectiveDefs, ): void { if (currentDef.hostDirectives !== null) { - for (const hostDirectiveConfig of currentDef.hostDirectives) { - const hostDirectiveDef = getDirectiveDef(hostDirectiveConfig.directive)!; - - if (typeof ngDevMode === 'undefined' || ngDevMode) { - validateHostDirective(hostDirectiveConfig, hostDirectiveDef); + for (const configOrFn of currentDef.hostDirectives) { + if (typeof configOrFn === 'function') { + const resolved = configOrFn(); + for (const config of resolved) { + trackHostDirectiveDef(createHostDirectiveDef(config), matchedDefs, hostDirectiveDefs); + } + } else { + trackHostDirectiveDef(configOrFn, matchedDefs, hostDirectiveDefs); } + } + } +} - // We need to patch the `declaredInputs` so that - // `ngOnChanges` can map the properties correctly. - patchDeclaredInputs(hostDirectiveDef.declaredInputs, hostDirectiveConfig.inputs); +/** Tracks a single host directive during directive matching. */ +function trackHostDirectiveDef( + def: HostDirectiveDef, + matchedDefs: DirectiveDef[], + hostDirectiveDefs: HostDirectiveDefs, +) { + const hostDirectiveDef = getDirectiveDef(def.directive)!; - // Host directives execute before the host so that its host bindings can be overwritten. - findHostDirectiveDefs(hostDirectiveDef, matchedDefs, hostDirectiveDefs); - hostDirectiveDefs.set(hostDirectiveDef, hostDirectiveConfig); - matchedDefs.push(hostDirectiveDef); - } + if (typeof ngDevMode === 'undefined' || ngDevMode) { + validateHostDirective(def, hostDirectiveDef); } + + // We need to patch the `declaredInputs` so that + // `ngOnChanges` can map the properties correctly. + patchDeclaredInputs(hostDirectiveDef.declaredInputs, def.inputs); + + // Host directives execute before the host so that its host bindings can be overwritten. + findHostDirectiveDefs(hostDirectiveDef, matchedDefs, hostDirectiveDefs); + hostDirectiveDefs.set(hostDirectiveDef, def); + matchedDefs.push(hostDirectiveDef); +} + +/** Creates a `HostDirectiveDef` from a used-defined host directive configuration. */ +function createHostDirectiveDef(config: HostDirectiveConfig): HostDirectiveDef { + return typeof config === 'function' + ? {directive: resolveForwardRef(config), inputs: EMPTY_OBJ, outputs: EMPTY_OBJ} + : { + directive: resolveForwardRef(config.directive), + inputs: bindingArrayToMap(config.inputs), + outputs: bindingArrayToMap(config.outputs), + }; } /** diff --git a/packages/core/src/render3/interfaces/definition.ts b/packages/core/src/render3/interfaces/definition.ts index a0b5f8565318..0828b51a0292 100644 --- a/packages/core/src/render3/interfaces/definition.ts +++ b/packages/core/src/render3/interfaces/definition.ts @@ -260,8 +260,17 @@ export interface DirectiveDef { ) => void) | null; - /** Additional directives to be applied whenever the directive has been matched. */ - hostDirectives: HostDirectiveDef[] | null; + /** + * Additional directives to be applied whenever the directive has been matched. + * + * `HostDirectiveConfig` objects represent a host directive that can be resolved eagerly and were + * 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[]`. + */ + hostDirectives: (HostDirectiveDef | (() => HostDirectiveConfig[]))[] | null; setInput: | (( @@ -498,6 +507,15 @@ export type HostDirectiveBindingMap = { */ export type HostDirectiveDefs = Map, HostDirectiveDef>; +/** Value that can be used to configure a host directive. */ +export type HostDirectiveConfig = + | Type + | { + directive: Type; + inputs?: string[]; + outputs?: string[]; + }; + export interface ComponentDefFeature { (componentDef: ComponentDef): void; /** diff --git a/packages/core/test/acceptance/host_directives_spec.ts b/packages/core/test/acceptance/host_directives_spec.ts index a547e7607346..b023d4c1fcef 100644 --- a/packages/core/test/acceptance/host_directives_spec.ts +++ b/packages/core/test/acceptance/host_directives_spec.ts @@ -14,7 +14,6 @@ import { Directive, ElementRef, EventEmitter, - forwardRef, inject, Inject, InjectionToken, @@ -26,6 +25,8 @@ import { Type, ViewChild, ViewContainerRef, + ɵɵdefineDirective, + ɵɵHostDirectivesFeature, } from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; @@ -78,17 +79,35 @@ describe('host directives', () => { it('should apply a host directive referenced through a forwardRef', () => { const logs: string[] = []; - @Directive({ - selector: '[dir]', - hostDirectives: [forwardRef(() => HostDir), {directive: forwardRef(() => OtherHostDir)}], - standalone: false, - }) + // This directive was "compiled" manually, because our tests are JIT-compiled and the JIT + // compiler doesn't produce the callback-based variant of the `ɵɵHostDirectivesFeature`. + // This represents the following metadata: + // @Directive({ + // selector: '[dir]', + // hostDirectives: [forwardRef(() => HostDir), {directive: forwardRef(() => OtherHostDir)}], + // standalone: false, + // }) class Dir { + static ɵfac = () => new Dir(); + static ɵdir = ɵɵdefineDirective({ + type: Dir, + selectors: [['', 'dir', '']], + standalone: false, + features: [ɵɵHostDirectivesFeature(() => [HostDir, {directive: OtherHostDir}])], + }); + constructor() { logs.push('Dir'); } } + @Directive({standalone: true}) + class OtherHostDir { + constructor() { + logs.push('OtherHostDir'); + } + } + @Directive({standalone: true}) class HostDir { constructor() { @@ -96,10 +115,51 @@ describe('host directives', () => { } } - @Directive({standalone: true}) + @Component({ + template: '
', + standalone: false, + }) + class App {} + + TestBed.configureTestingModule({declarations: [App, Dir]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(logs).toEqual(['HostDir', 'OtherHostDir', 'Dir']); + }); + + it('should apply a directive that references host directives through a forwardRef and is injected by its host directives', () => { + // This directive was "compiled" manually, because our tests are JIT-compiled and the JIT + // compiler doesn't produce the callback-based variant of the `ɵɵHostDirectivesFeature`. + // This represents the following metadata: + // @Directive({ + // selector: '[dir]', + // hostDirectives: [forwardRef(() => HostDir), {directive: forwardRef(() => OtherHostDir)}], + // standalone: false, + // host: {'one': 'override', 'two': 'override'} + // }) + class Dir { + static ɵfac = () => new Dir(); + static ɵdir = ɵɵdefineDirective({ + type: Dir, + selectors: [['', 'dir', '']], + standalone: false, + hostAttrs: ['one', 'override', 'two', 'override'], + features: [ɵɵHostDirectivesFeature(() => [HostDir, {directive: OtherHostDir}])], + }); + } + + @Directive({standalone: true, host: {'one': 'base'}}) class OtherHostDir { constructor() { - logs.push('OtherHostDir'); + inject(Dir); + } + } + + @Directive({standalone: true, host: {'two': 'base'}}) + class HostDir { + constructor() { + inject(Dir); } } @@ -113,7 +173,12 @@ describe('host directives', () => { const fixture = TestBed.createComponent(App); fixture.detectChanges(); - expect(logs).toEqual(['HostDir', 'OtherHostDir', 'Dir']); + // Note: we can't use the constructor call order here to determine the initialization order, + // because the act of injecting `Dir` will cause it to be created earlier than its host bindings + // will be invoked. Instead we check that the host bindings apply in the right order. + const host = fixture.nativeElement.querySelector('[dir]'); + expect(host.getAttribute('one')).toBe('override'); + expect(host.getAttribute('two')).toBe('override'); }); it('should apply a chain of host directives', () => {