Skip to content

Commit

Permalink
fix(core): inherit host directives (#52992)
Browse files Browse the repository at this point in the history
Adds support for inheriting host directives from the parent class. This is consistent with how we inherit other features like host bindings.

Fixes #51203.

PR Close #52992
  • Loading branch information
crisbeto authored and AndrewKushnir committed Nov 20, 2023
1 parent 29e0834 commit c7c7ea9
Show file tree
Hide file tree
Showing 5 changed files with 302 additions and 9 deletions.
8 changes: 7 additions & 1 deletion packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts
Expand Up @@ -9,7 +9,7 @@
import {Reference} from '../../imports';
import {ClassDeclaration} from '../../reflection';

import {DirectiveMeta, InputMapping, MetadataReader} from './api';
import {DirectiveMeta, HostDirectiveMeta, InputMapping, MetadataReader} from './api';
import {ClassPropertyMapping, ClassPropertyName} from './property_mapping';

/**
Expand All @@ -34,6 +34,7 @@ export function flattenInheritedDirectiveMetadata(
const undeclaredInputFields = new Set<ClassPropertyName>();
const restrictedInputFields = new Set<ClassPropertyName>();
const stringLiteralInputFields = new Set<ClassPropertyName>();
let hostDirectives: HostDirectiveMeta[]|null = null;
let isDynamic = false;
let inputs = ClassPropertyMapping.empty<InputMapping>();
let outputs = ClassPropertyMapping.empty();
Expand Down Expand Up @@ -69,6 +70,10 @@ export function flattenInheritedDirectiveMetadata(
for (const field of meta.stringLiteralInputFields) {
stringLiteralInputFields.add(field);
}
if (meta.hostDirectives !== null && meta.hostDirectives.length > 0) {
hostDirectives ??= [];
hostDirectives.push(...meta.hostDirectives);
}
};

addMetadata(topMeta);
Expand All @@ -83,5 +88,6 @@ export function flattenInheritedDirectiveMetadata(
stringLiteralInputFields,
baseClass: isDynamic ? 'dynamic' : null,
isStructural,
hostDirectives,
};
}
97 changes: 97 additions & 0 deletions packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
Expand Up @@ -3622,6 +3622,103 @@ suppress
expect(ts.flattenDiagnosticMessageText(diags[0].messageText, ''))
.toContain('HostBindDirective');
});

it('should check bindings to inherited host directive inputs', () => {
env.write('test.ts', `
import {Component, Directive, NgModule, Input} from '@angular/core';
@Directive({
standalone: true,
})
class HostDir {
@Input() input: number;
@Input() otherInput: string;
}
@Directive({
hostDirectives: [{directive: HostDir, inputs: ['input', 'otherInput: alias']}]
})
class Parent {}
@Directive({selector: '[dir]'})
class Dir extends Parent {}
@Component({
selector: 'test',
template: '<div dir [input]="person.name" [alias]="person.age"></div>',
})
class TestCmp {
person: {
name: string;
age: number;
};
}
@NgModule({
declarations: [TestCmp, Dir],
})
class Module {}
`);


const messages = env.driveDiagnostics().map(d => d.messageText);

expect(messages).toEqual([
`Type 'string' is not assignable to type 'number'.`,
`Type 'number' is not assignable to type 'string'.`
]);
});

it('should check bindings to inherited host directive outputs', () => {
env.write('test.ts', `
import {Component, Directive, NgModule, Output, EventEmitter} from '@angular/core';
@Directive({
standalone: true,
})
class HostDir {
@Output() stringEvent = new EventEmitter<string>();
@Output() numberEvent = new EventEmitter<number>();
}
@Directive({
hostDirectives: [
{directive: HostDir, outputs: ['stringEvent', 'numberEvent: numberAlias']}
]
})
class Parent {}
@Directive({selector: '[dir]'})
class Dir extends Parent {}
@Component({
selector: 'test',
template: \`
<div
dir
(numberAlias)="handleStringEvent($event)"
(stringEvent)="handleNumberEvent($event)"></div>
\`,
})
class TestCmp {
handleStringEvent(event: string): void {}
handleNumberEvent(event: number): void {}
}
@NgModule({
declarations: [TestCmp, Dir],
})
class Module {}
`);


const messages = env.driveDiagnostics().map(d => d.messageText);

expect(messages).toEqual([
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
`Argument of type 'string' is not assignable to parameter of type 'number'.`
]);
});
});

describe('deferred blocks', () => {
Expand Down
10 changes: 6 additions & 4 deletions packages/compiler/src/render3/view/compiler.ts
Expand Up @@ -118,6 +118,12 @@ function addFeatures(
break;
}
}
// Note: host directives feature needs to be inserted before the
// inheritance feature to ensure the correct execution order.
if (meta.hostDirectives?.length) {
features.push(o.importExpr(R3.HostDirectivesFeature).callFn([createHostDirectivesFeatureArg(
meta.hostDirectives)]));
}
if (meta.usesInheritance) {
features.push(o.importExpr(R3.InheritDefinitionFeature));
}
Expand All @@ -131,10 +137,6 @@ function addFeatures(
if (meta.hasOwnProperty('template') && meta.isStandalone) {
features.push(o.importExpr(R3.StandaloneFeature));
}
if (meta.hostDirectives?.length) {
features.push(o.importExpr(R3.HostDirectivesFeature).callFn([createHostDirectivesFeatureArg(
meta.hostDirectives)]));
}
if (features.length) {
definitionMap.set('features', o.literalArr(features));
}
Expand Down
15 changes: 11 additions & 4 deletions packages/core/src/render3/features/host_directives_feature.ts
Expand Up @@ -11,7 +11,7 @@ import {Type} from '../../interface/type';
import {assertEqual} from '../../util/assert';
import {EMPTY_OBJ} from '../../util/empty';
import {getComponentDef, getDirectiveDef} from '../definition';
import {DirectiveDef, HostDirectiveBindingMap, HostDirectiveDef, HostDirectiveDefs} from '../interfaces/definition';
import {DirectiveDef, DirectiveDefFeature, HostDirectiveBindingMap, HostDirectiveDef, HostDirectiveDefs} from '../interfaces/definition';

/** Values that can be used to define a host directive through the `HostDirectivesFeature`. */
type HostDirectiveConfig = Type<unknown>|{
Expand Down Expand Up @@ -42,9 +42,8 @@ type HostDirectiveConfig = Type<unknown>|{
*/
export function ɵɵHostDirectivesFeature(rawHostDirectives: HostDirectiveConfig[]|
(() => HostDirectiveConfig[])) {
return (definition: DirectiveDef<unknown>) => {
definition.findHostDirectiveDefs = findHostDirectiveDefs;
definition.hostDirectives =
const feature: DirectiveDefFeature = (definition: DirectiveDef<unknown>) => {
const resolved =
(Array.isArray(rawHostDirectives) ? rawHostDirectives : rawHostDirectives()).map(dir => {
return typeof dir === 'function' ?
{directive: resolveForwardRef(dir), inputs: EMPTY_OBJ, outputs: EMPTY_OBJ} :
Expand All @@ -54,7 +53,15 @@ export function ɵɵHostDirectivesFeature(rawHostDirectives: HostDirectiveConfig
outputs: bindingArrayToMap(dir.outputs)
};
});
if (definition.hostDirectives === null) {
definition.findHostDirectiveDefs = findHostDirectiveDefs;
definition.hostDirectives = resolved;
} else {
definition.hostDirectives.unshift(...resolved);
}
};
feature.ngInherit = true;
return feature;
}

function findHostDirectiveDefs(
Expand Down

0 comments on commit c7c7ea9

Please sign in to comment.