Skip to content

Commit 173a1ac

Browse files
alxhubmhevery
authored andcommitted
fix(ivy): better inference for circularly referenced directive types (angular#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 angular#35372 Fixes angular#35603 Fixes angular#35522 PR Close angular#35622
1 parent 2d89b5d commit 173a1ac

File tree

3 files changed

+84
-7
lines changed

3 files changed

+84
-7
lines changed

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,19 @@ export function generateTypeCheckBlock(
9898
* Each `TcbOp` may insert statements into the body of the TCB, and also optionally return a
9999
* `ts.Expression` which can be used to reference the operation's result.
100100
*/
101-
abstract class TcbOp { abstract execute(): ts.Expression|null; }
101+
abstract class TcbOp {
102+
abstract execute(): ts.Expression|null;
103+
104+
/**
105+
* Replacement value or operation used while this `TcbOp` is executing (i.e. to resolve circular
106+
* references during its execution).
107+
*
108+
* This is usually a `null!` expression (which asks TS to infer an appropriate type), but another
109+
* `TcbOp` can be returned in cases where additional code generation is necessary to deal with
110+
* circular references.
111+
*/
112+
circularFallback(): TcbOp|ts.Expression { return INFER_TYPE_FOR_CIRCULAR_OP_EXPR; }
113+
}
102114

103115
/**
104116
* A `TcbOp` which creates an expression for a native DOM element (or web component) from a
@@ -317,6 +329,41 @@ class TcbDirectiveOp extends TcbOp {
317329
this.scope.addStatement(tsCreateVariable(id, typeCtor));
318330
return id;
319331
}
332+
333+
circularFallback(): TcbOp {
334+
return new TcbDirectiveCircularFallbackOp(this.tcb, this.scope, this.node, this.dir);
335+
}
336+
}
337+
338+
/**
339+
* A `TcbOp` which is used to generate a fallback expression if the inference of a directive type
340+
* via `TcbDirectiveOp` requires a reference to its own type. This can happen using a template
341+
* reference:
342+
*
343+
* ```html
344+
* <some-cmp #ref [prop]="ref.foo"></some-cmp>
345+
* ```
346+
*
347+
* In this case, `TcbDirectiveCircularFallbackOp` will add a second inference of the directive type
348+
* to the type-check block, this time calling the directive's type constructor without any input
349+
* expressions. This infers the widest possible supertype for the directive, which is used to
350+
* resolve any recursive references required to infer the real type.
351+
*/
352+
class TcbDirectiveCircularFallbackOp extends TcbOp {
353+
constructor(
354+
private tcb: Context, private scope: Scope, private node: TmplAstTemplate|TmplAstElement,
355+
private dir: TypeCheckableDirectiveMeta) {
356+
super();
357+
}
358+
359+
execute(): ts.Identifier {
360+
const id = this.tcb.allocateId();
361+
const typeCtor = this.tcb.env.typeCtorFor(this.dir);
362+
const circularPlaceholder = ts.createCall(
363+
typeCtor, /* typeArguments */ undefined, [ts.createNonNullExpression(ts.createNull())]);
364+
this.scope.addStatement(tsCreateVariable(id, circularPlaceholder));
365+
return id;
366+
}
320367
}
321368

322369
/**
@@ -832,10 +879,10 @@ class Scope {
832879
return op;
833880
}
834881

835-
// Set the result of the operation in the queue to a special expression. If executing this
836-
// operation results in a circular dependency, this will break the cycle and infer the least
837-
// narrow type where needed (which is how TypeScript deals with circular dependencies in types).
838-
this.opQueue[opIndex] = INFER_TYPE_FOR_CIRCULAR_OP_EXPR;
882+
// Set the result of the operation in the queue to its circular fallback. If executing this
883+
// operation results in a circular dependency, this will prevent an infinite loop and allow for
884+
// the resolution of such cycles.
885+
this.opQueue[opIndex] = op.circularFallback();
839886
const res = op.execute();
840887
// Once the operation has finished executing, it's safe to cache the real result.
841888
this.opQueue[opIndex] = res;

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,10 @@ describe('type check blocks', () => {
180180
exportAs: ['dir'],
181181
inputs: {input: 'input'},
182182
}];
183-
expect(tcb(TEMPLATE, DIRECTIVES)).toContain('var _t2 = Dir.ngTypeCtor({ "input": (null!) });');
183+
expect(tcb(TEMPLATE, DIRECTIVES))
184+
.toContain(
185+
'var _t3 = Dir.ngTypeCtor((null!)); ' +
186+
'var _t2 = Dir.ngTypeCtor({ "input": (_t3) });');
184187
});
185188

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

packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,32 @@ export declare class AnimationEvent {
293293
expect(diags.length).toBe(0);
294294
});
295295

296+
it('should support a directive being used in its own input expression', () => {
297+
env.tsconfig({strictTemplates: true});
298+
env.write('test.ts', `
299+
import {Component, Directive, NgModule, Input} from '@angular/core';
300+
301+
@Component({
302+
selector: 'test',
303+
template: '<target-cmp #ref [foo]="ref.bar"></target-cmp>',
304+
})
305+
export class TestCmp {}
306+
307+
@Component({template: '', selector: 'target-cmp'})
308+
export class TargetCmp {
309+
readonly bar = 'test';
310+
@Input() foo: string;
311+
}
312+
313+
@NgModule({
314+
declarations: [TestCmp, TargetCmp],
315+
})
316+
export class Module {}
317+
`);
318+
const diags = env.driveDiagnostics();
319+
expect(diags.length).toBe(0);
320+
});
321+
296322
describe('strictInputTypes', () => {
297323
beforeEach(() => {
298324
env.write('test.ts', `

0 commit comments

Comments
 (0)