Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ivy): better inference for circularly referenced directive types #35622

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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