From 64516da6b0845b2a8b170b364e10cde65f9423af Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Mon, 23 Jul 2018 17:01:22 -0700 Subject: [PATCH] feat(ivy): support inheriting input/output from bare base class (#25094) PR Close #25094 --- packages/core/src/metadata/directives.ts | 48 +++++- packages/core/src/render3/definition.ts | 80 +++++++++- .../features/inherit_definition_feature.ts | 38 +++-- packages/core/src/render3/index.ts | 3 +- .../core/src/render3/interfaces/definition.ts | 51 +++--- packages/core/src/util/decorators.ts | 32 ++-- packages/core/src/util/property.ts | 14 ++ .../Inherit_definition_feature_spec.ts | 145 +++++++++++++++++- packages/core/test/render3/ivy/jit_spec.ts | 29 +++- 9 files changed, 378 insertions(+), 62 deletions(-) diff --git a/packages/core/src/metadata/directives.ts b/packages/core/src/metadata/directives.ts index 7a0bcbe0d8..1a57f51d8e 100644 --- a/packages/core/src/metadata/directives.ts +++ b/packages/core/src/metadata/directives.ts @@ -11,6 +11,7 @@ import {Provider} from '../di'; import {R3_COMPILE_COMPONENT, R3_COMPILE_DIRECTIVE, R3_COMPILE_PIPE} from '../ivy_switch'; import {Type} from '../type'; import {TypeDecorator, makeDecorator, makePropDecorator} from '../util/decorators'; +import {fillProperties} from '../util/property'; import {ViewEncapsulation} from './view'; @@ -734,7 +735,7 @@ export interface Input { * selector: 'bank-account', * template: ` * Bank Name: {{bankName}} - * Account Id: {{id}} + * Account Id: {{id}} * ` * }) * class BankAccount { @@ -761,12 +762,47 @@ export interface Input { bindingPropertyName?: string; } +const initializeBaseDef = (target: any): void => { + const constructor = target.constructor; + const inheritedBaseDef = constructor.ngBaseDef; + + const baseDef = constructor.ngBaseDef = { + inputs: {}, + outputs: {}, + declaredInputs: {}, + }; + + if (inheritedBaseDef) { + fillProperties(baseDef.inputs, inheritedBaseDef.inputs); + fillProperties(baseDef.outputs, inheritedBaseDef.outputs); + fillProperties(baseDef.declaredInputs, inheritedBaseDef.declaredInputs); + } +}; + +/** + * Does the work of creating the `ngBaseDef` property for the @Input and @Output decorators. + * @param key "inputs" or "outputs" + */ +const updateBaseDefFromIOProp = (getProp: (baseDef: {inputs?: any, outputs?: any}) => any) => + (target: any, name: string, ...args: any[]) => { + const constructor = target.constructor; + + if (!constructor.hasOwnProperty('ngBaseDef')) { + initializeBaseDef(target); + } + + const baseDef = constructor.ngBaseDef; + const defProp = getProp(baseDef); + defProp[name] = args[0]; + }; + /** * * @Annotation */ -export const Input: InputDecorator = - makePropDecorator('Input', (bindingPropertyName?: string) => ({bindingPropertyName})); +export const Input: InputDecorator = makePropDecorator( + 'Input', (bindingPropertyName?: string) => ({bindingPropertyName}), undefined, + updateBaseDefFromIOProp(baseDef => baseDef.inputs || {})); /** * Type of the Output decorator / constructor function. @@ -800,8 +836,10 @@ export interface Output { bindingPropertyName?: string; } * * @Annotation */ -export const Output: OutputDecorator = - makePropDecorator('Output', (bindingPropertyName?: string) => ({bindingPropertyName})); +export const Output: OutputDecorator = makePropDecorator( + 'Output', (bindingPropertyName?: string) => ({bindingPropertyName}), undefined, + updateBaseDefFromIOProp(baseDef => baseDef.outputs || {})); + /** diff --git a/packages/core/src/render3/definition.ts b/packages/core/src/render3/definition.ts index 0de1e46439..392052d52d 100644 --- a/packages/core/src/render3/definition.ts +++ b/packages/core/src/render3/definition.ts @@ -16,7 +16,7 @@ import {Type} from '../type'; import {resolveRendererType2} from '../view/util'; import {diPublic} from './di'; -import {ComponentDefFeature, ComponentDefInternal, ComponentQuery, ComponentTemplate, ComponentType, DirectiveDefFeature, DirectiveDefInternal, DirectiveDefListOrFactory, DirectiveType, DirectiveTypesOrFactory, PipeDefInternal, PipeType, PipeTypesOrFactory} from './interfaces/definition'; +import {BaseDef, ComponentDefFeature, ComponentDefInternal, ComponentQuery, ComponentTemplate, ComponentType, DirectiveDefFeature, DirectiveDefInternal, DirectiveDefListOrFactory, DirectiveType, DirectiveTypesOrFactory, PipeDefInternal, PipeType, PipeTypesOrFactory} from './interfaces/definition'; import {CssSelectorList, SelectorFlags} from './interfaces/projection'; @@ -353,6 +353,84 @@ function invertObject(obj: any, secondary?: any): any { return newLookup; } +/** + * Create a base definition + * + * # Example + * ``` + * class ShouldBeInherited { + * static ngBaseDef = defineBase({ + * ... + * }) + * } + * @param baseDefinition The base definition parameters + */ +export function defineBase(baseDefinition: { + /** + * A map of input names. + * + * The format is in: `{[actualPropertyName: string]:(string|[string, string])}`. + * + * Given: + * ``` + * class MyComponent { + * @Input() + * publicInput1: string; + * + * @Input('publicInput2') + * declaredInput2: string; + * } + * ``` + * + * is described as: + * ``` + * { + * publicInput1: 'publicInput1', + * declaredInput2: ['declaredInput2', 'publicInput2'], + * } + * ``` + * + * Which the minifier may translate to: + * ``` + * { + * minifiedPublicInput1: 'publicInput1', + * minifiedDeclaredInput2: [ 'declaredInput2', 'publicInput2'], + * } + * ``` + * + * This allows the render to re-construct the minified, public, and declared names + * of properties. + * + * NOTE: + * - Because declared and public name are usually same we only generate the array + * `['declared', 'public']` format when they differ. + * - The reason why this API and `outputs` API is not the same is that `NgOnChanges` has + * inconsistent behavior in that it uses declared names rather than minified or public. For + * this reason `NgOnChanges` will be deprecated and removed in future version and this + * API will be simplified to be consistent with `outputs`. + */ + inputs?: {[P in keyof T]?: string | [string, string]}; + + /** + * A map of output names. + * + * The format is in: `{[actualPropertyName: string]:string}`. + * + * Which the minifier may translate to: `{[minifiedPropertyName: string]:string}`. + * + * This allows the render to re-construct the minified and non-minified names + * of properties. + */ + outputs?: {[P in keyof T]?: string}; +}): BaseDef { + const declaredInputs: {[P in keyof T]: P} = {} as any; + return { + inputs: invertObject(baseDefinition.inputs, declaredInputs), + declaredInputs: declaredInputs, + outputs: invertObject(baseDefinition.outputs), + }; +} + /** * Create a directive definition object. * diff --git a/packages/core/src/render3/features/inherit_definition_feature.ts b/packages/core/src/render3/features/inherit_definition_feature.ts index 8d4588459a..72bcec9784 100644 --- a/packages/core/src/render3/features/inherit_definition_feature.ts +++ b/packages/core/src/render3/features/inherit_definition_feature.ts @@ -7,22 +7,10 @@ */ import {Type} from '../../type'; -import {ComponentDefInternal, ComponentType, DirectiveDefFeature, DirectiveDefInternal} from '../interfaces/definition'; +import {fillProperties} from '../../util/property'; +import {ComponentDefInternal, DirectiveDefFeature, DirectiveDefInternal} from '../interfaces/definition'; -/** - * Sets properties on a target object from a source object, but only if - * the property doesn't already exist on the target object. - * @param target The target to set properties on - * @param source The source of the property keys and values to set - */ -function fillProperties(target: {[key: string]: string}, source: {[key: string]: string}) { - for (const key in source) { - if (source.hasOwnProperty(key) && !target.hasOwnProperty(key)) { - target[key] = source[key]; - } - } -} /** * Determines if a definition is a {@link ComponentDefInternal} or a {@link DirectiveDefInternal} * @param definition The definition to examine @@ -45,9 +33,9 @@ function getSuperType(type: Type): Type& export function InheritDefinitionFeature( definition: DirectiveDefInternal| ComponentDefInternal): void { let superType = getSuperType(definition.type); - let superDef: DirectiveDefInternal|ComponentDefInternal|undefined = undefined; - while (superType && !superDef) { + while (superType) { + let superDef: DirectiveDefInternal|ComponentDefInternal|undefined = undefined; if (isComponentDef(definition)) { superDef = superType.ngComponentDef || superType.ngDirectiveDef; } else { @@ -57,12 +45,15 @@ export function InheritDefinitionFeature( superDef = superType.ngDirectiveDef; } - if (superDef) { + const baseDef = (superType as any).ngBaseDef; + if (baseDef) { // Merge inputs and outputs - fillProperties(definition.inputs, superDef.inputs); - fillProperties(definition.declaredInputs, superDef.declaredInputs); - fillProperties(definition.outputs, superDef.outputs); + fillProperties(definition.inputs, baseDef.inputs); + fillProperties(definition.declaredInputs, baseDef.declaredInputs); + fillProperties(definition.outputs, baseDef.outputs); + } + if (superDef) { // Merge hostBindings const prevHostBindings = definition.hostBindings; const superHostBindings = superDef.hostBindings; @@ -77,6 +68,11 @@ export function InheritDefinitionFeature( } } + // Merge inputs and outputs + fillProperties(definition.inputs, superDef.inputs); + fillProperties(definition.declaredInputs, superDef.declaredInputs); + fillProperties(definition.outputs, superDef.outputs); + // Inherit hooks // Assume super class inheritance feature has already run. definition.afterContentChecked = @@ -97,6 +93,8 @@ export function InheritDefinitionFeature( } } } + + break; } else { // Even if we don't have a definition, check the type for the hooks and use those if need be const superPrototype = superType.prototype; diff --git a/packages/core/src/render3/index.ts b/packages/core/src/render3/index.ts index c9da55ea79..da441134c4 100644 --- a/packages/core/src/render3/index.ts +++ b/packages/core/src/render3/index.ts @@ -7,7 +7,7 @@ */ import {LifecycleHooksFeature, getHostElement, getRenderedText, renderComponent, whenRendered} from './component'; -import {defineComponent, defineDirective, defineNgModule, definePipe} from './definition'; +import {defineBase, defineComponent, defineDirective, defineNgModule, definePipe} from './definition'; import {InheritDefinitionFeature} from './features/inherit_definition_feature'; import {NgOnChangesFeature} from './features/ng_onchanges_feature'; import {PublicFeature} from './features/public_feature'; @@ -164,6 +164,7 @@ export { defineComponent, defineDirective, defineNgModule, + defineBase, definePipe, getHostElement, getRenderedText, diff --git a/packages/core/src/render3/interfaces/definition.ts b/packages/core/src/render3/interfaces/definition.ts index 7b321c38b5..3368172111 100644 --- a/packages/core/src/render3/interfaces/definition.ts +++ b/packages/core/src/render3/interfaces/definition.ts @@ -67,29 +67,15 @@ export interface PipeType extends Type { ngPipeDef: never; } export type DirectiveDefInternal = DirectiveDef; /** - * Runtime link information for Directives. + * Runtime information for classes that are inherited by components or directives + * that aren't defined as components or directives. * - * This is internal data structure used by the render to link - * directives into templates. + * This is an internal data structure used by the render to determine what inputs + * and outputs should be inherited. * - * NOTE: Always use `defineDirective` function to create this object, - * never create the object directly since the shape of this object - * can change between versions. - * - * @param Selector type metadata specifying the selector of the directive or component - * - * See: {@link defineDirective} + * See: {@link defineBase} */ -export interface DirectiveDef { - /** Token representing the directive. Used by DI. */ - type: Type; - - /** Function that makes a directive public to the DI system. */ - diPublic: ((def: DirectiveDef) => void)|null; - - /** The selectors that will be used to match nodes to this directive. */ - selectors: CssSelectorList; - +export interface BaseDef { /** * A dictionary mapping the inputs' minified property names to their public API names, which * are their aliases if any, or their original unminified property names @@ -109,6 +95,31 @@ export interface DirectiveDef { * (as in `@Output('alias') propertyName: any;`). */ readonly outputs: {[P in keyof T]: P}; +} + +/** + * Runtime link information for Directives. + * + * This is internal data structure used by the render to link + * directives into templates. + * + * NOTE: Always use `defineDirective` function to create this object, + * never create the object directly since the shape of this object + * can change between versions. + * + * @param Selector type metadata specifying the selector of the directive or component + * + * See: {@link defineDirective} + */ +export interface DirectiveDef extends BaseDef { + /** Token representing the directive. Used by DI. */ + type: Type; + + /** Function that makes a directive public to the DI system. */ + diPublic: ((def: DirectiveDef) => void)|null; + + /** The selectors that will be used to match nodes to this directive. */ + selectors: CssSelectorList; /** * Name under which the directive is exported (for use with local references in template) diff --git a/packages/core/src/util/decorators.ts b/packages/core/src/util/decorators.ts index afc39b620a..da5fe6ae02 100644 --- a/packages/core/src/util/decorators.ts +++ b/packages/core/src/util/decorators.ts @@ -41,31 +41,34 @@ export const PROP_METADATA = '__prop__metadata__'; /** * @suppress {globalThis} */ -export function makeDecorator( +export function makeDecorator( name: string, props?: (...args: any[]) => any, parentClass?: any, - chainFn?: (fn: Function) => void, typeFn?: (type: Type, ...args: any[]) => void): + additionalProcessing?: (type: Type) => void, + typeFn?: (type: Type, ...args: any[]) => void): {new (...args: any[]): any; (...args: any[]): any; (...args: any[]): (cls: any) => any;} { const metaCtor = makeMetadataCtor(props); - function DecoratorFactory(...args: any[]): (cls: any) => any { + function DecoratorFactory(...args: any[]): (cls: Type) => any { if (this instanceof DecoratorFactory) { metaCtor.call(this, ...args); return this; } - const annotationInstance = new (DecoratorFactory)(...args); - const TypeDecorator: TypeDecorator = function TypeDecorator(cls: Type) { - typeFn && typeFn(cls, ...args); + const annotationInstance = new (DecoratorFactory as any)(...args); + return function TypeDecorator(cls: Type) { + if (typeFn) typeFn(cls, ...args); // Use of Object.defineProperty is important since it creates non-enumerable property which // prevents the property is copied during subclassing. const annotations = cls.hasOwnProperty(ANNOTATIONS) ? (cls as any)[ANNOTATIONS] : Object.defineProperty(cls, ANNOTATIONS, {value: []})[ANNOTATIONS]; annotations.push(annotationInstance); + + + if (additionalProcessing) additionalProcessing(cls); + return cls; }; - if (chainFn) chainFn(TypeDecorator); - return TypeDecorator; } if (parentClass) { @@ -73,7 +76,7 @@ export function makeDecorator( } DecoratorFactory.prototype.ngMetadataName = name; - (DecoratorFactory).annotationCls = DecoratorFactory; + (DecoratorFactory as any).annotationCls = DecoratorFactory; return DecoratorFactory as any; } @@ -127,7 +130,8 @@ export function makeParamDecorator( } export function makePropDecorator( - name: string, props?: (...args: any[]) => any, parentClass?: any): any { + name: string, props?: (...args: any[]) => any, parentClass?: any, + additionalProcessing?: (target: any, name: string, ...args: any[]) => void): any { const metaCtor = makeMetadataCtor(props); function PropDecoratorFactory(...args: any[]): any { @@ -138,7 +142,7 @@ export function makePropDecorator( const decoratorInstance = new (PropDecoratorFactory)(...args); - return function PropDecorator(target: any, name: string) { + function PropDecorator(target: any, name: string) { const constructor = target.constructor; // Use of Object.defineProperty is important since it creates non-enumerable property which // prevents the property is copied during subclassing. @@ -147,7 +151,11 @@ export function makePropDecorator( Object.defineProperty(constructor, PROP_METADATA, {value: {}})[PROP_METADATA]; meta[name] = meta.hasOwnProperty(name) && meta[name] || []; meta[name].unshift(decoratorInstance); - }; + + if (additionalProcessing) additionalProcessing(target, name, ...args); + } + + return PropDecorator; } if (parentClass) { diff --git a/packages/core/src/util/property.ts b/packages/core/src/util/property.ts index b473aeb2ca..da02f03f58 100644 --- a/packages/core/src/util/property.ts +++ b/packages/core/src/util/property.ts @@ -14,3 +14,17 @@ export function getClosureSafeProperty(objWithPropertyToExtract: T, target: a } throw Error('Could not find renamed property on target object.'); } + +/** + * Sets properties on a target object from a source object, but only if + * the property doesn't already exist on the target object. + * @param target The target to set properties on + * @param source The source of the property keys and values to set + */ +export function fillProperties(target: {[key: string]: string}, source: {[key: string]: string}) { + for (const key in source) { + if (source.hasOwnProperty(key) && !target.hasOwnProperty(key)) { + target[key] = source[key]; + } + } +} diff --git a/packages/core/test/render3/Inherit_definition_feature_spec.ts b/packages/core/test/render3/Inherit_definition_feature_spec.ts index f4babe51b4..a39437b603 100644 --- a/packages/core/test/render3/Inherit_definition_feature_spec.ts +++ b/packages/core/test/render3/Inherit_definition_feature_spec.ts @@ -6,9 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ -import {DoCheck, EventEmitter, Input, OnChanges, Output, SimpleChange, SimpleChanges} from '../../src/core'; +import {EventEmitter, Output} from '../../src/core'; import {InheritDefinitionFeature} from '../../src/render3/features/inherit_definition_feature'; -import {DirectiveDefInternal, NgOnChangesFeature, defineComponent, defineDirective} from '../../src/render3/index'; +import {DirectiveDefInternal, defineBase, defineComponent, defineDirective} from '../../src/render3/index'; describe('InheritDefinitionFeature', () => { it('should inherit lifecycle hooks', () => { @@ -131,6 +131,147 @@ describe('InheritDefinitionFeature', () => { }); }); + it('should inherit inputs from ngBaseDefs along the way', () => { + + class Class5 { + input5 = 'data, so data'; + + static ngBaseDef = defineBase({ + inputs: { + input5: 'input5', + }, + }); + } + + // tslint:disable-next-line:class-as-namespace + class Class4 extends Class5 { + input4 = 'hehe'; + + static ngDirectiveDef = defineDirective({ + inputs: { + input4: 'input4', + }, + type: Class4, + selectors: [['', 'superDir', '']], + factory: () => new Class4(), + features: [InheritDefinitionFeature], + }); + } + + class Class3 extends Class4 {} + + class Class2 extends Class3 { + input3 = 'wee'; + + static ngBaseDef = defineBase({ + inputs: { + input3: ['alias3', 'input3'], + } + }) as any; + } + + // tslint:disable-next-line:class-as-namespace + class Class1 extends Class2 { + input1 = 'test'; + input2 = 'whatever'; + + static ngDirectiveDef = defineDirective({ + type: Class1, + inputs: { + input1: 'input1', + input2: 'input2', + }, + selectors: [['', 'subDir', '']], + factory: () => new Class1(), + features: [InheritDefinitionFeature], + }); + } + + const subDef = Class1.ngDirectiveDef as DirectiveDefInternal; + + expect(subDef.inputs).toEqual({ + input1: 'input1', + input2: 'input2', + alias3: 'input3', + input4: 'input4', + input5: 'input5', + }); + expect(subDef.declaredInputs).toEqual({ + input1: 'input1', + input2: 'input2', + input3: 'input3', + input4: 'input4', + input5: 'input5', + }); + }); + + it('should inherit outputs from ngBaseDefs along the way', () => { + + class Class5 { + output5 = 'data, so data'; + + static ngBaseDef = defineBase({ + outputs: { + output5: 'alias5', + }, + }); + } + + // tslint:disable-next-line:class-as-namespace + class Class4 extends Class5 { + output4 = 'hehe'; + + static ngDirectiveDef = defineDirective({ + outputs: { + output4: 'alias4', + }, + type: Class4, + selectors: [['', 'superDir', '']], + factory: () => new Class4(), + features: [InheritDefinitionFeature], + }); + } + + class Class3 extends Class4 {} + + class Class2 extends Class3 { + output3 = 'wee'; + + static ngBaseDef = defineBase({ + outputs: { + output3: 'alias3', + } + }) as any; + } + + // tslint:disable-next-line:class-as-namespace + class Class1 extends Class2 { + output1 = 'test'; + output2 = 'whatever'; + + static ngDirectiveDef = defineDirective({ + type: Class1, + outputs: { + output1: 'alias1', + output2: 'alias2', + }, + selectors: [['', 'subDir', '']], + factory: () => new Class1(), + features: [InheritDefinitionFeature], + }); + } + + const subDef = Class1.ngDirectiveDef as DirectiveDefInternal; + + expect(subDef.outputs).toEqual({ + alias1: 'output1', + alias2: 'output2', + alias3: 'output3', + alias4: 'output4', + alias5: 'output5', + }); + }); + it('should compose hostBindings', () => { const log: Array<[string, number, number]> = []; diff --git a/packages/core/test/render3/ivy/jit_spec.ts b/packages/core/test/render3/ivy/jit_spec.ts index 3e16eea884..3029b2dda1 100644 --- a/packages/core/test/render3/ivy/jit_spec.ts +++ b/packages/core/test/render3/ivy/jit_spec.ts @@ -12,10 +12,11 @@ import {InjectorDef, defineInjectable} from '@angular/core/src/di/defs'; import {Injectable} from '@angular/core/src/di/injectable'; import {inject, setCurrentInjector} from '@angular/core/src/di/injector'; import {ivyEnabled} from '@angular/core/src/ivy_switch'; -import {Component, HostBinding, HostListener, Pipe} from '@angular/core/src/metadata/directives'; +import {Component, HostBinding, HostListener, Input, Output, Pipe} from '@angular/core/src/metadata/directives'; import {NgModule, NgModuleDefInternal} from '@angular/core/src/metadata/ng_module'; import {ComponentDefInternal, PipeDefInternal} from '@angular/core/src/render3/interfaces/definition'; + ivyEnabled && describe('render3 jit', () => { let injector: any; beforeAll(() => { injector = setCurrentInjector(null); }); @@ -233,6 +234,32 @@ ivyEnabled && describe('render3 jit', () => { const pipeDef = (P as any).ngPipeDef as PipeDefInternal

; expect(pipeDef.pure).toBe(true, 'pipe should be pure'); }); + + it('should add ngBaseDef to types with @Input properties', () => { + class C { + @Input('alias1') + prop1 = 'test'; + + @Input('alias2') + prop2 = 'test'; + } + + expect((C as any).ngBaseDef).toBeDefined(); + expect((C as any).ngBaseDef.inputs).toEqual({prop1: 'alias1', prop2: 'alias2'}); + }); + + it('should add ngBaseDef to types with @Output properties', () => { + class C { + @Output('alias1') + prop1 = 'test'; + + @Output('alias2') + prop2 = 'test'; + } + + expect((C as any).ngBaseDef).toBeDefined(); + expect((C as any).ngBaseDef.outputs).toEqual({prop1: 'alias1', prop2: 'alias2'}); + }); }); it('ensure at least one spec exists', () => {});