refactor(ivy): revert onChanges change back to a feature (angular#28187)

- adds fixmeIvy annotation to tests that should remain updated so we can resolve those issues in the subsequent commits

PR Close angular#28187

Author: benlesh

packages/common/src/directives/ng_template_outlet.ts

@@ -34,20 +34,14 @@ import {Directive, EmbeddedViewRef, Input, OnChanges, SimpleChange, SimpleChange
  */
 @Directive({selector: '[ngTemplateOutlet]'})
 export class NgTemplateOutlet implements OnChanges {
-  private _viewRef: EmbeddedViewRef<any>|null = null;
+  // TODO(issue/24571): remove '!'.
+  private _viewRef !: EmbeddedViewRef<any>;
 
-  /**
-   * A context object to attach to the {@link EmbeddedViewRef}. This should be an
-   * object, the object's keys will be available for binding by the local template `let`
-   * declarations.
-   * Using the key `$implicit` in the context object will set its value as default.
-   */
-  @Input() public ngTemplateOutletContext: Object|null = null;
+  // TODO(issue/24571): remove '!'.
+  @Input() public ngTemplateOutletContext !: Object;
 
-  /**
-   * A string defining the template reference and optionally the context object for the template.
-   */
-  @Input() public ngTemplateOutlet: TemplateRef<any>|null = null;
+  // TODO(issue/24571): remove '!'.
+  @Input() public ngTemplateOutlet !: TemplateRef<any>;
 
   constructor(private _viewContainerRef: ViewContainerRef) {}
 
@@ -103,7 +97,7 @@ export class NgTemplateOutlet implements OnChanges {
 
   private _updateExistingContext(ctx: Object): void {
     for (let propName of Object.keys(ctx)) {
-      (<any>this._viewRef !.context)[propName] = (<any>this.ngTemplateOutletContext)[propName];
+      (<any>this._viewRef.context)[propName] = (<any>this.ngTemplateOutletContext)[propName];
     }
   }
 }

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) {},
@@ -2238,6 +2239,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"}
               });
             `;
@@ -2313,6 +2315,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));
+  }
   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/change_detection/change_detection_util.ts

@@ -64,6 +64,20 @@ export class WrappedValue {
   static isWrapped(value: any): value is WrappedValue { return value instanceof WrappedValue; }
 }
 
+/**
+ * Represents a basic change from a previous to a new value.
+ *
+ * @publicApi
+ */
+export class SimpleChange {
+  constructor(public previousValue: any, public currentValue: any, public firstChange: boolean) {}
+
+  /**
+   * Check whether the new value is the first value assigned.
+   */
+  isFirstChange(): boolean { return this.firstChange; }
+}
+
 export function isListLikeIterable(obj: any): boolean {
   if (!isJsObject(obj)) return false;
   return Array.isArray(obj) ||

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/interface/lifecycle_hooks.ts

@@ -5,9 +5,19 @@
  * 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 {SimpleChanges} from './simple_change';
+import {SimpleChanges, SimpleChange} from './simple_change';
 
 
+/**
+ * Defines an object that associates properties with
+ * instances of `SimpleChange`.
+ *
+ * @see `OnChanges`
+ *
+ * @publicApi
+ */
+export interface SimpleChanges { [propName: string]: SimpleChange; }
+
 /**
  * @description
  * A lifecycle hook that is called when any data-bound property of a directive changes.

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/component.ts

@@ -17,7 +17,6 @@ import {assertComponentType} from './assert';
 import {getComponentDef} from './definition';
 import {diPublicInInjector, getOrCreateNodeInjectorForNode} from './di';
 import {publishDefaultGlobalUtils} from './global_utils';
-import {registerPostOrderHooks, registerPreOrderHooks} from './hooks';
 import {CLEAN_PROMISE, createLView, createNodeAtIndex, createTNode, createTView, getOrCreateTView, initNodeFlags, instantiateRootComponent, locateHostElement, queueComponentIndexForCheck, refreshDescendantViews} from './instructions';
 import {ComponentDef, ComponentType, RenderFlags} from './interfaces/definition';
 import {TElementNode, TNode, TNodeFlags, TNodeType} from './interfaces/node';
@@ -26,6 +25,7 @@ import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from './inte
 import {CONTEXT, FLAGS, HEADER_OFFSET, HOST, HOST_NODE, LView, LViewFlags, RootContext, RootContextFlags, TVIEW} from './interfaces/view';
 import {enterView, getPreviousOrParentTNode, leaveView, resetComponentState, setCurrentDirectiveDef} from './state';
 import {defaultScheduler, getRootView, readPatchedLView, renderStringify} from './util';
+import { registerPreOrderHooks, registerPostOrderHooks } from './hooks';
 
 
 
@@ -240,8 +240,7 @@ export function LifecycleHooksFeature(component: any, def: ComponentDef<any>): v
   registerPreOrderHooks(dirIndex, def, rootTView);
   // TODO(misko): replace `as TNode` with createTNode call. (needs refactoring to lose dep on
   // LNode).
-  registerPostOrderHooks(
-      rootTView, { directiveStart: dirIndex, directiveEnd: dirIndex + 1 } as TNode);
+  registerPostOrderHooks(rootTView, { directiveStart: dirIndex, directiveEnd: dirIndex + 1 } as TNode);
 }
 
 /**

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,
@@ -567,7 +566,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;
 }