Skip to content
Permalink
Browse files

fix(ivy): handle non-standard input/output names in template type che…

…cking (#33741)

The template type checker generates code to check directive inputs and
outputs, whose name may contain characters that can not be used as
identifier in TypeScript. Prior to this change, such names would be
emitted into the generated code as is, resulting in invalid code and
unexpected template type check errors.

This commit fixes the bug by representing the potentially invalid names
as string literal instead of raw identifier.

Fixes #33590

PR Close #33741
  • Loading branch information
JoostK authored and alxhub committed Nov 11, 2019
1 parent fbd2133 commit 94257ac8d39b15a7360e19f392c9281cb58ea072
@@ -415,7 +415,7 @@ class TcbUnclaimedInputsOp extends TcbOp {
if (binding.name !== 'style' && binding.name !== 'class') {
// A direct binding to a property.
const propertyName = ATTR_TO_PROP[binding.name] || binding.name;
const prop = ts.createPropertyAccess(elId, propertyName);
const prop = ts.createElementAccess(elId, ts.createStringLiteral(propertyName));
const stmt = ts.createBinary(prop, ts.SyntaxKind.EqualsToken, wrapForDiagnostics(expr));
addParseSpanInfo(stmt, binding.sourceSpan);
this.scope.addStatement(ts.createExpressionStatement(stmt));
@@ -478,7 +478,7 @@ class TcbDirectiveOutputsOp extends TcbOp {
// that has a `subscribe` method that properly carries the `T` into the handler function.
const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Infer);

const outputField = ts.createPropertyAccess(dirId, field);
const outputField = ts.createElementAccess(dirId, ts.createStringLiteral(field));
const outputHelper =
ts.createCall(this.tcb.env.declareOutputHelper(), undefined, [outputField]);
const subscribeFn = ts.createPropertyAccess(outputHelper, 'subscribe');
@@ -1118,6 +1118,8 @@ function tcbCallTypeCtor(

// Construct an array of `ts.PropertyAssignment`s for each of the directive's inputs.
const members = inputs.map(input => {
const propertyName = ts.createStringLiteral(input.field);

if (input.type === 'binding') {
// For bound inputs, the property is assigned the binding expression.
let expr = input.expression;
@@ -1131,13 +1133,13 @@ function tcbCallTypeCtor(
expr = ts.createNonNullExpression(expr);
}

const assignment = ts.createPropertyAssignment(input.field, wrapForDiagnostics(expr));
const assignment = ts.createPropertyAssignment(propertyName, wrapForDiagnostics(expr));
addParseSpanInfo(assignment, input.sourceSpan);
return assignment;
} else {
// A type constructor is required to be called with all input properties, so any unset
// inputs are simply assigned a value of type `any` to ignore them.
return ts.createPropertyAssignment(input.field, NULL_AS_ANY);
return ts.createPropertyAssignment(propertyName, NULL_AS_ANY);
}
});

@@ -43,7 +43,7 @@ describe('type check blocks', () => {
selector: '[dir]',
inputs: {inputA: 'inputA'},
}];
expect(tcb(TEMPLATE, DIRECTIVES)).toContain('inputA: ("value")');
expect(tcb(TEMPLATE, DIRECTIVES)).toContain('"inputA": ("value")');
});

it('should handle empty bindings', () => {
@@ -54,7 +54,7 @@ describe('type check blocks', () => {
selector: '[dir-a]',
inputs: {inputA: 'inputA'},
}];
expect(tcb(TEMPLATE, DIRECTIVES)).toContain('inputA: (undefined)');
expect(tcb(TEMPLATE, DIRECTIVES)).toContain('"inputA": (undefined)');
});

it('should handle bindings without value', () => {
@@ -65,7 +65,7 @@ describe('type check blocks', () => {
selector: '[dir-a]',
inputs: {inputA: 'inputA'},
}];
expect(tcb(TEMPLATE, DIRECTIVES)).toContain('inputA: (undefined)');
expect(tcb(TEMPLATE, DIRECTIVES)).toContain('"inputA": (undefined)');
});

it('should handle implicit vars on ng-template', () => {
@@ -96,7 +96,7 @@ describe('type check blocks', () => {
},
}];
expect(tcb(TEMPLATE, DIRECTIVES))
.toContain('var _t2 = Dir.ngTypeCtor({ fieldA: ((ctx).foo), fieldB: (null as any) });');
.toContain('var _t2 = Dir.ngTypeCtor({ "fieldA": ((ctx).foo), "fieldB": (null as any) });');
});

it('should generate a forward element reference correctly', () => {
@@ -147,7 +147,7 @@ describe('type check blocks', () => {
}];
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain(
'var _t2 = Dir.ngTypeCtor({ color: (null as any), strong: (null as any), enabled: (null as any) });');
'var _t2 = Dir.ngTypeCtor({ "color": (null as any), "strong": (null as any), "enabled": (null as any) });');
expect(block).toContain('"blue"; false; true;');
});

@@ -162,7 +162,7 @@ describe('type check blocks', () => {
exportAs: ['dir'],
inputs: {input: 'input'},
}];
expect(tcb(TEMPLATE, DIRECTIVES)).toContain('var _t2 = Dir.ngTypeCtor({ input: (null!) });');
expect(tcb(TEMPLATE, DIRECTIVES)).toContain('var _t2 = Dir.ngTypeCtor({ "input": (null!) });');
});

it('should generate circular references between two directives correctly', () => {
@@ -188,8 +188,8 @@ describe('type check blocks', () => {
];
expect(tcb(TEMPLATE, DIRECTIVES))
.toContain(
'var _t3 = DirB.ngTypeCtor({ inputA: (null!) }); ' +
'var _t2 = DirA.ngTypeCtor({ inputA: (_t3) });');
'var _t3 = DirB.ngTypeCtor({ "inputA": (null!) }); ' +
'var _t2 = DirA.ngTypeCtor({ "inputA": (_t3) });');
});

it('should handle $any casts', () => {
@@ -203,7 +203,7 @@ describe('type check blocks', () => {
const TEMPLATE = `<label [for]="'test'"></label>`;
const CONFIG = {...ALL_ENABLED_CONFIG, checkTypeOfDomBindings: true};
expect(tcb(TEMPLATE, /* declarations */ undefined, CONFIG))
.toContain('_t1.htmlFor = ("test");');
.toContain('_t1["htmlFor"] = ("test");');
});
});

@@ -253,7 +253,7 @@ describe('type check blocks', () => {
const TEMPLATE = `<div dir (dirOutput)="foo($event)"></div>`;
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain(
'_outputHelper(_t2.outputField).subscribe(($event): any => (ctx).foo($event));');
'_outputHelper(_t2["outputField"]).subscribe(($event): any => (ctx).foo($event));');
});

it('should emit a listener function with AnimationEvent for animation events', () => {
@@ -339,14 +339,14 @@ describe('type check blocks', () => {

it('should include null and undefined when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('Dir.ngTypeCtor({ dirInput: ((ctx).a) })');
expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((ctx).a) })');
expect(block).toContain('(ctx).b;');
});
it('should use the non-null assertion operator when disabled', () => {
const DISABLED_CONFIG:
TypeCheckingConfig = {...BASE_CONFIG, strictNullInputBindings: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('Dir.ngTypeCtor({ dirInput: ((ctx).a!) })');
expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((ctx).a!) })');
expect(block).toContain('(ctx).b!;');
});
});
@@ -356,14 +356,14 @@ describe('type check blocks', () => {

it('should check types of bindings when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('Dir.ngTypeCtor({ dirInput: ((ctx).a) })');
expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((ctx).a) })');
expect(block).toContain('(ctx).b;');
});
it('should not check types of bindings when disabled', () => {
const DISABLED_CONFIG:
TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('Dir.ngTypeCtor({ dirInput: (((ctx).a as any)) })');
expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a as any)) })');
expect(block).toContain('((ctx).b as any);');
});
});
@@ -374,7 +374,7 @@ describe('type check blocks', () => {
it('should check types of directive outputs when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain(
'_outputHelper(_t2.outputField).subscribe(($event): any => (ctx).foo($event));');
'_outputHelper(_t2["outputField"]).subscribe(($event): any => (ctx).foo($event));');
expect(block).toContain(
'_t1.addEventListener("nonDirOutput", ($event): any => (ctx).foo($event));');
});
@@ -410,7 +410,7 @@ describe('type check blocks', () => {
it('should check types of DOM events when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain(
'_outputHelper(_t2.outputField).subscribe(($event): any => (ctx).foo($event));');
'_outputHelper(_t2["outputField"]).subscribe(($event): any => (ctx).foo($event));');
expect(block).toContain(
'_t1.addEventListener("nonDirOutput", ($event): any => (ctx).foo($event));');
});
@@ -420,7 +420,7 @@ describe('type check blocks', () => {
// Note that directive outputs are still checked, that is controlled by
// `checkTypeOfOutputEvents`
expect(block).toContain(
'_outputHelper(_t2.outputField).subscribe(($event): any => (ctx).foo($event));');
'_outputHelper(_t2["outputField"]).subscribe(($event): any => (ctx).foo($event));');
expect(block).toContain('($event: any): any => (ctx).foo($event);');
});
});
@@ -483,17 +483,17 @@ describe('type check blocks', () => {

it('should assign string value to the input when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('disabled: ("")');
expect(block).toContain('cols: ("3")');
expect(block).toContain('rows: (2)');
expect(block).toContain('"disabled": ("")');
expect(block).toContain('"cols": ("3")');
expect(block).toContain('"rows": (2)');
});

it('should use any for attributes but still check bound attributes when disabled', () => {
const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfAttributes: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('disabled: (null as any)');
expect(block).toContain('cols: (null as any)');
expect(block).toContain('rows: (2)');
expect(block).toContain('"disabled": (null as any)');
expect(block).toContain('"cols": (null as any)');
expect(block).toContain('"rows": (2)');
});
});

@@ -123,6 +123,43 @@ export declare class AnimationEvent {
expect(diags[0].messageText).toEqual(`Type 'string' is not assignable to type 'number'.`);
});

it('should support inputs and outputs with names that are not JavaScript identifiers', () => {
env.tsconfig(
{fullTemplateTypeCheck: true, strictInputTypes: true, strictOutputEventTypes: true});
env.write('test.ts', `
import {Component, Directive, NgModule, EventEmitter} from '@angular/core';
@Component({
selector: 'test',
template: '<div dir [some-input.xs]="2" (some-output)="handleEvent($event)"></div>',
})
class TestCmp {
handleEvent(event: number): void {}
}
@Directive({
selector: '[dir]',
inputs: ['some-input.xs'],
outputs: ['some-output'],
})
class TestDir {
'some-input.xs': string;
'some-output': EventEmitter<string>;
}
@NgModule({
declarations: [TestCmp, TestDir],
})
class Module {}
`);

const diags = env.driveDiagnostics();
expect(diags.length).toBe(2);
expect(diags[0].messageText).toEqual(`Type 'number' is not assignable to type 'string'.`);
expect(diags[1].messageText)
.toEqual(`Argument of type 'string' is not assignable to parameter of type 'number'.`);
});

it('should check event bindings', () => {
env.tsconfig({fullTemplateTypeCheck: true, strictOutputEventTypes: true});
env.write('test.ts', `

0 comments on commit 94257ac

Please sign in to comment.
You can’t perform that action at this time.