fix(ivy): allow directive inheritance in strict mode (#28634)

For TypeScript compilation units that have the "strictFunctionTypes"
option enabled, an error would be produced for Ivy's definition fields
in declaration files in the case of inheritance across directives or
pipes.

This change loosens the definition types to allow for subtypes of the
defined type where necessary.

A test package that has the "strict" option enabled verifies that we
won't regress in environments where strict type checking is enabled.

Fixes #28079

PR Close #28634

packages/core/src/render3/definition.ts

@@ -17,7 +17,7 @@ import {stringify} from '../util/stringify';
 
 import {EMPTY_ARRAY, EMPTY_OBJ} from './empty';
 import {NG_COMPONENT_DEF, NG_DIRECTIVE_DEF, NG_MODULE_DEF, NG_PIPE_DEF} from './fields';
-import {BaseDef, ComponentDef, ComponentDefFeature, ComponentQuery, ComponentTemplate, ComponentType, DirectiveDef, DirectiveDefFeature, DirectiveType, DirectiveTypesOrFactory, HostBindingsFunction, PipeDef, PipeType, PipeTypesOrFactory} from './interfaces/definition';
+import {BaseDef, ComponentDef, ComponentDefFeature, ComponentQuery, ComponentTemplate, ComponentType, DirectiveDef, DirectiveDefFeature, DirectiveType, DirectiveTypesOrFactory, FactoryFn, HostBindingsFunction, PipeDef, PipeType, PipeTypesOrFactory} from './interfaces/definition';
 import {CssSelectorList} from './interfaces/projection';
 
 let _renderCompCount = 0;
@@ -49,7 +49,7 @@ export function defineComponent<T>(componentDefinition: {
   /**
    * Factory method used to create an instance of directive.
    */
-  factory: (t: Type<T>| null) => T;
+  factory: FactoryFn<T>;
 
   /**
    * The number of nodes, local refs, and pipes in this component template.
@@ -515,7 +515,7 @@ export const defineDirective = defineComponent as any as<T>(directiveDefinition:
   /**
    * Factory method used to create an instance of directive.
    */
-  factory: (t: Type<T>| null) => T;
+  factory: FactoryFn<T>;
 
   /**
    * A map of input names.
@@ -624,7 +624,7 @@ export function definePipe<T>(pipeDef: {
   type: Type<T>,
 
   /** A factory for creating a pipe instance. */
-  factory: (t: Type<T>| null) => T,
+  factory: FactoryFn<T>,
 
   /** Whether the pipe is pure. */
   pure?: boolean

packages/core/src/render3/instructions.ts

@@ -432,7 +432,7 @@ function renderComponentOrTemplate<T>(
       // creation mode pass
       if (templateFn) {
         namespaceHTML();
-        templateFn(RenderFlags.Create, context !);
+        templateFn(RenderFlags.Create, context);
       }
 
       refreshDescendantViews(hostView);
@@ -440,7 +440,7 @@ function renderComponentOrTemplate<T>(
     }
 
     // update mode pass
-    templateFn && templateFn(RenderFlags.Update, context !);
+    templateFn && templateFn(RenderFlags.Update, context);
     refreshDescendantViews(hostView);
   } finally {
     if (normalExecutionPath && !creationModeIsActive && rendererFactory.end) {

packages/core/src/render3/interfaces/definition.ts

@@ -15,7 +15,10 @@ import {CssSelectorList} from './projection';
  * Definition of what a template rendering function should look like for a component.
  */
 export type ComponentTemplate<T> = {
-  (rf: RenderFlags, ctx: T): void; ngPrivateData?: never;
+  // Note: the ctx parameter is typed as T|U, as using only U would prevent a template with
+  // e.g. ctx: {} from being assigned to ComponentTemplate<any> as TypeScript won't infer U = any
+  // in that scenario. By including T this incompatibility is resolved.
+  <U extends T>(rf: RenderFlags, ctx: T | U): void; ngPrivateData?: never;
 };
 
 /**
@@ -23,6 +26,22 @@ export type ComponentTemplate<T> = {
  */
 export type ComponentQuery<T> = ComponentTemplate<T>;
 
+/**
+ * Definition of what a factory function should look like.
+ */
+export type FactoryFn<T> = {
+  /**
+   * Subclasses without an explicit constructor call through to the factory of their base
+   * definition, providing it with their own constructor to instantiate.
+   */
+  <U extends T>(t: Type<U>): U;
+
+  /**
+   * If no constructor to instantiate is provided, an instance of type T itself is created.
+   */
+  (t: null): T;
+};
+
 /**
  * Flags passed into template functions to determine which blocks (i.e. creation, update)
  * should be executed.
@@ -113,7 +132,7 @@ export interface DirectiveDef<T> extends BaseDef<T> {
   type: Type<T>;
 
   /** Function that resolves providers and publishes them into the DI system. */
-  providersResolver: ((def: DirectiveDef<T>) => void)|null;
+  providersResolver: (<U extends T>(def: DirectiveDef<U>) => void)|null;
 
   /** The selectors that will be used to match nodes to this directive. */
   readonly selectors: CssSelectorList;
@@ -126,7 +145,7 @@ export interface DirectiveDef<T> extends BaseDef<T> {
   /**
    * Factory function used to create a new directive instance.
    */
-  factory: (t: Type<T>|null) => T;
+  factory: FactoryFn<T>;
 
   /**
    * Function to create instances of content queries associated with a given directive.
@@ -155,7 +174,8 @@ export interface DirectiveDef<T> extends BaseDef<T> {
   readonly features: DirectiveDefFeature[]|null;
 
   setInput:
-      ((this: DirectiveDef<T>, instance: T, value: any, publicName: string,
+      (<U extends T>(
+           this: DirectiveDef<U>, instance: U, value: any, publicName: string,
            privateName: string) => void)|null;
 }
 
@@ -285,7 +305,7 @@ export interface PipeDef<T> {
   /**
    * Factory function used to create a new pipe instance.
    */
-  factory: (t: Type<T>|null) => T;
+  factory: FactoryFn<T>;
 
   /**
    * Whether or not the pipe is pure.
@@ -343,7 +363,8 @@ export type DirectiveTypeList =
     (DirectiveDef<any>| ComponentDef<any>|
      Type<any>/* Type as workaround for: Microsoft/TypeScript/issues/4881 */)[];
 
-export type HostBindingsFunction<T> = (rf: RenderFlags, ctx: T, elementIndex: number) => void;
+export type HostBindingsFunction<T> =
+    <U extends T>(rf: RenderFlags, ctx: U, elementIndex: number) => void;
 
 /**
  * Type used for PipeDefs on component definition.

packages/core/test/render3/di_spec.ts

@@ -184,7 +184,7 @@ describe('di', () => {
           consts: 0,
           vars: 0,
           factory: () => new Comp(directiveInject(DirB)),
-          template: (ctx: any, fm: boolean) => {}
+          template: (rf: RenderFlags, ctx: Comp) => {}
         });
       }
 

packages/core/test/render3/inherit_definition_feature_spec.ts

@@ -7,7 +7,7 @@
  */
 
 import {ElementRef, Inject, InjectionToken, QueryList, ɵAttributeMarker as AttributeMarker} from '../../src/core';
-import {ComponentDef, DirectiveDef, InheritDefinitionFeature, NgOnChangesFeature, ProvidersFeature, RenderFlags, allocHostVars, bind, contentQuery, defineBase, defineComponent, defineDirective, directiveInject, element, elementEnd, elementProperty, elementStart, load, loadContentQuery, loadViewQuery, queryRefresh, viewQuery} from '../../src/render3/index';
+import {allocHostVars, bind, ComponentDef, contentQuery, defineBase, defineComponent, defineDirective, DirectiveDef, directiveInject, element, elementEnd, elementProperty, elementStart, InheritDefinitionFeature, load, loadContentQuery, loadViewQuery, NgOnChangesFeature, ProvidersFeature, queryRefresh, RenderFlags, viewQuery,} from '../../src/render3/index';
 
 import {ComponentFixture, createComponent, getDirectiveOnNode} from './render_util';
 
@@ -457,8 +457,8 @@ describe('InheritDefinitionFeature', () => {
           consts: 0,
           vars: 0,
           selectors: [['', 'subDir', '']],
-          viewQuery: (directiveIndex: number, elementIndex: number) => {
-            log.push(['sub', directiveIndex, elementIndex]);
+          viewQuery: (rf: RenderFlags, ctx: SubComponent) => {
+            log.push(['sub', rf, ctx]);
           },
           factory: () => new SubComponent(),
           features: [InheritDefinitionFeature]
@@ -469,9 +469,10 @@ describe('InheritDefinitionFeature', () => {
 
       const context = {foo: 'bar'};
 
-      subDef.viewQuery !(1, context);
+      subDef.viewQuery !(RenderFlags.Create, context);
 
-      expect(log).toEqual([['super', 1, context], ['sub', 1, context]]);
+      expect(log).toEqual(
+          [['super', RenderFlags.Create, context], ['sub', RenderFlags.Create, context]]);
     });
 
 

packages/core/test/strict_types/BUILD.bazel

@@ -0,0 +1,23 @@
+package(default_visibility = ["//visibility:private"])
+
+load("//tools:defaults.bzl", "jasmine_node_test", "ts_library")
+
+ts_library(
+    name = "strict_types_lib",
+    testonly = True,
+    srcs = glob(
+        ["**/*.ts"],
+    ),
+    tsconfig = ":tsconfig.json",
+    deps = [
+        "//packages/core",
+    ],
+)
+
+jasmine_node_test(
+    name = "strict_types",
+    deps = [
+        ":strict_types_lib",
+        "//tools/testing:node",
+    ],
+)

packages/core/test/strict_types/inheritance_spec.ts

@@ -0,0 +1,37 @@
+/**
+ * @license
+ * Copyright Google Inc. 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 {ɵComponentDefWithMeta as ComponentDefWithMeta, ɵPipeDefWithMeta as PipeDefWithMeta} from '@angular/core';
+
+declare class SuperComponent {
+  static ngComponentDef: ComponentDefWithMeta<SuperComponent, '[super]', never, {}, {}, never>;
+}
+
+declare class SubComponent extends SuperComponent {
+  // Declaring a field in the subtype makes its structure incompatible with that of the
+  // supertype. Special care needs to be taken in Ivy's definition types, or TypeScript
+  // would produce type errors when the "strictFunctionTypes" option is enabled.
+  onlyInSubtype: string;
+
+  static ngComponentDef: ComponentDefWithMeta<SubComponent, '[sub]', never, {}, {}, never>;
+}
+
+declare class SuperPipe { static ngPipeDef: PipeDefWithMeta<SuperPipe, 'super'>; }
+
+declare class SubPipe extends SuperPipe {
+  onlyInSubtype: string;
+
+  static ngPipeDef: PipeDefWithMeta<SubPipe, 'sub'>;
+}
+
+describe('inheritance strict type checking', () => {
+  // Verify that Ivy definition fields in declaration files conform to TypeScript's strict
+  // type checking constraints in the case of inheritance across directives/components/pipes.
+  // https://github.com/angular/angular/issues/28079
+  it('should compile without errors', () => {});
+});

packages/core/test/strict_types/tsconfig.json

@@ -0,0 +1,6 @@
+{
+  "extends": "../../../tsconfig-test.json",
+  "compilerOptions": {
+    "strict": true
+  }
+}