refactor(compiler-cli): finalize transform support for signal input transforms (angular#53521)

Signal inputs do not need coercion members for their transforms. That is because the `InputSignal` type- which is accessible in the class member- already holds the type of potential "write values". This eliminates the need for coercion members which were simply used to somehow capture this write type (especially when libraries are consumed and only `.d.ts` is available). We can simplify this, and also significantlky loosen restrictions of transform functions- given that we can fully rely on TypeScript for inferring the type. There is no requirement in being able to "transplant" the type into different places- hence also allowing supporting transform functions with generics, or overloads. In a follow-up commit, once more parts are place, there will be some compliance tests to ensure these new "loosend restrictions". PR Close angular#53521

Author: devversion

packages/compiler-cli/src/ngtsc/annotations/common/src/input_transforms.ts

@@ -17,6 +17,9 @@ export function compileInputTransformFields(inputs: ClassPropertyMapping<InputMa
  const extraFields: CompileResult[] = [];
 
  for (const input of inputs) {
+ // Note: Signal inputs capture their transform `WriteT` as part of the `InputSignal`.
+ // Such inputs will not have a `transform` captured and not generate coercion members.
+
  if (input.transform) {
  extraFields.push({
  name: `ngAcceptInputType_${input.classPropertyName}`,

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

@@ -11,9 +11,9 @@ import ts from 'typescript';
 
 import {ErrorCode, FatalDiagnosticError, makeRelatedInformation} from '../../../diagnostics';
 import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitter} from '../../../imports';
-import {ClassPropertyMapping, HostDirectiveMeta, InputMapping, InputTransform} from '../../../metadata';
+import {ClassPropertyMapping, DecoratorInputTransform, HostDirectiveMeta, InputMapping} from '../../../metadata';
 import {DynamicValue, EnumValue, PartialEvaluator, ResolvedValue, traceDynamicValue} from '../../../partial_evaluator';
-import {AmbientImport, ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral} from '../../../reflection';
+import {AmbientImport, ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, FunctionDefinition, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral} from '../../../reflection';
 import {CompilationMode} from '../../../transform';
 import {createSourceSpan, createValueHasWrongTypeError, forwardRefResolver, getConstructorDependencies, ReferencesRegistry, toR3Reference, tryUnwrapForwardRef, unwrapConstructorDependencies, unwrapExpression, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference,} from '../../common';
 
@@ -651,7 +651,7 @@ function parseInputsArray(
  const name = value.get('name');
  const alias = value.get('alias');
  const required = value.get('required');
- let transform: InputTransform|null = null;
+ let transform: DecoratorInputTransform|null = null;
 
  if (typeof name !== 'string') {
  throw createValueHasWrongTypeError(
@@ -668,7 +668,8 @@ function parseInputsArray(
  `Transform of value at position ${i} of @Directive.inputs array must be a function`);
  }
 
- transform = parseInputTransformFunction(clazz, name, transformValue, reflector, refEmitter);
+ transform = parseDecoratorInputTransformFunction(
+ clazz, name, transformValue, reflector, refEmitter);
  }
 
  inputs[name] = {
@@ -746,7 +747,7 @@ function tryParseInputFieldMapping(
 
  const publicInputName = alias ?? classPropertyName;
 
- let transform: InputTransform|null = null;
+ let transform: DecoratorInputTransform|null = null;
  if (options instanceof Map && options.has('transform')) {
  const transformValue = options.get('transform');
 
@@ -755,7 +756,7 @@ function tryParseInputFieldMapping(
  optionsNode!, transformValue, `Input transform must be a function`);
  }
 
- transform = parseInputTransformFunction(
+ transform = parseDecoratorInputTransformFunction(
  clazz, classPropertyName, transformValue, reflector, refEmitter);
  }
 
@@ -784,7 +785,8 @@ function tryParseInputFieldMapping(
  classPropertyName,
  bindingPropertyName,
  required: signalInput.isRequired,
- // TODO: signal inputs- followup.
+ // Signal inputs do not capture complex transform metadata.
+ // See more details in the `transform` type of `InputMapping`.
  transform: null,
  };
  }
@@ -821,10 +823,20 @@ function parseInputFields(
  return inputs;
 }
 
-/** Parses the `transform` function and its type of a specific input. */
-export function parseInputTransformFunction(
+/**
+ * Parses the `transform` function and its type for a decorator `@Input`.
+ *
+ * This logic verifies feasibility of extracting the transform write type
+ * into a different place, so that the input write type can be captured at
+ * a later point in a static acceptance member.
+ *
+ * Note: This is not needed for signal inputs where the transform type is
+ * automatically captured in the type of the `InputSignal`.
+ *
+ */
+export function parseDecoratorInputTransformFunction(
  clazz: ClassDeclaration, classPropertyName: string, value: DynamicValue|Reference,
- reflector: ReflectionHost, refEmitter: ReferenceEmitter): InputTransform {
+ reflector: ReflectionHost, refEmitter: ReferenceEmitter): DecoratorInputTransform {
  const definition = reflector.getDefinitionOfFunction(value.node);
 
  if (definition === null) {

packages/compiler-cli/src/ngtsc/metadata/src/api.ts

@@ -134,12 +134,35 @@ export enum MatchSource {
 /** Metadata for a single input mapping. */
 export type InputMapping = InputOrOutput&{
  required: boolean;
- transform: InputTransform|null
+
+ /**
+ * Transform for the input. Null if no transform is configured.
+ *
+ * For signal-based inputs, this is always `null` even if a transform
+ * is configured. Signal inputs capture their transform write type
+ * automatically in the `InputSignal`, nor is there a need to emit a
+ * reference to the transform.
+ *
+ * For zone-based decorator `@Input`s this is different because the transform
+ * write type needs to be captured in a coercion member as the decorator information
+ * is lost in the `.d.ts` for type-checking.
+ */
+ transform: DecoratorInputTransform|null
 };
 
-/** Metadata for an input's transform function. */
-export interface InputTransform {
+/** Metadata for an `@Input()` transform function. */
+export interface DecoratorInputTransform {
+ /**
+ * Reference to the transform function so that it can be
+ * referenced when the input metadata is emitted in the declaration.
+ */
  node: ts.Node;
+ /**
+ * Emittable type for the input transform. Null for signal inputs
+ *
+ * This type will be used for inputs to capture the transform type
+ * for type-checking in corresponding `ngAcceptInputType_` members.
+ */
  type: Reference<ts.TypeNode>;
 }
 

packages/compiler-cli/src/ngtsc/metadata/src/dts.ts

@@ -206,7 +206,8 @@ function readInputsType(type: ts.TypeNode): Record<string, InputMapping> {
  // Signal inputs were not supported pre v16- so those inputs are never signal based.
  isSignal: false,
  // Input transform are only tracked for locally-compiled directives. Directives coming
- // from the .d.ts already have them included through `ngAcceptInputType` class members.
+ // from the .d.ts already have them included through `ngAcceptInputType` class members,
+ // or via the `InputSignal` type of the member.
  transform: null,
  };
  } else {
@@ -220,7 +221,8 @@ function readInputsType(type: ts.TypeNode): Record<string, InputMapping> {
  required: config.required,
  isSignal: !!config.isSignal,
  // Input transform are only tracked for locally-compiled directives. Directives coming
- // from the .d.ts already have them included through `ngAcceptInputType` class members.
+ // from the .d.ts already have them included through `ngAcceptInputType` class members,
+ // or via the `InputSignal` type of the member.
  transform: null,
  };
  }

packages/compiler-cli/src/ngtsc/metadata/src/util.ts

@@ -124,7 +124,6 @@ export function extractDirectiveTypeCheckMeta(
  staticMembers.map(extractCoercedInput).filter((inputName): inputName is ClassPropertyName => {
  // If the input refers to a signal input, we will not respect coercion members.
  // A transform function should be used instead.
- // TODO(signals): consider throwing a diagnostic?
  if (inputName === null || inputs.getByClassPropertyName(inputName)?.isSignal) {
  return false;
  }

packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts

@@ -143,14 +143,15 @@ function constructTypeCtorParameter(
  plainKeys.push(
  ts.factory.createLiteralTypeNode(ts.factory.createStringLiteral(classPropertyName)));
  } else {
+ const coercionType = transform != null ?
+ transform.type.node :
+ tsCreateTypeQueryForCoercedInput(rawType.typeName, classPropertyName);
+
  coercedKeys.push(ts.factory.createPropertySignature(
  /* modifiers */ undefined,
  /* name */ classPropertyName,
  /* questionToken */ undefined,
- /* type */
- transform == null ?
- tsCreateTypeQueryForCoercedInput(rawType.typeName, classPropertyName) :
- transform.type.node));
+ /* type */ coercionType));
  }
  }
  if (plainKeys.length > 0) {

packages/compiler-cli/src/ngtsc/typecheck/test/input_signal_diagnostics_spec.ts

@@ -17,6 +17,7 @@ runInEachFileSystem(() => {
  id: string,
  inputs: Record<string, {type: string, isSignal: boolean, restrictionModifier?: string}>,
  template: string,
+ extraDirectiveMembers?: string[],
  component?: string, expected: (string|jasmine.AsymmetricMatcher<string>)[],
  options?: Partial<TypeCheckingConfig>,
  focus?: boolean,
@@ -223,7 +224,23 @@ runInEachFileSystem(() => {
  honorAccessModifiersForInputBindings: false,
  },
  },
- // coercion not supported
+ // coercion is not supported / respected
+ {
+ id: 'coercion members are not respected',
+ inputs: {
+ pattern: {
+ type: 'InputSignal<string, string>',
+ isSignal: true,
+ },
+ },
+ extraDirectiveMembers: [
+ 'static ngAcceptInputType_pattern: string|boolean',
+ ],
+ template: `<div dir [pattern]="false">`,
+ expected: [
+ `TestComponent.html(1, 11): Type 'boolean' is not assignable to type 'string'.`,
+ ],
+ },
  // transforms
  {
  id: 'signal inputs write transform type respected',
@@ -249,6 +266,7 @@ runInEachFileSystem(() => {
 
  class Dir {
  ${inputFields.join('\n')}
+ ${c.extraDirectiveMembers?.join('\n') ?? ''}
  }
  class TestComponent {
  ${c.component ?? ''}

packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts

@@ -13,7 +13,7 @@ import {absoluteFrom, AbsoluteFsPath, getSourceFileOrError, LogicalFileSystem} f
 import {TestFile} from '../../file_system/testing';
 import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, Reference, ReferenceEmitter, RelativePathStrategy} from '../../imports';
 import {NOOP_INCREMENTAL_BUILD} from '../../incremental';
-import {ClassPropertyMapping, CompoundMetadataReader, DirectiveMeta, HostDirectivesResolver, InputMapping, InputTransform, MatchSource, MetadataReaderWithIndex, MetaKind, NgModuleIndex} from '../../metadata';
+import {ClassPropertyMapping, CompoundMetadataReader, DecoratorInputTransform, DirectiveMeta, HostDirectivesResolver, InputMapping, MatchSource, MetadataReaderWithIndex, MetaKind, NgModuleIndex} from '../../metadata';
 import {NOOP_PERF_RECORDER} from '../../perf';
 import {TsCreateProgramDriver} from '../../program_driver';
 import {ClassDeclaration, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
@@ -249,7 +249,7 @@ export interface TestDirective extends Partial<Pick<
  bindingPropertyName: string;
  required: boolean;
  isSignal: boolean;
- transform: InputTransform|null;
+ transform: DecoratorInputTransform|null;
  }
  };
  outputs?: {[fieldName: string]: string};

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

@@ -320,6 +320,12 @@ export interface R3InputMetadata {
  bindingPropertyName: string;
  required: boolean;
  isSignal: boolean;
+ /**
+ * Transform function for the input.
+ *
+ * Null if there is no transform, or if this is a signal input.
+ * Signal inputs capture their transform as part of the `InputSignal`.
+ */
  transformFunction: o.Expression|null;
 }
 

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

@@ -115,6 +115,9 @@ export interface DirectiveDef<T> {
  * A dictionary mapping the private names of inputs to their transformation functions.
  * Note: the private names are used for the keys, rather than the public ones, because public
  * names can be re-aliased in host directives which would invalidate the lookup.
+ *
+ * Note: Signal inputs will not have transforms captured here. This is because their
+ * transform function is already integrated into the `InputSignal`.
  */
  readonly inputTransforms: {[classPropertyName: string]: InputTransformFunction}|null;