Skip to content
Permalink
Browse files

fix(ivy): don't infer template context types when in full mode

The Ivy template type-checker is capable of inferring the type of a
structural directive (such as NgForOf<T>). Previously, this was done with
fullTemplateTypeCheck: true, even if strictTemplates was false. View Engine
previously did not do this inference, and so this causes breakages if the
type of the template context is not what the user expected.

In particular, consider the template:

```html
<div *ngFor="let user of users as all">
  {{user.index}} out of {{all.length}}
</div>
```

As long as `users` is an array, this seems reasonable, because it appears
that `all` is an alias for the `users` array. However, this is misleading.

In reality, `NgForOf` is rendered with a template context that contains
both a `$implicit` value (for the loop variable `user`) as well as a
`ngForOf` value, which is the actual value assigned to `all`. The type of
`NgForOf`'s template context is `NgForContext<T>`, which declares `ngForOf`'s
type to be `NgIterable<T>`, which does not have a `length` property (due to
its incorporation of the `Iterable` type).

This commit stops the template type-checker from inferring template context
types unless strictTemplates is set (and strictInputTypes is not disabled).

Fixes angular#33527.
  • Loading branch information
alxhub committed Nov 1, 2019
1 parent bb290ce commit 97ff38e3a92183a70da537c8e4bc6cb1a9f91bf5
@@ -426,7 +426,7 @@ export class NgtscProgram implements api.Program {
if (this.options.fullTemplateTypeCheck) {
const strictTemplates = !!this.options.strictTemplates;
typeCheckingConfig = {
applyTemplateContextGuards: true,
applyTemplateContextGuards: strictTemplates,
checkQueries: false,
checkTemplateBodies: true,
checkTypeOfInputBindings: strictTemplates,
@@ -471,6 +471,7 @@ export class NgtscProgram implements api.Program {
// based on "fullTemplateTypeCheck".
if (this.options.strictInputTypes !== undefined) {
typeCheckingConfig.checkTypeOfInputBindings = this.options.strictInputTypes;
typeCheckingConfig.applyTemplateContextGuards = this.options.strictInputTypes;
}
if (this.options.strictNullInputTypes !== undefined) {
typeCheckingConfig.strictNullInputBindings = this.options.strictNullInputTypes;
@@ -43,6 +43,16 @@ export declare class IndexPipe {
static ɵpipe: i0.ɵPipeDefWithMeta<IndexPipe, 'index'>;
}
export declare class SlicePipe {
transform<T>(value: ReadonlyArray<T>, start: number, end?: number): Array<T>;
transform(value: string, start: number, end?: number): string;
transform(value: null, start: number, end?: number): null;
transform(value: undefined, start: number, end?: number): undefined;
transform(value: any, start: number, end?: number): any;
static ɵpipe: i0.ɵPipeDefWithMeta<SlicePipe, 'slice'>;
}
export declare class NgForOf<T> {
ngForOf: i0.NgIterable<T>;
static ngTemplateContextGuard<T>(dir: NgForOf<T>, ctx: any): ctx is NgForOfContext<T>;
@@ -56,7 +66,7 @@ export declare class NgIf {
}
export declare class CommonModule {
static ɵmod: i0.ɵɵNgModuleDefWithMeta<CommonModule, [typeof NgIf, typeof NgForOf, typeof IndexPipe], never, [typeof NgIf, typeof NgForOf, typeof IndexPipe]>;
static ɵmod: i0.ɵɵNgModuleDefWithMeta<CommonModule, [typeof NgIf, typeof NgForOf, typeof IndexPipe, typeof SlicePipe], never, [typeof NgIf, typeof NgForOf, typeof IndexPipe, typeof SlicePipe]>;
}
`);
env.write('node_modules/@angular/animations/index.d.ts', `
@@ -862,30 +872,48 @@ export declare class AnimationEvent {
expect(diags[0].length).toBe(19);
});

it('should property type-check a microsyntax variable with the same name as the expression',
() => {
env.write('test.ts', `
import {CommonModule} from '@angular/common';
import {Component, Input, NgModule} from '@angular/core';
describe('microsyntax variables', () => {
beforeEach(() => {
// Use the same template for both tests
env.write('test.ts', `
import {CommonModule} from '@angular/common';
import {Component, NgModule} from '@angular/core';
@Component({
selector: 'test',
template: \`<div *ngFor="let foo of foos as foos">
{{foo.name}} of {{foos.length}}
</div>
\`,
})
export class TestCmp {
foos: {name: string}[];
}
@NgModule({
declarations: [TestCmp],
imports: [CommonModule],
})
export class Module {}
`);
});

@Component({
selector: 'test',
template: '<div *ngIf="foo as foo">{{foo}}</div>',
})
export class TestCmp<T extends {name: string}> {
foo: any;
}
it('should be treated as \'any\' without strictTemplates', () => {
env.tsconfig({fullTemplateTypeCheck: true, strictTemplates: false});

@NgModule({
declarations: [TestCmp],
imports: [CommonModule],
})
export class Module {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});

const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
it('should be correctly inferred under strictTemplates', () => {
env.tsconfig({fullTemplateTypeCheck: true, strictTemplates: true});

const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect((diags[0].messageText as ts.DiagnosticMessageChain).messageText)
.toBe(`Property 'length' does not exist on type 'NgIterable<{ name: string; }>'.`);
});
});

it('should properly type-check inherited directives', () => {
env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true});

0 comments on commit 97ff38e

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