Fw 956 refactor on changes

packages/compiler-cli/src/ngtsc/annotations/src/directive.ts

@@ -174,6 +174,11 @@ export function extractDirectiveMetadata(
   const providers: Expression|null =
       directive.has('providers') ? new WrappedNodeExpr(directive.get('providers') !) : null;
 
+  // Determine if `ngOnChanges` is a lifecycle hook defined on the component.
+  const usesOnChanges = members.some(
+      member => !member.isStatic && member.kind === ClassMemberKind.Method &&
+          member.name === 'ngOnChanges');
+
   // Parse exportAs.
   let exportAs: string[]|null = null;
   if (directive.has('exportAs')) {
@@ -192,6 +197,9 @@ export function extractDirectiveMetadata(
   const metadata: R3DirectiveMetadata = {
     name: clazz.name !.text,
     deps: getConstructorDependencies(clazz, reflector, isCore), host,
+    lifecycle: {
+        usesOnChanges,
+    },
     inputs: {...inputsFromMeta, ...inputsFromFields},
     outputs: {...outputsFromMeta, ...outputsFromFields}, queries, selector,
     type: new WrappedNodeExpr(clazz.name !),

packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts

@@ -2117,6 +2117,7 @@ describe('compiler compliance', () => {
             selectors: [["lifecycle-comp"]],
             factory: function LifecycleComp_Factory(t) { return new (t || LifecycleComp)(); },
             inputs: {nameMin: ["name", "nameMin"]},
+            features: [$r3$.ɵNgOnChangesFeature()],
             consts: 0,
             vars: 0,
             template:  function LifecycleComp_Template(rf, ctx) {},
@@ -2241,6 +2242,7 @@ describe('compiler compliance', () => {
                 factory: function ForOfDirective_Factory(t) {
                   return new (t || ForOfDirective)($r3$.ɵdirectiveInject(ViewContainerRef), $r3$.ɵdirectiveInject(TemplateRef));
                 },
+                features: [$r3$.ɵNgOnChangesFeature()],
                 inputs: {forOf: "forOf"}
               });
             `;
@@ -2316,6 +2318,7 @@ describe('compiler compliance', () => {
             factory: function ForOfDirective_Factory(t) {
               return new (t || ForOfDirective)($r3$.ɵdirectiveInject(ViewContainerRef), $r3$.ɵdirectiveInject(TemplateRef));
             },
+            features: [$r3$.ɵNgOnChangesFeature()],
             inputs: {forOf: "forOf"}
           });
         `;

packages/compiler/src/compiler_facade_interface.ts

@@ -115,6 +115,7 @@ export interface R3DirectiveMetadataFacade {
   queries: R3QueryMetadataFacade[];
   host: {[key: string]: string};
   propMetadata: {[key: string]: any[]};
+  lifecycle: {usesOnChanges: boolean;};
   inputs: string[];
   outputs: string[];
   usesInheritance: boolean;

packages/compiler/src/render3/r3_identifiers.ts

@@ -188,6 +188,8 @@ export class Identifiers {
   static registerContentQuery:
       o.ExternalReference = {name: 'ɵregisterContentQuery', moduleName: CORE};
 
+  static NgOnChangesFeature: o.ExternalReference = {name: 'ɵNgOnChangesFeature', moduleName: CORE};
+
   static InheritDefinitionFeature:
       o.ExternalReference = {name: 'ɵInheritDefinitionFeature', moduleName: CORE};
 

packages/compiler/src/render3/view/api.ts

@@ -74,6 +74,17 @@ export interface R3DirectiveMetadata {
     properties: {[key: string]: string};
   };
 
+  /**
+   * Information about usage of specific lifecycle events which require special treatment in the
+   * code generator.
+   */
+  lifecycle: {
+    /**
+     * Whether the directive uses NgOnChanges.
+     */
+    usesOnChanges: boolean;
+  };
+
   /**
    * A mapping of input field names to the property names.
    */

packages/compiler/src/render3/view/compiler.ts

@@ -125,6 +125,7 @@ function baseDirectiveFields(
  */
 function addFeatures(
     definitionMap: DefinitionMap, meta: R3DirectiveMetadata | R3ComponentMetadata) {
+  // e.g. `features: [NgOnChangesFeature()]`
   const features: o.Expression[] = [];
 
   const providers = meta.providers;
@@ -140,7 +141,9 @@ function addFeatures(
   if (meta.usesInheritance) {
     features.push(o.importExpr(R3.InheritDefinitionFeature));
   }
-
+  if (meta.lifecycle.usesOnChanges) {
+    features.push(o.importExpr(R3.NgOnChangesFeature).callFn(EMPTY_ARRAY));
+  }
   if (features.length) {
     definitionMap.set('features', o.literalArr(features));
   }
@@ -421,6 +424,10 @@ function directiveMetadataFromGlobalMetadata(
     selector: directive.selector,
     deps: dependenciesFromGlobalMetadata(directive.type, outputCtx, reflector),
     queries: queriesFromGlobalMetadata(directive.queries, outputCtx),
+    lifecycle: {
+      usesOnChanges:
+          directive.type.lifecycleHooks.some(lifecycle => lifecycle == LifecycleHooks.OnChanges),
+    },
     host: {
       attributes: directive.hostAttributes,
       listeners: summary.hostListeners,

packages/core/src/compiler/compiler_facade_interface.ts

@@ -115,6 +115,7 @@ export interface R3DirectiveMetadataFacade {
   queries: R3QueryMetadataFacade[];
   host: {[key: string]: string};
   propMetadata: {[key: string]: any[]};
+  lifecycle: {usesOnChanges: boolean;};
   inputs: string[];
   outputs: string[];
   usesInheritance: boolean;

packages/core/src/core_render3_private_export.ts

@@ -28,6 +28,7 @@ export {
   templateRefExtractor as ɵtemplateRefExtractor,
   ProvidersFeature as ɵProvidersFeature,
   InheritDefinitionFeature as ɵInheritDefinitionFeature,
+  NgOnChangesFeature as ɵNgOnChangesFeature,
   LifecycleHooksFeature as ɵLifecycleHooksFeature,
   NgModuleType as ɵNgModuleType,
   NgModuleRef as ɵRender3NgModuleRef,

packages/core/src/metadata/directives.ts

@@ -9,12 +9,13 @@
 import {ChangeDetectionStrategy} from '../change_detection/constants';
 import {Provider} from '../di';
 import {Type} from '../interface/type';
-import {NG_BASE_DEF, NG_COMPONENT_DEF, NG_DIRECTIVE_DEF} from '../render3/fields';
+import {NG_BASE_DEF} from '../render3/fields';
 import {compileComponent as render3CompileComponent, compileDirective as render3CompileDirective} from '../render3/jit/directive';
 import {compilePipe as render3CompilePipe} from '../render3/jit/pipe';
 import {TypeDecorator, makeDecorator, makePropDecorator} from '../util/decorators';
 import {noop} from '../util/noop';
 import {fillProperties} from '../util/property';
+
 import {ViewEncapsulation} from './view';
 
 
@@ -714,54 +715,29 @@ const initializeBaseDef = (target: any): void => {
 };
 
 /**
- * Returns a function that will update the static definition on a class to have the
- * appropriate input or output mapping.
- *
- * Will also add an {@link ngBaseDef} property to a directive if no `ngDirectiveDef`
- * or `ngComponentDef` is present. This is done because a class may have {@link InputDecorator}s and
- * {@link OutputDecorator}s without having a {@link ComponentDecorator} or {@link DirectiveDecorator},
- * and those inputs and outputs should still be inheritable, we need to add an
- * `ngBaseDef` property if there are no existing `ngComponentDef` or `ngDirectiveDef`
- * properties, so that we can track the inputs and outputs for inheritance purposes.
- *
- * @param getPropertyToUpdate A function that maps to either the `inputs` property or the
- * `outputs` property of a definition.
- * @returns A function that, the called, will add a `ngBaseDef` if no other definition is present,
- * then update the `inputs` or `outputs` on it, depending on what was selected by `getPropertyToUpdate`
- *
- *
- * @see InputDecorator
- * @see OutputDecorator
- * @see InheritenceFeature
+ * Does the work of creating the `ngBaseDef` property for the @Input and @Output decorators.
+ * @param key "inputs" or "outputs"
  */
-function getOrCreateDefinitionAndUpdateMappingFor(
-    getPropertyToUpdate: (baseDef: {inputs?: any, outputs?: any}) => any) {
-  return function updateIOProp(target: any, name: string, ...args: any[]) {
-    const constructor = target.constructor;
-
-    let def: any =
-        constructor[NG_COMPONENT_DEF] || constructor[NG_DIRECTIVE_DEF] || constructor[NG_BASE_DEF];
-
-    if (!def) {
-      initializeBaseDef(target);
-      def = constructor[NG_BASE_DEF];
-    }
-
-    const defProp = getPropertyToUpdate(def);
-    // Use of `in` because we *do* want to check the prototype chain here.
-    if (!(name in defProp)) {
+const updateBaseDefFromIOProp = (getProp: (baseDef: {inputs?: any, outputs?: any}) => any) =>
+    (target: any, name: string, ...args: any[]) => {
+      const constructor = target.constructor;
+
+      if (!constructor.hasOwnProperty(NG_BASE_DEF)) {
+        initializeBaseDef(target);
+      }
+
+      const baseDef = constructor.ngBaseDef;
+      const defProp = getProp(baseDef);
       defProp[name] = args[0];
-    }
-  };
-}
+    };
 
 /**
  * @Annotation
  * @publicApi
  */
 export const Input: InputDecorator = makePropDecorator(
     'Input', (bindingPropertyName?: string) => ({bindingPropertyName}), undefined,
-    getOrCreateDefinitionAndUpdateMappingFor(def => def.inputs || {}));
+    updateBaseDefFromIOProp(baseDef => baseDef.inputs || {}));
 
 /**
  * Type of the Output decorator / constructor function.
@@ -801,7 +777,7 @@ export interface Output { bindingPropertyName?: string; }
  */
 export const Output: OutputDecorator = makePropDecorator(
     'Output', (bindingPropertyName?: string) => ({bindingPropertyName}), undefined,
-    getOrCreateDefinitionAndUpdateMappingFor(def => def.outputs || {}));
+    updateBaseDefFromIOProp(baseDef => baseDef.outputs || {}));
 
 
 

packages/core/src/render3/context_discovery.ts

@@ -12,7 +12,6 @@ import {LContext, MONKEY_PATCH_KEY_NAME} from './interfaces/context';
 import {TNode, TNodeFlags} from './interfaces/node';
 import {RElement} from './interfaces/renderer';
 import {CONTEXT, HEADER_OFFSET, HOST, LView, TVIEW} from './interfaces/view';
-import {unwrapOnChangesDirectiveWrapper} from './onchanges_util';
 import {getComponentViewByIndex, getNativeByTNode, readElementValue, readPatchedData} from './util';
 
 
@@ -258,7 +257,7 @@ function findViaDirective(lView: LView, directiveInstance: {}): number {
     const directiveIndexStart = tNode.directiveStart;
     const directiveIndexEnd = tNode.directiveEnd;
     for (let i = directiveIndexStart; i < directiveIndexEnd; i++) {
-      if (unwrapOnChangesDirectiveWrapper(lView[i]) === directiveInstance) {
+      if (lView[i] === directiveInstance) {
         return tNode.index;
       }
     }

packages/core/src/render3/definition.ts

@@ -194,7 +194,7 @@ export function defineComponent<T>(componentDefinition: {
   /**
    * A list of optional features to apply.
    *
-   * See: {@link ProvidersFeature}
+   * See: {@link NgOnChangesFeature}, {@link ProvidersFeature}
    */
   features?: ComponentDefFeature[];
 
@@ -256,7 +256,6 @@ export function defineComponent<T>(componentDefinition: {
     inputs: null !,   // assigned in noSideEffects
     outputs: null !,  // assigned in noSideEffects
     exportAs: componentDefinition.exportAs || null,
-    onChanges: typePrototype.ngOnChanges || null,
     onInit: typePrototype.ngOnInit || null,
     doCheck: typePrototype.ngDoCheck || null,
     afterContentInit: typePrototype.ngAfterContentInit || null,
@@ -277,6 +276,7 @@ export function defineComponent<T>(componentDefinition: {
     id: 'c',
     styles: componentDefinition.styles || EMPTY_ARRAY,
     _: null as never,
+    setInput: null,
   };
   def._ = noSideEffects(() => {
     const directiveTypes = componentDefinition.directives !;
@@ -381,20 +381,22 @@ export function defineNgModule<T>(def: {type: T} & Partial<NgModuleDef<T>>): nev
  *
 
  */
-function invertObject(obj: any, secondary?: any): any {
-  if (obj == null) return EMPTY_OBJ;
+function invertObject<T>(
+    obj?: {[P in keyof T]?: string | [string, string]},
+    secondary?: {[key: string]: string}): {[P in keyof T]: string} {
+  if (obj == null) return EMPTY_OBJ as any;
   const newLookup: any = {};
   for (const minifiedKey in obj) {
     if (obj.hasOwnProperty(minifiedKey)) {
-      let publicName: string = obj[minifiedKey];
+      let publicName: string|[string, string] = obj[minifiedKey] !;
       let declaredName = publicName;
       if (Array.isArray(publicName)) {
         declaredName = publicName[1];
         publicName = publicName[0];
       }
       newLookup[publicName] = minifiedKey;
       if (secondary) {
-        (secondary[publicName] = declaredName);
+        (secondary[publicName] = declaredName as string);
       }
     }
   }
@@ -471,11 +473,11 @@ export function defineBase<T>(baseDefinition: {
    */
   outputs?: {[P in keyof T]?: string};
 }): BaseDef<T> {
-  const declaredInputs: {[P in keyof T]: P} = {} as any;
+  const declaredInputs: {[P in keyof T]: string} = {} as any;
   return {
-    inputs: invertObject(baseDefinition.inputs, declaredInputs),
+    inputs: invertObject<T>(baseDefinition.inputs as any, declaredInputs),
     declaredInputs: declaredInputs,
-    outputs: invertObject(baseDefinition.outputs),
+    outputs: invertObject<T>(baseDefinition.outputs as any),
   };
 }
 
@@ -567,7 +569,7 @@ export const defineDirective = defineComponent as any as<T>(directiveDefinition:
   /**
    * A list of optional features to apply.
    *
-   * See: {@link ProvidersFeature}, {@link InheritDefinitionFeature}
+   * See: {@link NgOnChangesFeature}, {@link ProvidersFeature}, {@link InheritDefinitionFeature}
    */
   features?: DirectiveDefFeature[];
 

packages/core/src/render3/di.ts

@@ -20,7 +20,6 @@ import {NO_PARENT_INJECTOR, NodeInjectorFactory, PARENT_INJECTOR, RelativeInject
 import {AttributeMarker, TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeFlags, TNodeProviderIndexes, TNodeType} from './interfaces/node';
 import {DECLARATION_VIEW, HOST_NODE, INJECTOR, LView, TData, TVIEW, TView} from './interfaces/view';
 import {assertNodeOfPossibleTypes} from './node_assert';
-import {unwrapOnChangesDirectiveWrapper} from './onchanges_util';
 import {getLView, getPreviousOrParentTNode, setTNodeAndViewData} from './state';
 import {findComponentView, getParentInjectorIndex, getParentInjectorView, hasParentInjector, isComponent, isComponentDef, renderStringify} from './util';
 
@@ -523,8 +522,6 @@ export function getNodeInjectable(
       factory.resolving = false;
       setTNodeAndViewData(savePreviousOrParentTNode, saveLView);
     }
-  } else {
-    value = unwrapOnChangesDirectiveWrapper(value);
   }
   return value;
 }

packages/core/src/render3/features/inherit_definition_feature.ts

@@ -7,6 +7,7 @@
  */
 
 import {Type} from '../../interface/type';
+import {Component} from '../../metadata/directives';
 import {fillProperties} from '../../util/property';
 import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty';
 import {ComponentDef, DirectiveDef, DirectiveDefFeature, RenderFlags} from '../interfaces/definition';
@@ -60,6 +61,7 @@ export function InheritDefinitionFeature(definition: DirectiveDef<any>| Componen
     }
 
     if (baseDef) {
+      // Merge inputs and outputs
       fillProperties(definition.inputs, baseDef.inputs);
       fillProperties(definition.declaredInputs, baseDef.declaredInputs);
       fillProperties(definition.outputs, baseDef.outputs);
@@ -124,6 +126,7 @@ export function InheritDefinitionFeature(definition: DirectiveDef<any>| Componen
         }
       }
 
+
       // Merge inputs and outputs
       fillProperties(definition.inputs, superDef.inputs);
       fillProperties(definition.declaredInputs, superDef.declaredInputs);
@@ -139,7 +142,6 @@ export function InheritDefinitionFeature(definition: DirectiveDef<any>| Componen
       definition.doCheck = definition.doCheck || superDef.doCheck;
       definition.onDestroy = definition.onDestroy || superDef.onDestroy;
       definition.onInit = definition.onInit || superDef.onInit;
-      definition.onChanges = definition.onChanges || superDef.onChanges;
 
       // Run parent features
       const features = superDef.features;
@@ -166,7 +168,6 @@ export function InheritDefinitionFeature(definition: DirectiveDef<any>| Componen
         definition.doCheck = definition.doCheck || superPrototype.ngDoCheck;
         definition.onDestroy = definition.onDestroy || superPrototype.ngOnDestroy;
         definition.onInit = definition.onInit || superPrototype.ngOnInit;
-        definition.onChanges = definition.onChanges || superPrototype.ngOnChanges;
       }
     }