Skip to content

Commit

Permalink
feat(ivy): input type coercion for template type-checking
Browse files Browse the repository at this point in the history
Often the types of an `@Input`'s field don't fully reflect the types of
assignable values. This can happen when an input has a getter/setter pair
where the getter always returns a narrow type, and the setter coerces a
wider value down to the narrow type.

For example, you could imagine an input of the form:

```typescript
@input() get value(): string {
  return this._value;
}

set value(v: {toString(): string}) {
  this._value = v.toString();
}
```

Here, the getter always returns a `string`, but the setter accepts any value
that can be `toString()`'d, and coerces it to a string.

Unfortunately TypeScript does not actually support this syntax, and so
Angular users are forced to type their setters as narrowly as the getters,
even though at runtime the coercion works just fine.

To support these kinds of patterns (e.g. as used by Material), this commit
adds a compiler feature called "input coercion". When a binding is made to
the 'value' input of a directive like MatInput, the compiler will look for a
static function with the name ngCoerceInput_value. If such a function is
found, the type-checking expression for the input will be wrapped in a call
to the function, allowing for the expression of a type conversion between
the binding expression and the value being written to the input's field.

To solve the case above, for example, MatInput might write:

```typescript
class MatInput {
  // rest of the directive...

  static ngCoerceInput_value(value: {toString(): string}): string {
    return null!;
  }
}
```

FW-1475 #resolve
  • Loading branch information
alxhub committed Oct 17, 2019
1 parent d5b5900 commit fea1589
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 4 deletions.
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/metadata/src/api.ts
Expand Up @@ -34,6 +34,7 @@ export interface DirectiveMeta extends T2DirectiveMeta {
queries: string[];
ngTemplateGuards: TemplateGuardMeta[];
hasNgTemplateContextGuard: boolean;
coercedInputs: Set<string>;

/**
* A `Reference` to the base class for the directive, if one was detected.
Expand Down
14 changes: 13 additions & 1 deletion packages/compiler-cli/src/ngtsc/metadata/src/util.ts
Expand Up @@ -82,13 +82,18 @@ export function readStringArrayType(type: ts.TypeNode): string[] {
export function extractDirectiveGuards(node: ClassDeclaration, reflector: ReflectionHost): {
ngTemplateGuards: TemplateGuardMeta[],
hasNgTemplateContextGuard: boolean,
coercedInputs: Set<string>,
} {
const staticMembers = reflector.getMembersOfClass(node).filter(member => member.isStatic);
const ngTemplateGuards = staticMembers.map(extractTemplateGuard)
.filter((guard): guard is TemplateGuardMeta => guard !== null);
const hasNgTemplateContextGuard = staticMembers.some(
member => member.kind === ClassMemberKind.Method && member.name === 'ngTemplateContextGuard');
return {hasNgTemplateContextGuard, ngTemplateGuards};

const coercedInputs =
new Set(staticMembers.map(extractCoercedInput)
.filter((inputName): inputName is string => inputName !== null));
return {hasNgTemplateContextGuard, ngTemplateGuards, coercedInputs};
}

function extractTemplateGuard(member: ClassMember): TemplateGuardMeta|null {
Expand All @@ -115,6 +120,13 @@ function extractTemplateGuard(member: ClassMember): TemplateGuardMeta|null {
}
}

function extractCoercedInput(member: ClassMember): string|null {
if (!member.name.startsWith('ngCoerceInput_')) {
return null !;
}
return member.name.split('_', 2)[1];
}

/**
* A `MetadataReader` that reads from an ordered set of child readers until it obtains the requested
* metadata.
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts
Expand Up @@ -228,6 +228,7 @@ function fakeDirective(ref: Reference<ClassDeclaration>): DirectiveMeta {
queries: [],
hasNgTemplateContextGuard: false,
ngTemplateGuards: [],
coercedInputs: new Set<string>(),
baseClass: null,
};
}
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/src/api.ts
Expand Up @@ -21,6 +21,7 @@ export interface TypeCheckableDirectiveMeta extends DirectiveMeta {
ref: Reference<ClassDeclaration>;
queries: string[];
ngTemplateGuards: TemplateGuardMeta[];
coercedInputs: Set<string>;
hasNgTemplateContextGuard: boolean;
}

Expand Down
10 changes: 10 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
Expand Up @@ -880,6 +880,16 @@ function tcbGetDirectiveInputs(
expr = ts.createStringLiteral(attr.value);
}

// Wrap the expression if the directive has a coercion function provided.
if (dir.coercedInputs.has(attr.name)) {
const dirId = tcb.env.reference(dir.ref as Reference<ClassDeclaration<ts.ClassDeclaration>>);
const coercionFn = ts.createPropertyAccess(dirId, `ngCoerceInput_${attr.name}`);
expr = ts.createCall(
/* expression */ coercionFn,
/* typeArguments */ undefined,
/* argumentsArray */[expr]);
}

directiveInputs.push({
type: 'binding',
field: field,
Expand Down
12 changes: 9 additions & 3 deletions packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts
Expand Up @@ -128,9 +128,14 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = {
};

// Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead.
export type TestDirective =
Partial<Pick<TypeCheckableDirectiveMeta, Exclude<keyof TypeCheckableDirectiveMeta, 'ref'>>>&
{selector: string, name: string, file?: AbsoluteFsPath, type: 'directive'};
export type TestDirective = Partial<Pick<
TypeCheckableDirectiveMeta,
Exclude<keyof TypeCheckableDirectiveMeta, 'ref'|'coercedInputs'>>>&
{
selector: string,
name: string, file?: AbsoluteFsPath,
type: 'directive', coercedInputs?: string[],
};
export type TestPipe = {
name: string,
file?: AbsoluteFsPath,
Expand Down Expand Up @@ -257,6 +262,7 @@ function prepareDeclarations(
inputs: decl.inputs || {},
isComponent: decl.isComponent || false,
ngTemplateGuards: decl.ngTemplateGuards || [],
coercedInputs: new Set<string>(decl.coercedInputs || []),
outputs: decl.outputs || {},
queries: decl.queries || [],
};
Expand Down
Expand Up @@ -235,6 +235,34 @@ describe('type check blocks', () => {
});
});

describe('input coercion', () => {
it('should coerce a basic input', () => {
const DIRECTIVES: TestDeclaration[] = [{
type: 'directive',
name: 'MatInput',
selector: '[matInput]',
inputs: {'value': 'value'},
coercedInputs: ['value'],
}];
const TEMPLATE = `<input matInput [value]="expr">`;
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('value: (MatInput.ngCoerceInput_value((ctx).expr))');
});

it('should coerce based on input name, not field name', () => {
const DIRECTIVES: TestDeclaration[] = [{
type: 'directive',
name: 'MatInput',
selector: '[matInput]',
inputs: {'field': 'value'},
coercedInputs: ['value'],
}];
const TEMPLATE = `<input matInput [value]="expr">`;
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('field: (MatInput.ngCoerceInput_value((ctx).expr))');
});
});

describe('config', () => {
const DIRECTIVES: TestDeclaration[] = [{
type: 'directive',
Expand Down
71 changes: 71 additions & 0 deletions packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
Expand Up @@ -370,6 +370,77 @@ export declare class CommonModule {
expect(diags[1].length).toEqual(15);
});

describe('input coercion', () => {
beforeEach(() => {
env.tsconfig({
'fullTemplateTypeCheck': true,
});
env.write('node_modules/@angular/material/index.d.ts', `
import * as i0 from '@angular/core';
export declare class MatInput {
value: string;
static ɵdir: i0.ɵɵDirectiveDefWithMeta<MatInput, '[matInput]', never, {'value': 'value'}, {}, never>;
static ngCoerceInput_value(v: string|number): string;
}
export declare class MatInputModule {
static ɵmod: i0.ɵɵNgModuleDefWithMeta<MatInputModule, [typeof MatInput], never, [typeof MatInput]>;
}
`);
});

it('should coerce an input using a coercion function if provided', () => {
env.write('test.ts', `
import {Component, NgModule} from '@angular/core';
import {MatInputModule} from '@angular/material';
@Component({
selector: 'blah',
template: '<input matInput [value]="someNumber">',
})
export class FooCmp {
someNumber = 3;
}
@NgModule({
declarations: [FooCmp],
imports: [MatInputModule],
})
export class FooModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});

it('should give an error if the binding expression type is not accepted by the coercion function',
() => {
env.write('test.ts', `
import {Component, NgModule} from '@angular/core';
import {MatInputModule} from '@angular/material';
@Component({
selector: 'blah',
template: '<input matInput [value]="invalidType">',
})
export class FooCmp {
invalidType = true;
}
@NgModule({
declarations: [FooCmp],
imports: [MatInputModule],
})
export class FooModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText)
.toBe(
`Argument of type 'boolean' is not assignable to parameter of type 'string | number'.`);
});
});

describe('legacy schema checking with the DOM schema', () => {
beforeEach(
() => { env.tsconfig({ivyTemplateTypeCheck: true, fullTemplateTypeCheck: false}); });
Expand Down

0 comments on commit fea1589

Please sign in to comment.