diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index fe518bbd29c6d..98a9651353a3a 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -15,7 +15,7 @@ import {absoluteFrom, relative} from '../../file_system'; import {DefaultImportRecorder, ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; import {DependencyTracker} from '../../incremental/api'; import {IndexingContext} from '../../indexer'; -import {DirectiveMeta, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata'; +import {ClassPropertyMapping, DirectiveMeta, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata'; import {flattenInheritedDirectiveMetadata} from '../../metadata/src/inheritance'; import {EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; @@ -56,6 +56,9 @@ export interface ComponentAnalysisData { template: ParsedTemplateWithSource; metadataStmt: Statement|null; + inputs: ClassPropertyMapping; + outputs: ClassPropertyMapping; + /** * Providers extracted from the `providers` field of the component annotation which will require * an Angular factory definition at runtime. @@ -191,7 +194,7 @@ export class ComponentDecoratorHandler implements } // Next, read the `@Component`-specific fields. - const {decorator: component, metadata} = directiveResult; + const {decorator: component, metadata, inputs, outputs} = directiveResult; // Go through the root directories for this project, and select the one with the smallest // relative path representation. @@ -328,6 +331,8 @@ export class ComponentDecoratorHandler implements const output: AnalysisOutput = { analysis: { baseClass: readBaseClass(node, this.reflector, this.evaluator), + inputs, + outputs, meta: { ...metadata, template: { @@ -345,7 +350,7 @@ export class ComponentDecoratorHandler implements i18nUseExternalIds: this.i18nUseExternalIds, relativeContextFilePath, }, - typeCheckMeta: extractDirectiveTypeCheckMeta(node, metadata.inputs, this.reflector), + typeCheckMeta: extractDirectiveTypeCheckMeta(node, inputs, this.reflector), metadataStmt: generateSetClassMetadataCall( node, this.reflector, this.defaultImportRecorder, this.isCore, this.annotateForClosureCompiler), @@ -370,8 +375,8 @@ export class ComponentDecoratorHandler implements name: node.name.text, selector: analysis.meta.selector, exportAs: analysis.meta.exportAs, - inputs: analysis.meta.inputs, - outputs: analysis.meta.outputs, + inputs: analysis.inputs, + outputs: analysis.outputs, queries: analysis.meta.queries.map(query => query.propertyName), isComponent: true, baseClass: analysis.baseClass, diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index 6bdd3398a7654..e85af13159fef 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {DefaultImportRecorder, Reference} from '../../imports'; -import {DirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata'; +import {ClassPropertyMapping, DirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata'; import {extractDirectiveTypeCheckMeta} from '../../metadata/src/util'; import {DynamicValue, EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; @@ -39,6 +39,8 @@ export interface DirectiveHandlerData { meta: R3DirectiveMetadata; metadataStmt: Statement|null; providersRequiringFactory: Set>|null; + inputs: ClassPropertyMapping; + outputs: ClassPropertyMapping; } export class DirectiveDecoratorHandler implements @@ -83,11 +85,10 @@ export class DirectiveDecoratorHandler implements const directiveResult = extractDirectiveMetadata( node, decorator, this.reflector, this.evaluator, this.defaultImportRecorder, this.isCore, flags, this.annotateForClosureCompiler); - const analysis = directiveResult && directiveResult.metadata; - - if (analysis === undefined) { + if (directiveResult === undefined) { return {}; } + const analysis = directiveResult.metadata; let providersRequiringFactory: Set>|null = null; if (directiveResult !== undefined && directiveResult.decorator.has('providers')) { @@ -97,12 +98,14 @@ export class DirectiveDecoratorHandler implements return { analysis: { + inputs: directiveResult.inputs, + outputs: directiveResult.outputs, meta: analysis, metadataStmt: generateSetClassMetadataCall( node, this.reflector, this.defaultImportRecorder, this.isCore, this.annotateForClosureCompiler), baseClass: readBaseClass(node, this.reflector, this.evaluator), - typeCheckMeta: extractDirectiveTypeCheckMeta(node, analysis.inputs, this.reflector), + typeCheckMeta: extractDirectiveTypeCheckMeta(node, directiveResult.inputs, this.reflector), providersRequiringFactory } }; @@ -117,8 +120,8 @@ export class DirectiveDecoratorHandler implements name: node.name.text, selector: analysis.meta.selector, exportAs: analysis.meta.exportAs, - inputs: analysis.meta.inputs, - outputs: analysis.meta.outputs, + inputs: analysis.inputs, + outputs: analysis.outputs, queries: analysis.meta.queries.map(query => query.propertyName), isComponent: false, baseClass: analysis.baseClass, @@ -199,8 +202,13 @@ export class DirectiveDecoratorHandler implements export function extractDirectiveMetadata( clazz: ClassDeclaration, decorator: Readonly, reflector: ReflectionHost, evaluator: PartialEvaluator, defaultImportRecorder: DefaultImportRecorder, isCore: boolean, - flags: HandlerFlags, annotateForClosureCompiler: boolean, defaultSelector: string|null = null): - {decorator: Map, metadata: R3DirectiveMetadata}|undefined { + flags: HandlerFlags, annotateForClosureCompiler: boolean, + defaultSelector: string|null = null): { + decorator: Map, + metadata: R3DirectiveMetadata, + inputs: ClassPropertyMapping, + outputs: ClassPropertyMapping, +}|undefined { let directive: Map; if (decorator === null || decorator.args === null || decorator.args.length === 0) { directive = new Map(); @@ -331,6 +339,9 @@ export function extractDirectiveMetadata( const type = wrapTypeReference(reflector, clazz); const internalType = new WrappedNodeExpr(reflector.getInternalNameOfClass(clazz)); + const inputs = ClassPropertyMapping.fromMappedObject({...inputsFromMeta, ...inputsFromFields}); + const outputs = ClassPropertyMapping.fromMappedObject({...outputsFromMeta, ...outputsFromFields}); + const metadata: R3DirectiveMetadata = { name: clazz.name.text, deps: ctorDeps, @@ -338,8 +349,8 @@ export function extractDirectiveMetadata( lifecycle: { usesOnChanges, }, - inputs: {...inputsFromMeta, ...inputsFromFields}, - outputs: {...outputsFromMeta, ...outputsFromFields}, + inputs: inputs.toJointMappedObject(), + outputs: outputs.toDirectMappedObject(), queries, viewQueries, selector, @@ -352,7 +363,12 @@ export function extractDirectiveMetadata( exportAs, providers }; - return {decorator: directive, metadata}; + return { + decorator: directive, + metadata, + inputs, + outputs, + }; } export function extractQueryMetadata( diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts index bfdee6b4b1478..1eed4de0100ad 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts @@ -5,6 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {CssSelector, DirectiveMeta as T2DirectiveMeta, parseTemplate, R3TargetBinder, SelectorMatcher, TmplAstElement} from '@angular/compiler'; import * as ts from 'typescript'; import {absoluteFrom} from '../../file_system'; @@ -73,6 +74,49 @@ runInEachFileSystem(() => { expect(span.start.toString()).toContain('/entry.ts@5:22'); expect(span.end.toString()).toContain('/entry.ts@5:29'); }); + + it('should produce metadata compatible with template binding', () => { + const src = ` + import {Directive, Input} from '@angular/core'; + + @Directive({selector: '[dir]'}) + export class TestDir { + @Input('propName') + fieldName: string; + } + `; + const {program} = makeProgram([ + { + name: _('/node_modules/@angular/core/index.d.ts'), + contents: 'export const Directive: any; export const Input: any;', + }, + { + name: _('/entry.ts'), + contents: src, + }, + ]); + + const analysis = analyzeDirective(program, 'TestDir'); + const matcher = new SelectorMatcher(); + const dirMeta: T2DirectiveMeta = { + exportAs: null, + inputs: analysis.inputs, + outputs: analysis.outputs, + isComponent: false, + name: 'Dir', + }; + matcher.addSelectables(CssSelector.parse('[dir]'), dirMeta); + + const {nodes} = parseTemplate('
', 'unimportant.html'); + const binder = new R3TargetBinder(matcher).bind({template: nodes}); + const propBinding = (nodes[0] as TmplAstElement).inputs[0]; + const propBindingConsumer = binder.getConsumerOfBinding(propBinding); + + // Assert that the consumer of the binding is the directive, which means that the metadata + // fed into the SelectorMatcher was compatible with the binder, and did not confuse property + // and field names. + expect(propBindingConsumer).toBe(dirMeta); + }); }); // Helpers diff --git a/packages/compiler-cli/src/ngtsc/indexer/test/util.ts b/packages/compiler-cli/src/ngtsc/indexer/test/util.ts index 4e16da6e111ca..5013f4652c27e 100644 --- a/packages/compiler-cli/src/ngtsc/indexer/test/util.ts +++ b/packages/compiler-cli/src/ngtsc/indexer/test/util.ts @@ -11,6 +11,7 @@ import * as ts from 'typescript'; import {absoluteFrom, AbsoluteFsPath} from '../../file_system'; import {Reference} from '../../imports'; +import {ClassPropertyMapping} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; import {getDeclaration, makeProgram} from '../../testing'; import {ComponentMeta} from '../src/context'; @@ -50,8 +51,8 @@ export function getBoundTemplate( selector, name: declaration.name.getText(), isComponent: true, - inputs: {}, - outputs: {}, + inputs: ClassPropertyMapping.fromMappedObject({}), + outputs: ClassPropertyMapping.fromMappedObject({}), exportAs: null, }); }); diff --git a/packages/compiler-cli/src/ngtsc/metadata/index.ts b/packages/compiler-cli/src/ngtsc/metadata/index.ts index ee3f910b1bb71..2f857eee0bc40 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/index.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/index.ts @@ -10,3 +10,4 @@ export * from './src/api'; export {DtsMetadataReader} from './src/dts'; export {CompoundMetadataRegistry, LocalMetadataRegistry, InjectableClassRegistry} from './src/registry'; export {extractDirectiveTypeCheckMeta, CompoundMetadataReader} from './src/util'; +export {BindingPropertyName, ClassPropertyMapping, ClassPropertyName, InputOrOutput} from './src/property_mapping'; diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts index 8009207f50aa9..3ec001db9578d 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts @@ -12,6 +12,8 @@ import * as ts from 'typescript'; import {Reference} from '../../imports'; import {ClassDeclaration} from '../../reflection'; +import {ClassPropertyMapping, ClassPropertyName} from './property_mapping'; + /** * Metadata collected for an `NgModule`. @@ -52,25 +54,25 @@ export interface DirectiveTypeCheckMeta { * Directive's class. This allows inputs to accept a wider range of types and coerce the input to * a narrower type with a getter/setter. See https://angular.io/guide/template-typecheck. */ - coercedInputFields: Set; + coercedInputFields: Set; /** * The set of input fields which map to `readonly`, `private`, or `protected` members in the * Directive's class. */ - restrictedInputFields: Set; + restrictedInputFields: Set; /** * The set of input fields which are declared as string literal members in the Directive's class. * We need to track these separately because these fields may not be valid JS identifiers so * we cannot use them with property access expressions when assigning inputs. */ - stringLiteralInputFields: Set; + stringLiteralInputFields: Set; /** * The set of input fields which do not have corresponding members in the Directive's class. */ - undeclaredInputFields: Set; + undeclaredInputFields: Set; /** * Whether the Directive's class is generic, i.e. `class MyDir {...}`. @@ -89,6 +91,16 @@ export interface DirectiveMeta extends T2DirectiveMeta, DirectiveTypeCheckMeta { selector: string|null; queries: string[]; + /** + * A mapping of input field names to the property names. + */ + inputs: ClassPropertyMapping; + + /** + * A mapping of output field names to the property names. + */ + outputs: ClassPropertyMapping; + /** * A `Reference` to the base class for the directive, if one was detected. * diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts index faf588ab67d4f..4bbdc14fd8acf 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts @@ -12,6 +12,7 @@ import {Reference} from '../../imports'; import {ClassDeclaration, isNamedClassDeclaration, ReflectionHost} from '../../reflection'; import {DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta} from './api'; +import {ClassPropertyMapping} from './property_mapping'; import {extractDirectiveTypeCheckMeta, extractReferencesFromType, readStringArrayType, readStringMapType, readStringType} from './util'; /** @@ -76,7 +77,10 @@ export class DtsMetadataReader implements MetadataReader { return null; } - const inputs = readStringMapType(def.type.typeArguments[3]); + const inputs = + ClassPropertyMapping.fromMappedObject(readStringMapType(def.type.typeArguments[3])); + const outputs = + ClassPropertyMapping.fromMappedObject(readStringMapType(def.type.typeArguments[4])); return { ref, name: clazz.name.text, @@ -84,7 +88,7 @@ export class DtsMetadataReader implements MetadataReader { selector: readStringType(def.type.typeArguments[1]), exportAs: readStringArrayType(def.type.typeArguments[2]), inputs, - outputs: readStringMapType(def.type.typeArguments[4]), + outputs, queries: readStringArrayType(def.type.typeArguments[5]), ...extractDirectiveTypeCheckMeta(clazz, inputs, this.reflector), baseClass: readBaseClass(clazz, this.checker, this.reflector), diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts index 455d103d7dde9..f5271f4351d32 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts @@ -7,9 +7,11 @@ */ import {Reference} from '../../imports'; -import {DirectiveMeta, MetadataReader} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; +import {DirectiveMeta, MetadataReader} from './api'; +import {ClassPropertyMapping, ClassPropertyName} from './property_mapping'; + /** * Given a reference to a directive, return a flattened version of its `DirectiveMeta` metadata * which includes metadata from its entire inheritance chain. @@ -25,13 +27,13 @@ export function flattenInheritedDirectiveMetadata( throw new Error(`Metadata not found for directive: ${dir.debugName}`); } - let inputs: {[key: string]: string|[string, string]} = {}; - let outputs: {[key: string]: string} = {}; - const coercedInputFields = new Set(); - const undeclaredInputFields = new Set(); - const restrictedInputFields = new Set(); - const stringLiteralInputFields = new Set(); + const coercedInputFields = new Set(); + const undeclaredInputFields = new Set(); + const restrictedInputFields = new Set(); + const stringLiteralInputFields = new Set(); let isDynamic = false; + let inputs = ClassPropertyMapping.empty(); + let outputs = ClassPropertyMapping.empty(); const addMetadata = (meta: DirectiveMeta): void => { if (meta.baseClass === 'dynamic') { @@ -45,8 +47,9 @@ export function flattenInheritedDirectiveMetadata( isDynamic = true; } } - inputs = {...inputs, ...meta.inputs}; - outputs = {...outputs, ...meta.outputs}; + + inputs = ClassPropertyMapping.merge(inputs, meta.inputs); + outputs = ClassPropertyMapping.merge(outputs, meta.outputs); for (const coercedInputField of meta.coercedInputFields) { coercedInputFields.add(coercedInputField); diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/property_mapping.ts b/packages/compiler-cli/src/ngtsc/metadata/src/property_mapping.ts new file mode 100644 index 0000000000000..6a78fe1e68dbb --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/metadata/src/property_mapping.ts @@ -0,0 +1,200 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {InputOutputPropertySet} from '@angular/compiler'; + +/** + * The name of a class property that backs an input or output declared by a directive or component. + * + * This type exists for documentation only. + */ +export type ClassPropertyName = string; + +/** + * The name by which an input or output of a directive or component is bound in an Angular template. + * + * This type exists for documentation only. + */ +export type BindingPropertyName = string; + +/** + * An input or output of a directive that has both a named JavaScript class property on a component + * or directive class, as well as an Angular template property name used for binding. + */ +export interface InputOrOutput { + /** + * The name of the JavaScript property on the component or directive instance for this input or + * output. + */ + readonly classPropertyName: ClassPropertyName; + + /** + * The property name used to bind this input or output in an Angular template. + */ + readonly bindingPropertyName: BindingPropertyName; +} + +/** + * A mapping of component property and template binding property names, for example containing the + * inputs of a particular directive or component. + * + * A single component property has exactly one input/output annotation (and therefore one binding + * property name) associated with it, but the same binding property name may be shared across many + * component property names. + * + * Allows bidirectional querying of the mapping - looking up all inputs/outputs with a given + * property name, or mapping from a specific class property to its binding property name. + */ +export class ClassPropertyMapping implements InputOutputPropertySet { + /** + * Mapping from class property names to the single `InputOrOutput` for that class property. + */ + private forwardMap: Map; + + /** + * Mapping from property names to one or more `InputOrOutput`s which share that name. + */ + private reverseMap: Map; + + private constructor(forwardMap: Map) { + this.forwardMap = forwardMap; + this.reverseMap = reverseMapFromForwardMap(forwardMap); + } + + /** + * Construct a `ClassPropertyMapping` with no entries. + */ + static empty(): ClassPropertyMapping { + return new ClassPropertyMapping(new Map()); + } + + /** + * Construct a `ClassPropertyMapping` from a primitive JS object which maps class property names + * to either binding property names or an array that contains both names, which is used in on-disk + * metadata formats (e.g. in .d.ts files). + */ + static fromMappedObject(obj: { + [classPropertyName: string]: BindingPropertyName|[ClassPropertyName, BindingPropertyName] + }): ClassPropertyMapping { + const forwardMap = new Map(); + + for (const classPropertyName of Object.keys(obj)) { + const value = obj[classPropertyName]; + const bindingPropertyName = Array.isArray(value) ? value[0] : value; + const inputOrOutput: InputOrOutput = {classPropertyName, bindingPropertyName}; + forwardMap.set(classPropertyName, inputOrOutput); + } + + return new ClassPropertyMapping(forwardMap); + } + + /** + * Merge two mappings into one, with class properties from `b` taking precedence over class + * properties from `a`. + */ + static merge(a: ClassPropertyMapping, b: ClassPropertyMapping): ClassPropertyMapping { + const forwardMap = new Map(a.forwardMap.entries()); + for (const [classPropertyName, inputOrOutput] of b.forwardMap) { + forwardMap.set(classPropertyName, inputOrOutput); + } + + return new ClassPropertyMapping(forwardMap); + } + + /** + * All class property names mapped in this mapping. + */ + get classPropertyNames(): ClassPropertyName[] { + return Array.from(this.forwardMap.keys()); + } + + /** + * All binding property names mapped in this mapping. + */ + get propertyNames(): BindingPropertyName[] { + return Array.from(this.reverseMap.keys()); + } + + /** + * Check whether a mapping for the given property name exists. + */ + hasBindingPropertyName(propertyName: BindingPropertyName): boolean { + return this.reverseMap.has(propertyName); + } + + /** + * Lookup all `InputOrOutput`s that use this `propertyName`. + */ + getByBindingPropertyName(propertyName: string): ReadonlyArray|null { + return this.reverseMap.has(propertyName) ? this.reverseMap.get(propertyName)! : null; + } + + /** + * Lookup the `InputOrOutput` associated with a `classPropertyName`. + */ + getByClassPropertyName(classPropertyName: string): InputOrOutput|null { + return this.forwardMap.has(classPropertyName) ? this.forwardMap.get(classPropertyName)! : null; + } + + /** + * Convert this mapping to a primitive JS object which maps each class property directly to the + * binding property name associated with it. + */ + toDirectMappedObject(): {[classPropertyName: string]: BindingPropertyName} { + const obj: {[classPropertyName: string]: BindingPropertyName} = {}; + for (const [classPropertyName, inputOrOutput] of this.forwardMap) { + obj[classPropertyName] = inputOrOutput.bindingPropertyName; + } + return obj; + } + + /** + * Convert this mapping to a primitive JS object which maps each class property either to itself + * (for cases where the binding property name is the same) or to an array which contains both + * names if they differ. + * + * This object format is used when mappings are serialized (for example into .d.ts files). + */ + toJointMappedObject(): + {[classPropertyName: string]: BindingPropertyName|[BindingPropertyName, ClassPropertyName]} { + const obj: { + [classPropertyName: string]: BindingPropertyName|[BindingPropertyName, ClassPropertyName] + } = {}; + for (const [classPropertyName, inputOrOutput] of this.forwardMap) { + if (inputOrOutput.bindingPropertyName as string === classPropertyName as string) { + obj[classPropertyName] = inputOrOutput.bindingPropertyName; + } else { + obj[classPropertyName] = [inputOrOutput.bindingPropertyName, classPropertyName]; + } + } + return obj; + } + + /** + * Implement the iterator protocol and return entry objects which contain the class and binding + * property names (and are useful for destructuring). + */ + * [Symbol.iterator](): IterableIterator<[ClassPropertyName, BindingPropertyName]> { + for (const [classPropertyName, inputOrOutput] of this.forwardMap.entries()) { + yield [classPropertyName, inputOrOutput.bindingPropertyName]; + } + } +} + +function reverseMapFromForwardMap(forwardMap: Map): + Map { + const reverseMap = new Map(); + for (const [_, inputOrOutput] of forwardMap) { + if (!reverseMap.has(inputOrOutput.bindingPropertyName)) { + reverseMap.set(inputOrOutput.bindingPropertyName, []); + } + + reverseMap.get(inputOrOutput.bindingPropertyName)!.push(inputOrOutput); + } + return reverseMap; +} diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts index 2f465dd977054..5d7f6c9841174 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts @@ -13,6 +13,7 @@ import {ClassDeclaration, ClassMember, ClassMemberKind, isNamedClassDeclaration, import {nodeDebugInfo} from '../../util/src/typescript'; import {DirectiveMeta, DirectiveTypeCheckMeta, MetadataReader, NgModuleMeta, PipeMeta, TemplateGuardMeta} from './api'; +import {ClassPropertyMapping, ClassPropertyName} from './property_mapping'; export function extractReferencesFromType( checker: ts.TypeChecker, def: ts.TypeNode, ngModuleImportedFrom: string|null, @@ -91,7 +92,7 @@ export function readStringArrayType(type: ts.TypeNode): string[] { * making this metadata invariant to changes of inherited classes. */ export function extractDirectiveTypeCheckMeta( - node: ClassDeclaration, inputs: {[fieldName: string]: string|[string, string]}, + node: ClassDeclaration, inputs: ClassPropertyMapping, reflector: ReflectionHost): DirectiveTypeCheckMeta { const members = reflector.getMembersOfClass(node); const staticMembers = members.filter(member => member.isStatic); @@ -102,23 +103,23 @@ export function extractDirectiveTypeCheckMeta( const coercedInputFields = new Set(staticMembers.map(extractCoercedInput) - .filter((inputName): inputName is string => inputName !== null)); + .filter((inputName): inputName is ClassPropertyName => inputName !== null)); - const restrictedInputFields = new Set(); - const stringLiteralInputFields = new Set(); - const undeclaredInputFields = new Set(); + const restrictedInputFields = new Set(); + const stringLiteralInputFields = new Set(); + const undeclaredInputFields = new Set(); - for (const fieldName of Object.keys(inputs)) { - const field = members.find(member => member.name === fieldName); + for (const classPropertyName of inputs.classPropertyNames) { + const field = members.find(member => member.name === classPropertyName); if (field === undefined || field.node === null) { - undeclaredInputFields.add(fieldName); + undeclaredInputFields.add(classPropertyName); continue; } if (isRestricted(field.node)) { - restrictedInputFields.add(fieldName); + restrictedInputFields.add(classPropertyName); } if (field.nameNode !== null && ts.isStringLiteral(field.nameNode)) { - stringLiteralInputFields.add(fieldName); + stringLiteralInputFields.add(classPropertyName); } } diff --git a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts index f8b12d5fc0f35..0216fd8864c80 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts +++ b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts @@ -9,7 +9,7 @@ import * as ts from 'typescript'; import {Reference, ReferenceEmitter} from '../../imports'; -import {CompoundMetadataRegistry, DirectiveMeta, LocalMetadataRegistry, MetadataRegistry, PipeMeta} from '../../metadata'; +import {ClassPropertyMapping, CompoundMetadataRegistry, DirectiveMeta, LocalMetadataRegistry, MetadataRegistry, PipeMeta} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; import {ScopeData} from '../src/api'; import {DtsModuleScopeResolver} from '../src/dependency'; @@ -236,8 +236,8 @@ function fakeDirective(ref: Reference): DirectiveMeta { name, selector: `[${ref.debugName}]`, isComponent: name.startsWith('Cmp'), - inputs: {}, - outputs: {}, + inputs: ClassPropertyMapping.fromMappedObject({}), + outputs: ClassPropertyMapping.fromMappedObject({}), exportAs: null, queries: [], hasNgTemplateContextGuard: false, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts index 658260fbf1c8b..594475009adfb 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {AbsoluteFsPath} from '../../file_system'; import {Reference} from '../../imports'; -import {DirectiveTypeCheckMeta} from '../../metadata'; +import {ClassPropertyMapping, DirectiveTypeCheckMeta} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; @@ -22,6 +22,8 @@ import {ClassDeclaration} from '../../reflection'; export interface TypeCheckableDirectiveMeta extends DirectiveMeta, DirectiveTypeCheckMeta { ref: Reference; queries: string[]; + inputs: ClassPropertyMapping; + outputs: ClassPropertyMapping; } export type TemplateId = string&{__brand: 'TemplateId'}; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index fc37c26b78f33..30c15189bc1ac 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -241,8 +241,8 @@ export class TypeCheckContextImpl implements TypeCheckContext { // it comes from a .d.ts file. .d.ts declarations don't have bodies. body: !dirNode.getSourceFile().isDeclarationFile, fields: { - inputs: Object.keys(dir.inputs), - outputs: Object.keys(dir.outputs), + inputs: dir.inputs.classPropertyNames, + outputs: dir.outputs.classPropertyNames, // TODO(alxhub): support queries queries: dir.queries, }, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index e0d8bb1905e77..bfa79ad8c41f7 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -79,8 +79,8 @@ export class Environment { fnName, body: true, fields: { - inputs: Object.keys(dir.inputs), - outputs: Object.keys(dir.outputs), + inputs: dir.inputs.classPropertyNames, + outputs: dir.outputs.classPropertyNames, // TODO: support queries queries: dir.queries, }, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index 5576556a71541..e295889c7cc0b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -10,6 +10,7 @@ import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiv import * as ts from 'typescript'; import {Reference} from '../../imports'; +import {ClassPropertyName} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; import {TemplateId, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata} from '../api'; @@ -431,7 +432,7 @@ class TcbDirectiveCtorOp extends TcbOp { } // Add unset directive inputs for each of the remaining unset fields. - for (const fieldName of Object.keys(this.dir.inputs)) { + for (const [fieldName] of this.dir.inputs) { if (!genericInputs.has(fieldName)) { genericInputs.set(fieldName, {type: 'unset', field: fieldName}); } @@ -743,22 +744,14 @@ class TcbDirectiveOutputsOp extends TcbOp { execute(): null { let dirId: ts.Expression|null = null; - - - // `dir.outputs` is an object map of field names on the directive class to event names. - // This is backwards from what's needed to match event handlers - a map of event names to field - // names is desired. Invert `dir.outputs` into `fieldByEventName` to create this map. - const fieldByEventName = new Map(); const outputs = this.dir.outputs; - for (const key of Object.keys(outputs)) { - fieldByEventName.set(outputs[key], key); - } for (const output of this.node.outputs) { - if (output.type !== ParsedEventType.Regular || !fieldByEventName.has(output.name)) { + if (output.type !== ParsedEventType.Regular || !outputs.hasBindingPropertyName(output.name)) { continue; } - const field = fieldByEventName.get(output.name)!; + // TODO(alxhub): consider supporting multiple fields with the same property name for outputs. + const field = outputs.getByBindingPropertyName(output.name)![0].classPropertyName; if (this.tcb.env.config.checkTypeOfOutputEvents) { // For strict checking of directive events, generate a call to the `subscribe` method @@ -1225,9 +1218,8 @@ class Scope { if (node instanceof TmplAstElement) { // Go through the directives and remove any inputs that it claims from `elementInputs`. for (const dir of directives) { - for (const fieldName of Object.keys(dir.inputs)) { - const value = dir.inputs[fieldName]; - claimedInputs.add(Array.isArray(value) ? value[0] : value); + for (const propertyName of dir.inputs.propertyNames) { + claimedInputs.add(propertyName); } } @@ -1264,8 +1256,8 @@ class Scope { if (node instanceof TmplAstElement) { // Go through the directives and register any outputs that it claims in `claimedOutputs`. for (const dir of directives) { - for (const outputField of Object.keys(dir.outputs)) { - claimedOutputs.add(dir.outputs[outputField]); + for (const outputProperty of dir.outputs.propertyNames) { + claimedOutputs.add(outputProperty); } } @@ -1276,7 +1268,7 @@ class Scope { interface TcbBoundInput { attribute: TmplAstBoundAttribute|TmplAstTextAttribute; - fieldNames: string[]; + fieldNames: ClassPropertyName[]; } /** @@ -1537,7 +1529,6 @@ function getBoundInputs( tcb: Context): TcbBoundInput[] { const boundInputs: TcbBoundInput[] = []; - const propertyToFieldNames = invertInputs(directive.inputs); const processAttribute = (attr: TmplAstBoundAttribute|TmplAstTextAttribute) => { // Skip non-property bindings. if (attr instanceof TmplAstBoundAttribute && attr.type !== BindingType.Property) { @@ -1550,10 +1541,11 @@ function getBoundInputs( } // Skip the attribute if the directive does not have an input for it. - if (!propertyToFieldNames.has(attr.name)) { + const inputs = directive.inputs.getByBindingPropertyName(attr.name); + if (inputs === null) { return; } - const fieldNames = propertyToFieldNames.get(attr.name)!; + const fieldNames = inputs.map(input => input.classPropertyName); boundInputs.push({attribute: attr, fieldNames}); }; @@ -1580,26 +1572,6 @@ function translateInput( } } -/** - * Inverts the input-mapping from field-to-property name into property-to-field name, to be able - * to match a property in a template with the corresponding field on a directive. - */ -function invertInputs(inputs: {[fieldName: string]: string|[string, string]}): - Map { - const propertyToFieldNames = new Map(); - for (const fieldName of Object.keys(inputs)) { - const propertyNames = inputs[fieldName]; - const propertyName = Array.isArray(propertyNames) ? propertyNames[0] : propertyNames; - - if (propertyToFieldNames.has(propertyName)) { - propertyToFieldNames.get(propertyName)!.push(fieldName); - } else { - propertyToFieldNames.set(propertyName, [fieldName]); - } - } - return propertyToFieldNames; -} - /** * An input binding that corresponds with a field of a directive. */ diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel index 6e8e8b2f145d9..523f14ae0daff 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel @@ -16,6 +16,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/file_system/testing", "//packages/compiler-cli/src/ngtsc/imports", "//packages/compiler-cli/src/ngtsc/incremental", + "//packages/compiler-cli/src/ngtsc/metadata", "//packages/compiler-cli/src/ngtsc/reflection", "//packages/compiler-cli/src/ngtsc/shims", "//packages/compiler-cli/src/ngtsc/testing", diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index 3c8a5c96b8c26..248f5ea352b85 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -13,6 +13,7 @@ import {absoluteFrom, AbsoluteFsPath, getSourceFileOrError, LogicalFileSystem} f import {TestFile} from '../../file_system/testing'; import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; import {NOOP_INCREMENTAL_BUILD} from '../../incremental'; +import {ClassPropertyMapping} from '../../metadata'; import {ClassDeclaration, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection'; import {makeProgram} from '../../testing'; import {getRootDirs} from '../../util/src/typescript'; @@ -180,8 +181,9 @@ export type TestDirective = Partial>>&{ + 'undeclaredInputFields'|'inputs'|'outputs'>>>&{ selector: string, name: string, file?: AbsoluteFsPath, type: 'directive', + inputs?: {[fieldName: string]: string}, outputs?: {[fieldName: string]: string}, coercedInputFields?: string[], restrictedInputFields?: string[], stringLiteralInputFields?: string[], undeclaredInputFields?: string[], isGeneric?: boolean; }; @@ -418,7 +420,7 @@ function prepareDeclarations( ref: new Reference(resolveDeclaration(decl)), exportAs: decl.exportAs || null, hasNgTemplateContextGuard: decl.hasNgTemplateContextGuard || false, - inputs: decl.inputs || {}, + inputs: ClassPropertyMapping.fromMappedObject(decl.inputs || {}), isComponent: decl.isComponent || false, ngTemplateGuards: decl.ngTemplateGuards || [], coercedInputFields: new Set(decl.coercedInputFields || []), @@ -426,7 +428,7 @@ function prepareDeclarations( stringLiteralInputFields: new Set(decl.stringLiteralInputFields || []), undeclaredInputFields: new Set(decl.undeclaredInputFields || []), isGeneric: decl.isGeneric ?? false, - outputs: decl.outputs || {}, + outputs: ClassPropertyMapping.fromMappedObject(decl.outputs || {}), queries: decl.queries || [], }; matcher.addSelectables(selector, meta); diff --git a/packages/compiler/src/render3/view/t2_api.ts b/packages/compiler/src/render3/view/t2_api.ts index e327c55b7951b..35513968804e0 100644 --- a/packages/compiler/src/render3/view/t2_api.ts +++ b/packages/compiler/src/render3/view/t2_api.ts @@ -26,6 +26,16 @@ export interface Target { template?: Node[]; } +/** + * A data structure which can indicate whether a given property name is present or not. + * + * This is used to represent the set of inputs or outputs present on a directive, and allows the + * binder to query for the presence of a mapping for property names. + */ +export interface InputOutputPropertySet { + hasBindingPropertyName(propertyName: string): boolean; +} + /** * Metadata regarding a directive that's needed to match it against template elements. This is * provided by a consumer of the t2 APIs. @@ -46,14 +56,14 @@ export interface DirectiveMeta { * * Goes from property names to field names. */ - inputs: {[property: string]: string|[string, string]}; + inputs: InputOutputPropertySet; /** * Set of outputs which this directive claims. * * Goes from property names to field names. */ - outputs: {[property: string]: string}; + outputs: InputOutputPropertySet; /** * Name under which the directive is exported, if any (exportAs in Angular). diff --git a/packages/compiler/src/render3/view/t2_binder.ts b/packages/compiler/src/render3/view/t2_binder.ts index 35230722950d0..53b363afcc172 100644 --- a/packages/compiler/src/render3/view/t2_binder.ts +++ b/packages/compiler/src/render3/view/t2_binder.ts @@ -278,7 +278,7 @@ class DirectiveBinder implements Visitor { type BoundNode = BoundAttribute|BoundEvent|TextAttribute; const setAttributeBinding = (attribute: BoundNode, ioType: keyof Pick) => { - const dir = directives.find(dir => dir[ioType].hasOwnProperty(attribute.name)); + const dir = directives.find(dir => dir[ioType].hasBindingPropertyName(attribute.name)); const binding = dir !== undefined ? dir : node; this.bindings.set(attribute, binding); }; diff --git a/packages/compiler/test/render3/view/binding_spec.ts b/packages/compiler/test/render3/view/binding_spec.ts index 02e9a7ee9544e..a1c51254544d2 100644 --- a/packages/compiler/test/render3/view/binding_spec.ts +++ b/packages/compiler/test/render3/view/binding_spec.ts @@ -8,41 +8,56 @@ import * as e from '../../../src/expression_parser/ast'; import * as a from '../../../src/render3/r3_ast'; -import {DirectiveMeta} from '../../../src/render3/view/t2_api'; +import {DirectiveMeta, InputOutputPropertySet} from '../../../src/render3/view/t2_api'; import {R3TargetBinder} from '../../../src/render3/view/t2_binder'; import {parseTemplate} from '../../../src/render3/view/template'; import {CssSelector, SelectorMatcher} from '../../../src/selector'; import {findExpression} from './util'; +/** + * A `InputOutputPropertySet` which only uses an identity mapping for fields and properties. + */ +class IdentityInputMapping implements InputOutputPropertySet { + private names: Set; + + constructor(names: string[]) { + this.names = new Set(names); + } + + hasBindingPropertyName(propertyName: string): boolean { + return this.names.has(propertyName); + } +} + function makeSelectorMatcher(): SelectorMatcher { const matcher = new SelectorMatcher(); matcher.addSelectables(CssSelector.parse('[ngFor][ngForOf]'), { name: 'NgFor', exportAs: null, - inputs: {'ngForOf': 'ngForOf'}, - outputs: {}, + inputs: new IdentityInputMapping(['ngForOf']), + outputs: new IdentityInputMapping([]), isComponent: false, }); matcher.addSelectables(CssSelector.parse('[dir]'), { name: 'Dir', exportAs: null, - inputs: {}, - outputs: {}, + inputs: new IdentityInputMapping([]), + outputs: new IdentityInputMapping([]), isComponent: false, }); matcher.addSelectables(CssSelector.parse('[hasOutput]'), { name: 'HasOutput', exportAs: null, - inputs: {}, - outputs: {'outputBinding': 'outputBinding'}, + inputs: new IdentityInputMapping([]), + outputs: new IdentityInputMapping(['outputBinding']), isComponent: false, }); matcher.addSelectables(CssSelector.parse('[hasInput]'), { name: 'HasInput', exportAs: null, - inputs: {'inputBinding': 'inputBinding'}, - outputs: {}, + inputs: new IdentityInputMapping(['inputBinding']), + outputs: new IdentityInputMapping([]), isComponent: false, }); return matcher; @@ -85,8 +100,8 @@ describe('t2 binding', () => { matcher.addSelectables(CssSelector.parse('text[dir]'), { name: 'Dir', exportAs: null, - inputs: {}, - outputs: {}, + inputs: new IdentityInputMapping([]), + outputs: new IdentityInputMapping([]), isComponent: false, }); const binder = new R3TargetBinder(matcher);