Skip to content

Commit

Permalink
revert: feat(ivy): input type coercion for template type-checking (an…
Browse files Browse the repository at this point in the history
…gular#33243)

This reverts commit 1b4eaea.
  • Loading branch information
matsko committed Oct 21, 2019
1 parent 160547d commit 5b7f428
Show file tree
Hide file tree
Showing 8 changed files with 5 additions and 142 deletions.
1 change: 0 additions & 1 deletion packages/compiler-cli/src/ngtsc/metadata/src/api.ts
Expand Up @@ -34,7 +34,6 @@ 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
24 changes: 2 additions & 22 deletions packages/compiler-cli/src/ngtsc/metadata/src/util.ts
Expand Up @@ -82,25 +82,20 @@ 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');

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

function extractTemplateGuard(member: ClassMember): TemplateGuardMeta|null {
if (!member.name.startsWith('ngTemplateGuard_')) {
return null;
}
const inputName = afterUnderscore(member.name);
const inputName = member.name.split('_', 2)[1];
if (member.kind === ClassMemberKind.Property) {
let type: string|null = null;
if (member.type !== null && ts.isLiteralTypeNode(member.type) &&
Expand All @@ -120,13 +115,6 @@ function extractTemplateGuard(member: ClassMember): TemplateGuardMeta|null {
}
}

function extractCoercedInput(member: ClassMember): string|null {
if (!member.name.startsWith('ngCoerceInput_')) {
return null !;
}
return afterUnderscore(member.name);
}

/**
* A `MetadataReader` that reads from an ordered set of child readers until it obtains the requested
* metadata.
Expand Down Expand Up @@ -170,11 +158,3 @@ export class CompoundMetadataReader implements MetadataReader {
return null;
}
}

function afterUnderscore(str: string): string {
const pos = str.indexOf('_');
if (pos === -1) {
throw new Error(`Expected '${str}' to contain '_'`);
}
return str.substr(pos + 1);
}
1 change: 0 additions & 1 deletion packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts
Expand Up @@ -228,7 +228,6 @@ function fakeDirective(ref: Reference<ClassDeclaration>): DirectiveMeta {
queries: [],
hasNgTemplateContextGuard: false,
ngTemplateGuards: [],
coercedInputs: new Set<string>(),
baseClass: null,
};
}
Expand Down
1 change: 0 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/src/api.ts
Expand Up @@ -21,7 +21,6 @@ export interface TypeCheckableDirectiveMeta extends DirectiveMeta {
ref: Reference<ClassDeclaration>;
queries: string[];
ngTemplateGuards: TemplateGuardMeta[];
coercedInputs: Set<string>;
hasNgTemplateContextGuard: boolean;
}

Expand Down
10 changes: 0 additions & 10 deletions packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
Expand Up @@ -1144,16 +1144,6 @@ 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: 3 additions & 9 deletions packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts
Expand Up @@ -160,14 +160,9 @@ 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'|'coercedInputs'>>>&
{
selector: string,
name: string, file?: AbsoluteFsPath,
type: 'directive', coercedInputs?: string[],
};
export type TestDirective =
Partial<Pick<TypeCheckableDirectiveMeta, Exclude<keyof TypeCheckableDirectiveMeta, 'ref'>>>&
{selector: string, name: string, file?: AbsoluteFsPath, type: 'directive'};
export type TestPipe = {
name: string,
file?: AbsoluteFsPath,
Expand Down Expand Up @@ -300,7 +295,6 @@ 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 @@ -268,34 +268,7 @@ describe('type check blocks', () => {
expect(block).toContain(
'_t1.addEventListener("event", $event => (ctx).foo(($event as any)));');
});
});

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', () => {
Expand Down
71 changes: 0 additions & 71 deletions packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
Expand Up @@ -406,77 +406,6 @@ 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 5b7f428

Please sign in to comment.