Skip to content

Commit

Permalink
feat(compiler-cli): support host directives for local compilation mode (
Browse files Browse the repository at this point in the history
angular#53877)

At the moment local compilation breaks for host directives because the current logic relies on global static analysis. This change creates a local version by cutting the diagnostics and copying the directive identifier as it is to the generated code without attempting to statically resolve it.

PR Close angular#53877
  • Loading branch information
pmvald authored and ChellappanRajan committed Jan 23, 2024
1 parent aa6b35f commit 7991cce
Show file tree
Hide file tree
Showing 11 changed files with 909 additions and 612 deletions.
1 change: 1 addition & 0 deletions goldens/public-api/compiler-cli/error_code.md
Expand Up @@ -61,6 +61,7 @@ export enum ErrorCode {
INPUT_DECLARED_ON_STATIC_MEMBER = 1100,
INTERPOLATED_SIGNAL_NOT_INVOKED = 8109,
INVALID_BANANA_IN_BOX = 8101,
LOCAL_COMPILATION_HOST_DIRECTIVE_INVALID = 11003,
LOCAL_COMPILATION_IMPORTED_STYLES_STRING = 11002,
LOCAL_COMPILATION_IMPORTED_TEMPLATE_STRING = 11001,
MISSING_CONTROL_FLOW_DIRECTIVE = 8103,
Expand Down
Expand Up @@ -10,7 +10,7 @@ import ts from 'typescript';

import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../../diagnostics';
import {Reference} from '../../../imports';
import {ClassPropertyName, DirectiveMeta, flattenInheritedDirectiveMetadata, HostDirectiveMeta, MetadataReader} from '../../../metadata';
import {ClassPropertyName, DirectiveMeta, flattenInheritedDirectiveMetadata, HostDirectiveMeta, isHostDirectiveMetaForGlobalMode, MetadataReader} from '../../../metadata';
import {describeResolvedType, DynamicValue, PartialEvaluator, ResolvedValue, traceDynamicValue} from '../../../partial_evaluator';
import {ClassDeclaration, ReflectionHost} from '../../../reflection';
import {DeclarationData, LocalModuleScopeRegistry} from '../../../scope';
Expand Down Expand Up @@ -161,6 +161,10 @@ export function validateHostDirectives(
const diagnostics: ts.DiagnosticWithLocation[] = [];

for (const current of hostDirectives) {
if (!isHostDirectiveMetaForGlobalMode(current)) {
throw new Error('Impossible state: diagnostics code path for local compilation');
}

const hostMeta = flattenInheritedDirectiveMetadata(metaReader, current.directive);

if (hostMeta === null) {
Expand Down Expand Up @@ -202,6 +206,10 @@ function validateHostDirectiveMappings(
bindingType: 'input'|'output', hostDirectiveMeta: HostDirectiveMeta, meta: DirectiveMeta,
origin: ts.Expression, diagnostics: ts.DiagnosticWithLocation[],
requiredBindings: Set<ClassPropertyName>|null) {
if (!isHostDirectiveMetaForGlobalMode(hostDirectiveMeta)) {
throw new Error('Impossible state: diagnostics code path for local compilation');
}

const className = meta.name;
const hostDirectiveMappings =
bindingType === 'input' ? hostDirectiveMeta.inputs : hostDirectiveMeta.outputs;
Expand Down
100 changes: 74 additions & 26 deletions packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts
Expand Up @@ -6,16 +6,16 @@
* found in the LICENSE file at https://angular.io/license
*/

import {createMayBeForwardRefExpression, emitDistinctChangesOnlyDefaultValue, Expression, ExternalExpr, ForwardRefHandling, getSafePropertyAccessString, MaybeForwardRefExpression, ParsedHostBindings, ParseError, parseHostBindings, R3DirectiveMetadata, R3HostDirectiveMetadata, R3InputMetadata, R3QueryMetadata, verifyHostBindings, WrappedNodeExpr} from '@angular/compiler';
import {createMayBeForwardRefExpression, emitDistinctChangesOnlyDefaultValue, Expression, ExternalExpr, ForwardRefHandling, getSafePropertyAccessString, MaybeForwardRefExpression, ParsedHostBindings, ParseError, parseHostBindings, R3DirectiveMetadata, R3HostDirectiveMetadata, R3InputMetadata, R3QueryMetadata, R3Reference, verifyHostBindings, WrappedNodeExpr} from '@angular/compiler';
import ts from 'typescript';

import {ErrorCode, FatalDiagnosticError, makeRelatedInformation} from '../../../diagnostics';
import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitter} from '../../../imports';
import {ClassPropertyMapping, DecoratorInputTransform, HostDirectiveMeta, InputMapping} from '../../../metadata';
import {ClassPropertyMapping, DecoratorInputTransform, HostDirectiveMeta, InputMapping, isHostDirectiveMetaForGlobalMode} from '../../../metadata';
import {DynamicValue, EnumValue, PartialEvaluator, ResolvedValue, traceDynamicValue} from '../../../partial_evaluator';
import {AmbientImport, ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, FunctionDefinition, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral} from '../../../reflection';
import {CompilationMode} from '../../../transform';
import {createSourceSpan, createValueHasWrongTypeError, forwardRefResolver, getConstructorDependencies, ReferencesRegistry, toR3Reference, tryUnwrapForwardRef, unwrapConstructorDependencies, unwrapExpression, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference,} from '../../common';
import {createSourceSpan, createValueHasWrongTypeError, forwardRefResolver, getConstructorDependencies, isExpressionForwardReference, ReferencesRegistry, toR3Reference, tryUnwrapForwardRef, unwrapConstructorDependencies, unwrapExpression, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference,} from '../../common';

import {tryParseSignalInputMapping} from './input_function';

Expand Down Expand Up @@ -197,15 +197,24 @@ export function extractDirectiveMetadata(
const usesInheritance = reflector.hasBaseClass(clazz);
const sourceFile = clazz.getSourceFile();
const type = wrapTypeReference(reflector, clazz);

const rawHostDirectives = directive.get('hostDirectives') || null;
const hostDirectives =
rawHostDirectives === null ? null : extractHostDirectives(rawHostDirectives, evaluator);

if (hostDirectives !== null) {
// The template type-checker will need to import host directive types, so add them
// as referenced by `clazz`. This will ensure that libraries are required to export
// host directives which are visible from publicly exported components.
referencesRegistry.add(clazz, ...hostDirectives.map(hostDir => hostDir.directive));
const hostDirectives = rawHostDirectives === null ?
null :
extractHostDirectives(rawHostDirectives, evaluator, compilationMode);

if (compilationMode !== CompilationMode.LOCAL && hostDirectives !== null) {
// In global compilation mode where we do type checking, the template type-checker will need to
// import host directive types, so add them as referenced by `clazz`. This will ensure that
// libraries are required to export host directives which are visible from publicly exported
// components.
referencesRegistry.add(clazz, ...hostDirectives.map(hostDir => {
if (!isHostDirectiveMetaForGlobalMode(hostDir)) {
throw new Error('Impossible state');
}

return hostDir.directive;
}));
}

const metadata: R3DirectiveMetadata = {
Expand Down Expand Up @@ -1048,7 +1057,8 @@ function evaluateHostExpressionBindings(
* @param rawHostDirectives Expression that defined the `hostDirectives`.
*/
function extractHostDirectives(
rawHostDirectives: ts.Expression, evaluator: PartialEvaluator): HostDirectiveMeta[] {
rawHostDirectives: ts.Expression, evaluator: PartialEvaluator,
compilationMode: CompilationMode): HostDirectiveMeta[] {
const resolved = evaluator.evaluate(rawHostDirectives, forwardRefResolver);
if (!Array.isArray(resolved)) {
throw createValueHasWrongTypeError(
Expand All @@ -1058,21 +1068,50 @@ function extractHostDirectives(
return resolved.map(value => {
const hostReference = value instanceof Map ? value.get('directive') : value;

if (!(hostReference instanceof Reference)) {
throw createValueHasWrongTypeError(
rawHostDirectives, hostReference, 'Host directive must be a reference');
// Diagnostics
if (compilationMode !== CompilationMode.LOCAL) {
if (!(hostReference instanceof Reference)) {
throw createValueHasWrongTypeError(
rawHostDirectives, hostReference, 'Host directive must be a reference');
}

if (!isNamedClassDeclaration(hostReference.node)) {
throw createValueHasWrongTypeError(
rawHostDirectives, hostReference, 'Host directive reference must be a class');
}
}

if (!isNamedClassDeclaration(hostReference.node)) {
throw createValueHasWrongTypeError(
rawHostDirectives, hostReference, 'Host directive reference must be a class');
let directive: Reference<ClassDeclaration>|Expression;
let nameForErrors = (fieldName: string) => '@Directive.hostDirectives';
if (compilationMode === CompilationMode.LOCAL && hostReference instanceof DynamicValue) {
// At the moment in local compilation we only support simple array for host directives, i.e.,
// an array consisting of the directive identifiers. We don't support forward refs or other
// expressions applied on externally imported directives. The main reason is simplicity, and
// that almost nobody wants to use host directives this way (e.g., what would be the point of
// forward ref for imported symbols?!)
if (!ts.isIdentifier(hostReference.node) &&
!ts.isPropertyAccessExpression(hostReference.node)) {
throw new FatalDiagnosticError(
ErrorCode.LOCAL_COMPILATION_HOST_DIRECTIVE_INVALID, hostReference.node,
`In local compilation mode, host directive cannot be an expression`);
}

directive = new WrappedNodeExpr(hostReference.node);
} else if (hostReference instanceof Reference) {
directive = hostReference as Reference<ClassDeclaration>;
nameForErrors = (fieldName: string) => `@Directive.hostDirectives.${
(directive as Reference<ClassDeclaration>).node.name.text}.${fieldName}`;
} else {
throw new Error('Impossible state');
}

const meta: HostDirectiveMeta = {
directive: hostReference as Reference<ClassDeclaration>,
isForwardReference: hostReference.synthetic,
inputs: parseHostDirectivesMapping('inputs', value, hostReference.node, rawHostDirectives),
outputs: parseHostDirectivesMapping('outputs', value, hostReference.node, rawHostDirectives),
directive,
isForwardReference: hostReference instanceof Reference && hostReference.synthetic,
inputs:
parseHostDirectivesMapping('inputs', value, nameForErrors('input'), rawHostDirectives),
outputs:
parseHostDirectivesMapping('outputs', value, nameForErrors('output'), rawHostDirectives),
};

return meta;
Expand All @@ -1087,10 +1126,9 @@ function extractHostDirectives(
* @param sourceExpression Expression that the host directive is referenced in.
*/
function parseHostDirectivesMapping(
field: 'inputs'|'outputs', resolvedValue: ResolvedValue, classReference: ClassDeclaration,
field: 'inputs'|'outputs', resolvedValue: ResolvedValue, nameForErrors: string,
sourceExpression: ts.Expression): {[bindingPropertyName: string]: string}|null {
if (resolvedValue instanceof Map && resolvedValue.has(field)) {
const nameForErrors = `@Directive.hostDirectives.${classReference.name.text}.${field}`;
const rawInputs = resolvedValue.get(field);

if (isStringArrayOrDie(rawInputs, nameForErrors, sourceExpression)) {
Expand All @@ -1105,9 +1143,19 @@ function parseHostDirectivesMapping(
function toHostDirectiveMetadata(
hostDirective: HostDirectiveMeta, context: ts.SourceFile,
refEmitter: ReferenceEmitter): R3HostDirectiveMetadata {
let directive: R3Reference;
if (hostDirective.directive instanceof Reference) {
directive =
toR3Reference(hostDirective.directive.node, hostDirective.directive, context, refEmitter);
} else {
directive = {
value: hostDirective.directive,
type: hostDirective.directive,
};
}

return {
directive:
toR3Reference(hostDirective.directive.node, hostDirective.directive, context, refEmitter),
directive,
isForwardReference: hostDirective.isForwardReference,
inputs: hostDirective.inputs || null,
outputs: hostDirective.outputs || null
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
Expand Up @@ -479,4 +479,10 @@ export enum ErrorCode {
* compilation mode.
*/
LOCAL_COMPILATION_IMPORTED_STYLES_STRING = 11002,

/**
* Raised when the compiler wasn't able to resolve the metadata of a host directive in local
* compilation mode.
*/
LOCAL_COMPILATION_HOST_DIRECTIVE_INVALID = 11003,
}
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/ngtsc/metadata/index.ts
Expand Up @@ -11,7 +11,7 @@ export {DtsMetadataReader} from './src/dts';
export {flattenInheritedDirectiveMetadata} from './src/inheritance';
export {CompoundMetadataRegistry, LocalMetadataRegistry} from './src/registry';
export {ResourceRegistry, Resource, ComponentResources, isExternalResource, ExternalResource} from './src/resource_registry';
export {extractDirectiveTypeCheckMeta, hasInjectableFields, CompoundMetadataReader} from './src/util';
export {extractDirectiveTypeCheckMeta, hasInjectableFields, CompoundMetadataReader, isHostDirectiveMetaForGlobalMode} from './src/util';
export {BindingPropertyName, ClassPropertyMapping, ClassPropertyName, InputOrOutput} from './src/property_mapping';
export {ExportedProviderStatusResolver} from './src/providers';
export {HostDirectivesResolver} from './src/host_directives_resolver';
28 changes: 25 additions & 3 deletions packages/compiler-cli/src/ngtsc/metadata/src/api.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {DirectiveMeta as T2DirectiveMeta, SchemaMetadata} from '@angular/compiler';
import {DirectiveMeta as T2DirectiveMeta, Expression, SchemaMetadata} from '@angular/compiler';
import ts from 'typescript';

import {Reference} from '../../imports';
Expand Down Expand Up @@ -261,8 +261,14 @@ export interface DirectiveMeta extends T2DirectiveMeta, DirectiveTypeCheckMeta {

/** Metadata collected about an additional directive that is being applied to a directive host. */
export interface HostDirectiveMeta {
/** Reference to the host directive class. */
directive: Reference<ClassDeclaration>;
/**
* Reference to the host directive class.
*
* Only in local compilation mode this can be Expression
* which indicates the expression could not be resolved due to being imported from some external
* file. In this case, the expression is the raw expression as appears in the decorator.
*/
directive: Reference<ClassDeclaration>|Expression;

/** Whether the reference to the host directive is a forward reference. */
isForwardReference: boolean;
Expand All @@ -274,6 +280,22 @@ export interface HostDirectiveMeta {
outputs: {[publicName: string]: string}|null;
}

/**
* Metadata collected about an additional directive that is being applied to a directive host in
* global compilation mode.
*/
export interface HostDirectiveMetaForGlobalMode extends HostDirectiveMeta {
directive: Reference<ClassDeclaration>;
}

/**
* Metadata collected about an additional directive that is being applied to a directive host in
* local compilation mode.
*/
export interface HostDirectiveMetaForLocalMode extends HostDirectiveMeta {
directive: Expression;
}

/**
* Metadata that describes a template guard for one of the directive's inputs.
*/
Expand Down
Expand Up @@ -11,6 +11,7 @@ import {ClassDeclaration} from '../../reflection';
import {ClassPropertyMapping, InputOrOutput} from '../src/property_mapping';

import {flattenInheritedDirectiveMetadata} from './inheritance';
import {isHostDirectiveMetaForGlobalMode} from './util';

const EMPTY_ARRAY: ReadonlyArray<any> = [];

Expand Down Expand Up @@ -41,6 +42,10 @@ export class HostDirectivesResolver {
directives: NonNullable<DirectiveMeta['hostDirectives']>,
results: DirectiveMeta[]): ReadonlyArray<DirectiveMeta> {
for (const current of directives) {
if (!isHostDirectiveMetaForGlobalMode(current)) {
throw new Error('Impossible state: resolving code path in local compilation mode');
}

const hostMeta = flattenInheritedDirectiveMetadata(this.metaReader, current.directive);

// This case has been checked for already and produces a diagnostic
Expand Down
7 changes: 6 additions & 1 deletion packages/compiler-cli/src/ngtsc/metadata/src/util.ts
Expand Up @@ -12,7 +12,7 @@ import {OwningModule, Reference} from '../../imports';
import {ClassDeclaration, ClassMember, ClassMemberKind, isNamedClassDeclaration, ReflectionHost, reflectTypeEntityToDeclaration} from '../../reflection';
import {nodeDebugInfo} from '../../util/src/typescript';

import {DirectiveMeta, DirectiveTypeCheckMeta, InputMapping, MetadataReader, NgModuleMeta, PipeMeta, TemplateGuardMeta} from './api';
import {DirectiveMeta, DirectiveTypeCheckMeta, HostDirectiveMeta, HostDirectiveMetaForGlobalMode, InputMapping, MetadataReader, NgModuleMeta, PipeMeta, TemplateGuardMeta} from './api';
import {ClassPropertyMapping, ClassPropertyName} from './property_mapping';

export function extractReferencesFromType(
Expand Down Expand Up @@ -257,3 +257,8 @@ export function hasInjectableFields(clazz: ClassDeclaration, host: ReflectionHos
const members = host.getMembersOfClass(clazz);
return members.some(({isStatic, name}) => isStatic && (name === 'ɵprov' || name === 'ɵfac'));
}

export function isHostDirectiveMetaForGlobalMode(hostDirectiveMeta: HostDirectiveMeta):
hostDirectiveMeta is HostDirectiveMetaForGlobalMode {
return hostDirectiveMeta.directive instanceof Reference;
}
15 changes: 9 additions & 6 deletions packages/compiler-cli/src/ngtsc/metadata/test/dts_spec.ts
Expand Up @@ -12,6 +12,7 @@ import {OwningModule, Reference} from '../../imports';
import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
import {loadFakeCore, makeProgram} from '../../testing';
import {DtsMetadataReader} from '../src/dts';
import {isHostDirectiveMetaForGlobalMode} from '../src/util';

runInEachFileSystem(() => {
beforeEach(() => {
Expand Down Expand Up @@ -216,12 +217,14 @@ runInEachFileSystem(() => {
const dtsReader = new DtsMetadataReader(typeChecker, new TypeScriptReflectionHost(typeChecker));

const meta = dtsReader.getDirectiveMetadata(new Reference(clazz))!;
const hostDirectives = meta.hostDirectives?.map(hostDir => ({
name: hostDir.directive.debugName,
directive: hostDir.directive,
inputs: hostDir.inputs,
outputs: hostDir.outputs
}));
const hostDirectives = meta.hostDirectives?.map(
hostDir => ({
name: isHostDirectiveMetaForGlobalMode(hostDir) ? hostDir.directive.debugName :
'Unresolved host dir',
directive: hostDir.directive,
inputs: hostDir.inputs,
outputs: hostDir.outputs
}));

expect(hostDirectives).toEqual([
{
Expand Down
Expand Up @@ -6,12 +6,12 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AST, ASTWithSource, BindingPipe, Call, ParseSourceSpan, PropertyRead, PropertyWrite, SafePropertyRead, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
import {AST, ASTWithSource, BindingPipe, ParseSourceSpan, PropertyRead, PropertyWrite, SafePropertyRead, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
import ts from 'typescript';

import {AbsoluteFsPath} from '../../file_system';
import {Reference} from '../../imports';
import {HostDirectiveMeta} from '../../metadata';
import {HostDirectiveMeta, isHostDirectiveMetaForGlobalMode} from '../../metadata';
import {ClassDeclaration} from '../../reflection';
import {ComponentScopeKind, ComponentScopeReader} from '../../scope';
import {isAssignment, isSymbolWithValueDeclaration} from '../../util/src/typescript';
Expand Down Expand Up @@ -161,6 +161,10 @@ export class SymbolBuilder {
host: TmplAstTemplate|TmplAstElement, hostDirectives: HostDirectiveMeta[],
symbols: DirectiveSymbol[]): void {
for (const current of hostDirectives) {
if (!isHostDirectiveMetaForGlobalMode(current)) {
throw new Error('Impossible state: typecheck code path in local compilation mode.');
}

if (!ts.isClassDeclaration(current.directive.node)) {
continue;
}
Expand Down

0 comments on commit 7991cce

Please sign in to comment.