Skip to content

Commit

Permalink
feat(compiler): allow ngIf to use the ngIf expression directly as a g…
Browse files Browse the repository at this point in the history
…uard

Allows a directive to use the expression passed directly to a property
as a guard instead of filtering the type through a type expression.

This more accurately matches the intent of the ngIf usage of its template
enabling better type inference.

Moved NgIf to using this type of guard instead of a function guard.

Closes: #20967
  • Loading branch information
chuckjaz authored and alxhub committed Dec 18, 2017
1 parent e48f477 commit 82bcd83
Show file tree
Hide file tree
Showing 6 changed files with 330 additions and 10 deletions.
3 changes: 2 additions & 1 deletion packages/common/src/directives/ng_if.ts
Expand Up @@ -152,7 +152,8 @@ export class NgIf {
} }
} }


public static ngIfTypeGuard: <T>(v: T|null|undefined|false) => v is T; /** @internal */
public static ngIfUseIfTypeGuard: void;
} }


/** /**
Expand Down
298 changes: 298 additions & 0 deletions packages/compiler-cli/test/diagnostics/check_types_spec.ts
Expand Up @@ -216,6 +216,304 @@ describe('ng type checker', () => {
export class MainModule {}` export class MainModule {}`
}); });
}); });

it('should narrow an *ngIf like directive with UseIf', () => {
a({
'src/app.component.ts': '',
'src/lib.ts': '',
'src/app.module.ts': `
import {NgModule, Component, Directive, HostListener, TemplateRef, Input} from '@angular/core';
export interface Person {
name: string;
}
@Component({
selector: 'comp',
template: '<div *myIf="person"> {{person.name}} </div>'
})
export class MainComp {
person?: Person;
}
export class MyIfContext {
public $implicit: any = null;
public myIf: any = null;
}
@Directive({selector: '[myIf]'})
export class MyIf {
constructor(templateRef: TemplateRef<MyIfContext>) {}
@Input()
set myIf(condition: any) {}
static myIfUseIfTypeGuard: void;
}
@NgModule({
declarations: [MainComp, MyIf],
})
export class MainModule {}`
});
});

it('should narrow a renamed *ngIf like directive with UseIf', () => {
a({
'src/app.component.ts': '',
'src/lib.ts': '',
'src/app.module.ts': `
import {NgModule, Component, Directive, HostListener, TemplateRef, Input} from '@angular/core';
export interface Person {
name: string;
}
@Component({
selector: 'comp',
template: '<div *my-if="person"> {{person.name}} </div>'
})
export class MainComp {
person?: Person;
}
export class MyIfContext {
public $implicit: any = null;
public myIf: any = null;
}
@Directive({selector: '[my-if]'})
export class MyIf {
constructor(templateRef: TemplateRef<MyIfContext>) {}
@Input('my-if')
set myIf(condition: any) {}
static myIfUseIfTypeGuard: void;
}
@NgModule({
declarations: [MainComp, MyIf],
})
export class MainModule {}`
});
});

it('should narrow a type in a nested *ngIf like directive with UseIf', () => {
a({
'src/app.component.ts': '',
'src/lib.ts': '',
'src/app.module.ts': `
import {NgModule, Component, Directive, HostListener, TemplateRef, Input} from '@angular/core';
export interface Address {
street: string;
}
export interface Person {
name: string;
address?: Address;
}
@Component({
selector: 'comp',
template: '<div *myIf="person"> {{person.name}} <span *myIf="person.address">{{person.address.street}}</span></div>'
})
export class MainComp {
person?: Person;
}
export class MyIfContext {
public $implicit: any = null;
public myIf: any = null;
}
@Directive({selector: '[myIf]'})
export class MyIf {
constructor(templateRef: TemplateRef<MyIfContext>) {}
@Input()
set myIf(condition: any) {}
static myIfUseIfTypeGuard: void;
}
@NgModule({
declarations: [MainComp, MyIf],
})
export class MainModule {}`
});
});

it('should narrow an *ngIf like directive with UseIf and &&', () => {
a({
'src/app.component.ts': '',
'src/lib.ts': '',
'src/app.module.ts': `
import {NgModule, Component, Directive, HostListener, TemplateRef, Input} from '@angular/core';
export interface Address {
street: string;
}
export interface Person {
name: string;
}
@Component({
selector: 'comp',
template: '<div *myIf="person && address"> {{person.name}} lives at {{address.street}} </div>'
})
export class MainComp {
person?: Person;
address?: Address;
}
export class MyIfContext {
public $implicit: any = null;
public myIf: any = null;
}
@Directive({selector: '[myIf]'})
export class MyIf {
constructor(templateRef: TemplateRef<MyIfContext>) {}
@Input()
set myIf(condition: any) {}
static myIfUseIfTypeGuard: void;
}
@NgModule({
declarations: [MainComp, MyIf],
})
export class MainModule {}`
});
});

it('should narrow an *ngIf like directive with UseIf and !!', () => {
a({
'src/app.component.ts': '',
'src/lib.ts': '',
'src/app.module.ts': `
import {NgModule, Component, Directive, HostListener, TemplateRef, Input} from '@angular/core';
export interface Person {
name: string;
}
@Component({
selector: 'comp',
template: '<div *myIf="!!person"> {{person.name}} </div>'
})
export class MainComp {
person?: Person;
}
export class MyIfContext {
public $implicit: any = null;
public myIf: any = null;
}
@Directive({selector: '[myIf]'})
export class MyIf {
constructor(templateRef: TemplateRef<MyIfContext>) {}
@Input()
set myIf(condition: any) {}
static myIfUseIfTypeGuard: void;
}
@NgModule({
declarations: [MainComp, MyIf],
})
export class MainModule {}`
});
});

it('should narrow an *ngIf like directive with UseIf and != null', () => {
a({
'src/app.component.ts': '',
'src/lib.ts': '',
'src/app.module.ts': `
import {NgModule, Component, Directive, HostListener, TemplateRef, Input} from '@angular/core';
export interface Person {
name: string;
}
@Component({
selector: 'comp',
template: '<div *myIf="person != null"> {{person.name}} </div>'
})
export class MainComp {
person: Person | null = null;
}
export class MyIfContext {
public $implicit: any = null;
public myIf: any = null;
}
@Directive({selector: '[myIf]'})
export class MyIf {
constructor(templateRef: TemplateRef<MyIfContext>) {}
@Input()
set myIf(condition: any) {}
static myIfUseIfTypeGuard: void;
}
@NgModule({
declarations: [MainComp, MyIf],
})
export class MainModule {}`
});
});

it('should narrow an *ngIf like directive with UseIf and != undefined', () => {
a({
'src/app.component.ts': '',
'src/lib.ts': '',
'src/app.module.ts': `
import {NgModule, Component, Directive, HostListener, TemplateRef, Input} from '@angular/core';
export interface Person {
name: string;
}
@Component({
selector: 'comp',
template: '<div *myIf="person != undefined"> {{person.name}} </div>'
})
export class MainComp {
person?: Person;
}
export class MyIfContext {
public $implicit: any = null;
public myIf: any = null;
}
@Directive({selector: '[myIf]'})
export class MyIf {
constructor(templateRef: TemplateRef<MyIfContext>) {}
@Input()
set myIf(condition: any) {}
static myIfUseIfTypeGuard: void;
}
@NgModule({
declarations: [MainComp, MyIf],
})
export class MainModule {}`
});
});
}); });


describe('casting $any', () => { describe('casting $any', () => {
Expand Down
14 changes: 12 additions & 2 deletions packages/compiler/src/aot/static_reflector.ts
Expand Up @@ -30,6 +30,7 @@ const USE_VALUE = 'useValue';
const PROVIDE = 'provide'; const PROVIDE = 'provide';
const REFERENCE_SET = new Set([USE_VALUE, 'useFactory', 'data']); const REFERENCE_SET = new Set([USE_VALUE, 'useFactory', 'data']);
const TYPEGUARD_POSTFIX = 'TypeGuard'; const TYPEGUARD_POSTFIX = 'TypeGuard';
const USE_IF = 'UseIf';


function shouldIgnore(value: any): boolean { function shouldIgnore(value: any): boolean {
return value && value.__symbolic == 'ignore'; return value && value.__symbolic == 'ignore';
Expand Down Expand Up @@ -296,8 +297,17 @@ export class StaticReflector implements CompileReflector {
const staticMembers = this._staticMembers(type); const staticMembers = this._staticMembers(type);
const result: {[key: string]: StaticSymbol} = {}; const result: {[key: string]: StaticSymbol} = {};
for (let name of staticMembers) { for (let name of staticMembers) {
result[name.substr(0, name.length - TYPEGUARD_POSTFIX.length)] = if (name.endsWith(TYPEGUARD_POSTFIX)) {
this.getStaticSymbol(type.filePath, type.name, [name]); let property = name.substr(0, name.length - TYPEGUARD_POSTFIX.length);
let value: any;
if (property.endsWith(USE_IF)) {
property = name.substr(0, property.length - USE_IF.length);
value = USE_IF;
} else {
value = this.getStaticSymbol(type.filePath, type.name, [name]);
}
result[property] = value;
}
} }
return result; return result;
} }
Expand Down
10 changes: 8 additions & 2 deletions packages/compiler/src/compiler_util/expression_converter.ts
Expand Up @@ -123,7 +123,7 @@ export function convertPropertyBinding(
return new ConvertPropertyBindingResult([], outputExpr); return new ConvertPropertyBindingResult([], outputExpr);
} }


stmts.push(currValExpr.set(outputExpr).toDeclStmt(null, [o.StmtModifier.Final])); stmts.push(currValExpr.set(outputExpr).toDeclStmt(o.DYNAMIC_TYPE, [o.StmtModifier.Final]));
return new ConvertPropertyBindingResult(stmts, currValExpr); return new ConvertPropertyBindingResult(stmts, currValExpr);
} }


Expand Down Expand Up @@ -334,7 +334,13 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
} }


visitLiteralPrimitive(ast: cdAst.LiteralPrimitive, mode: _Mode): any { visitLiteralPrimitive(ast: cdAst.LiteralPrimitive, mode: _Mode): any {
return convertToStatementIfNeeded(mode, o.literal(ast.value)); // For literal values of null, undefined, true, or false allow type inteference
// to infer the type.
const type =
ast.value === null || ast.value === undefined || ast.value === true || ast.value === true ?
o.INFERRED_TYPE :

This comment has been minimized.

Copy link
@tytskyi

tytskyi Jan 3, 2018

@chuckjaz ast.value === true || ast.value === true one of those supposed to be false?

undefined;
return convertToStatementIfNeeded(mode, o.literal(ast.value, type));
} }


private _getLocal(name: string): o.Expression|null { return this._localResolver.getLocal(name); } private _getLocal(name: string): o.Expression|null { return this._localResolver.getLocal(name); }
Expand Down

0 comments on commit 82bcd83

Please sign in to comment.