Skip to content

Commit

Permalink
fix(ivy): better inference for circularly referenced directive types (#…
Browse files Browse the repository at this point in the history
…35622)

It's possible to pass a directive as an input to itself. Consider:

```html
<some-cmp #ref [value]="ref">
```

Since the template type-checker attempts to infer a type for `<some-cmp>`
using the values of its inputs, this creates a circular reference where the
type of the `value` input is used in its own inference:

```typescript
var _t0 = SomeCmp.ngTypeCtor({value: _t0});
```

Obviously, this doesn't work. To resolve this, the template type-checker
used to generate a `null!` expression when a reference would otherwise be
circular:

```typescript
var _t0 = SomeCmp.ngTypeCtor({value: null!});
```

This effectively asks TypeScript to infer a value for this context, and
works well to resolve this simple cycle. However, if the template
instead tries to use the circular value in a larger expression:

```html
<some-cmp #ref [value]="ref.prop">
```

The checker would generate:

```typescript
var _t0 = SomeCmp.ngTypeCtor({value: (null!).prop});
```

In this case, TypeScript can't figure out any way `null!` could have a
`prop` key, and so it infers `never` as the type. `(never).prop` is thus a
type error.

This commit implements a better fallback pattern for circular references to
directive types like this. Instead of generating a `null!` in place for the
reference, a type is inferred by calling the type constructor again with
`null!` as its input. This infers the widest possible type for the directive
which is then used to break the cycle:

```typescript
var _t0 = SomeCmp.ngTypeCtor(null!);
var _t1 = SomeCmp.ngTypeCtor({value: _t0.prop});
```

This has the desired effect of validating that `.prop` is legal for the
directive type (the type of `#ref`) while also avoiding a cycle.

Fixes #35372
Fixes #35603
Fixes #35522

PR Close #35622
  • Loading branch information
alxhub authored and mhevery committed Feb 26, 2020
1 parent e6c416f commit 4c2bd64
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 7 deletions.
57 changes: 52 additions & 5 deletions packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
Expand Up @@ -98,7 +98,19 @@ export function generateTypeCheckBlock(
* Each `TcbOp` may insert statements into the body of the TCB, and also optionally return a
* `ts.Expression` which can be used to reference the operation's result.
*/
abstract class TcbOp { abstract execute(): ts.Expression|null; }
abstract class TcbOp {
abstract execute(): ts.Expression|null;

/**
* Replacement value or operation used while this `TcbOp` is executing (i.e. to resolve circular
* references during its execution).
*
* This is usually a `null!` expression (which asks TS to infer an appropriate type), but another
* `TcbOp` can be returned in cases where additional code generation is necessary to deal with
* circular references.
*/
circularFallback(): TcbOp|ts.Expression { return INFER_TYPE_FOR_CIRCULAR_OP_EXPR; }
}

/**
* A `TcbOp` which creates an expression for a native DOM element (or web component) from a
Expand Down Expand Up @@ -317,6 +329,41 @@ class TcbDirectiveOp extends TcbOp {
this.scope.addStatement(tsCreateVariable(id, typeCtor));
return id;
}

circularFallback(): TcbOp {
return new TcbDirectiveCircularFallbackOp(this.tcb, this.scope, this.node, this.dir);
}
}

/**
* A `TcbOp` which is used to generate a fallback expression if the inference of a directive type
* via `TcbDirectiveOp` requires a reference to its own type. This can happen using a template
* reference:
*
* ```html
* <some-cmp #ref [prop]="ref.foo"></some-cmp>
* ```
*
* In this case, `TcbDirectiveCircularFallbackOp` will add a second inference of the directive type
* to the type-check block, this time calling the directive's type constructor without any input
* expressions. This infers the widest possible supertype for the directive, which is used to
* resolve any recursive references required to infer the real type.
*/
class TcbDirectiveCircularFallbackOp extends TcbOp {
constructor(
private tcb: Context, private scope: Scope, private node: TmplAstTemplate|TmplAstElement,
private dir: TypeCheckableDirectiveMeta) {
super();
}

execute(): ts.Identifier {
const id = this.tcb.allocateId();
const typeCtor = this.tcb.env.typeCtorFor(this.dir);
const circularPlaceholder = ts.createCall(
typeCtor, /* typeArguments */ undefined, [ts.createNonNullExpression(ts.createNull())]);
this.scope.addStatement(tsCreateVariable(id, circularPlaceholder));
return id;
}
}

/**
Expand Down Expand Up @@ -832,10 +879,10 @@ class Scope {
return op;
}

// Set the result of the operation in the queue to a special expression. If executing this
// operation results in a circular dependency, this will break the cycle and infer the least
// narrow type where needed (which is how TypeScript deals with circular dependencies in types).
this.opQueue[opIndex] = INFER_TYPE_FOR_CIRCULAR_OP_EXPR;
// Set the result of the operation in the queue to its circular fallback. If executing this
// operation results in a circular dependency, this will prevent an infinite loop and allow for
// the resolution of such cycles.
this.opQueue[opIndex] = op.circularFallback();
const res = op.execute();
// Once the operation has finished executing, it's safe to cache the real result.
this.opQueue[opIndex] = res;
Expand Down
Expand Up @@ -180,7 +180,10 @@ 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 _t3 = Dir.ngTypeCtor((null!)); ' +
'var _t2 = Dir.ngTypeCtor({ "input": (_t3) });');
});

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

Expand Down
26 changes: 26 additions & 0 deletions packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
Expand Up @@ -293,6 +293,32 @@ export declare class AnimationEvent {
expect(diags.length).toBe(0);
});

it('should support a directive being used in its own input expression', () => {
env.tsconfig({strictTemplates: true});
env.write('test.ts', `
import {Component, Directive, NgModule, Input} from '@angular/core';
@Component({
selector: 'test',
template: '<target-cmp #ref [foo]="ref.bar"></target-cmp>',
})
export class TestCmp {}
@Component({template: '', selector: 'target-cmp'})
export class TargetCmp {
readonly bar = 'test';
@Input() foo: string;
}
@NgModule({
declarations: [TestCmp, TargetCmp],
})
export class Module {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});

describe('strictInputTypes', () => {
beforeEach(() => {
env.write('test.ts', `
Expand Down

0 comments on commit 4c2bd64

Please sign in to comment.