honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): handle non-standard input/output names in template type checking (#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
This commit is contained in:
JoostK 2019-11-11 21:03:23 +01:00 committed by Alex Rickabaugh
parent fd83d9479a
commit 70311ebca1
3 changed files with 66 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
View File

@ -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);
}
});

46
packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
View File

@ -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)');
});
});

37
packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
View File

@ -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', `