Skip to content

Commit 5268ae6

Browse files
alxhubbenlesh
authored andcommitted
feat(ivy): support for template type-checking pipe bindings (#29698)
This commit adds support for template type-checking a pipe binding which previously was not handled by the type-checking engine. In compatibility mode, the arguments to transform() are not checked and the type returned by a pipe is 'any'. In full type-checking mode, the transform() method's type signature is used to check the pipe usage and infer the return type of the pipe. Testing strategy: TCB tests included. PR Close #29698
1 parent 98f86de commit 5268ae6

File tree

11 files changed

+206
-25
lines changed

11 files changed

+206
-25
lines changed

packages/compiler-cli/src/ngtsc/annotations/src/component.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,15 @@ export class ComponentDecoratorHandler implements
313313
matcher.addSelectables(CssSelector.parse(meta.selector), extMeta);
314314
}
315315
const bound = new R3TargetBinder(matcher).bind({template: meta.parsedTemplate});
316-
ctx.addTemplate(new Reference(node), bound);
316+
const pipes = new Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>();
317+
for (const {name, ref} of scope.compilation.pipes) {
318+
if (!ts.isClassDeclaration(ref.node)) {
319+
throw new Error(
320+
`Unexpected non-class declaration ${ts.SyntaxKind[ref.node.kind]} for pipe ${ref.debugName}`);
321+
}
322+
pipes.set(name, ref as Reference<ClassDeclaration<ts.ClassDeclaration>>);
323+
}
324+
ctx.addTemplate(new Reference(node), bound, pipes);
317325
}
318326
}
319327

packages/compiler-cli/src/ngtsc/program.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,13 +393,15 @@ export class NgtscProgram implements api.Program {
393393
applyTemplateContextGuards: true,
394394
checkTemplateBodies: true,
395395
checkTypeOfBindings: true,
396+
checkTypeOfPipes: true,
396397
strictSafeNavigationTypes: true,
397398
};
398399
} else {
399400
typeCheckingConfig = {
400401
applyTemplateContextGuards: false,
401402
checkTemplateBodies: false,
402403
checkTypeOfBindings: false,
404+
checkTypeOfPipes: false,
403405
strictSafeNavigationTypes: false,
404406
};
405407
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ export interface TypeCheckBlockMetadata {
3232
* Semantic information about the template of the component.
3333
*/
3434
boundTarget: BoundTarget<TypeCheckableDirectiveMeta>;
35+
36+
/*
37+
* Pipes used in the template of the component.
38+
*/
39+
pipes: Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>;
3540
}
3641

3742
export interface TypeCtorMetadata {
@@ -62,6 +67,16 @@ export interface TypeCheckingConfig {
6267
*/
6368
checkTypeOfBindings: boolean;
6469

70+
/**
71+
* Whether to include type information from pipes in the type-checking operation.
72+
*
73+
* If this is `true`, then the pipe's type signature for `transform()` will be used to check the
74+
* usage of the pipe. If this is `false`, then the result of applying a pipe will be `any`, and
75+
* the types of the pipe's value and arguments will not be matched against the `transform()`
76+
* method.
77+
*/
78+
checkTypeOfPipes: boolean;
79+
6580
/**
6681
* Whether to narrow the types of template contexts.
6782
*/

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ export class TypeCheckContext {
6161
*/
6262
addTemplate(
6363
ref: Reference<ClassDeclaration<ts.ClassDeclaration>>,
64-
boundTarget: BoundTarget<TypeCheckableDirectiveMeta>): void {
64+
boundTarget: BoundTarget<TypeCheckableDirectiveMeta>,
65+
pipes: Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>): void {
6566
// Get all of the directives used in the template and record type constructors for all of them.
6667
for (const dir of boundTarget.getUsedDirectives()) {
6768
const dirRef = dir.ref as Reference<ClassDeclaration<ts.ClassDeclaration>>;
@@ -86,10 +87,10 @@ export class TypeCheckContext {
8687
if (requiresInlineTypeCheckBlock(ref.node)) {
8788
// This class didn't meet the requirements for external type checking, so generate an inline
8889
// TCB for the class.
89-
this.addInlineTypeCheckBlock(ref, {boundTarget});
90+
this.addInlineTypeCheckBlock(ref, {boundTarget, pipes});
9091
} else {
9192
// The class can be type-checked externally as normal.
92-
this.typeCheckFile.addTypeCheckBlock(ref, {boundTarget});
93+
this.typeCheckFile.addTypeCheckBlock(ref, {boundTarget, pipes});
9394
}
9495
}
9596

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {ClassDeclaration} from '../../reflection';
1414
import {ImportManager, translateExpression, translateType} from '../../translator';
1515

1616
import {TypeCheckableDirectiveMeta, TypeCheckingConfig, TypeCtorMetadata} from './api';
17+
import {tsDeclareVariable} from './ts_util';
1718
import {generateTypeCtorDeclarationFn, requiresInlineTypeCtor} from './type_constructor';
1819

1920
/**
@@ -29,12 +30,16 @@ import {generateTypeCtorDeclarationFn, requiresInlineTypeCtor} from './type_cons
2930
*/
3031
export class Environment {
3132
private nextIds = {
33+
pipeInst: 1,
3234
typeCtor: 1,
3335
};
3436

3537
private typeCtors = new Map<ClassDeclaration, ts.Expression>();
3638
protected typeCtorStatements: ts.Statement[] = [];
3739

40+
private pipeInsts = new Map<ClassDeclaration, ts.Expression>();
41+
protected pipeInstStatements: ts.Statement[] = [];
42+
3843
constructor(
3944
readonly config: TypeCheckingConfig, protected importManager: ImportManager,
4045
private refEmitter: ReferenceEmitter, protected contextFile: ts.SourceFile) {}
@@ -83,6 +88,23 @@ export class Environment {
8388
}
8489
}
8590

91+
/*
92+
* Get an expression referring to an instance of the given pipe.
93+
*/
94+
pipeInst(ref: Reference<ClassDeclaration<ts.ClassDeclaration>>): ts.Expression {
95+
if (this.pipeInsts.has(ref.node)) {
96+
return this.pipeInsts.get(ref.node) !;
97+
}
98+
99+
const pipeType = this.referenceType(ref);
100+
const pipeInstId = ts.createIdentifier(`_pipe${this.nextIds.pipeInst++}`);
101+
102+
this.pipeInstStatements.push(tsDeclareVariable(pipeInstId, pipeType));
103+
this.pipeInsts.set(ref.node, pipeInstId);
104+
105+
return pipeInstId;
106+
}
107+
86108
/**
87109
* Generate a `ts.Expression` that references the given node.
88110
*
@@ -131,6 +153,7 @@ export class Environment {
131153

132154
getPreludeStatements(): ts.Statement[] {
133155
return [
156+
...this.pipeInstStatements,
134157
...this.typeCtorStatements,
135158
];
136159
}

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {AST, BindingType, BoundTarget, ImplicitReceiver, PropertyRead, TmplAstBoundAttribute, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
9+
import {AST, BindingPipe, BindingType, BoundTarget, ImplicitReceiver, PropertyRead, TmplAstBoundAttribute, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
1010
import * as ts from 'typescript';
1111

1212
import {Reference} from '../../imports';
@@ -17,6 +17,7 @@ import {Environment} from './environment';
1717
import {astToTypescript} from './expression';
1818
import {checkIfClassIsExported, checkIfGenericTypesAreUnbound, tsCallMethod, tsCastToAny, tsCreateElement, tsCreateVariable, tsDeclareVariable} from './ts_util';
1919

20+
2021
/**
2122
* Given a `ts.ClassDeclaration` for a component, and metadata regarding that component, compose a
2223
* "type check block" function.
@@ -31,7 +32,7 @@ import {checkIfClassIsExported, checkIfGenericTypesAreUnbound, tsCallMethod, tsC
3132
export function generateTypeCheckBlock(
3233
env: Environment, ref: Reference<ClassDeclaration<ts.ClassDeclaration>>, name: ts.Identifier,
3334
meta: TypeCheckBlockMetadata): ts.FunctionDeclaration {
34-
const tcb = new Context(env, meta.boundTarget);
35+
const tcb = new Context(env, meta.boundTarget, meta.pipes);
3536
const scope = Scope.forNodes(tcb, null, tcb.boundTarget.target.template !);
3637
const ctxRawType = env.referenceType(ref);
3738
if (!ts.isTypeReferenceNode(ctxRawType)) {
@@ -355,7 +356,8 @@ export class Context {
355356
private nextId = 1;
356357

357358
constructor(
358-
readonly env: Environment, readonly boundTarget: BoundTarget<TypeCheckableDirectiveMeta>) {}
359+
readonly env: Environment, readonly boundTarget: BoundTarget<TypeCheckableDirectiveMeta>,
360+
private pipes: Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>) {}
359361

360362
/**
361363
* Allocate a new variable name for use within the `Context`.
@@ -364,6 +366,13 @@ export class Context {
364366
* might change depending on the type of data being stored.
365367
*/
366368
allocateId(): ts.Identifier { return ts.createIdentifier(`_t${this.nextId++}`); }
369+
370+
getPipeByName(name: string): ts.Expression {
371+
if (!this.pipes.has(name)) {
372+
throw new Error(`Missing pipe: ${name}`);
373+
}
374+
return this.env.pipeInst(this.pipes.get(name) !);
375+
}
367376
}
368377

369378
/**
@@ -793,6 +802,17 @@ function tcbResolve(ast: AST, tcb: Context, scope: Scope): ts.Expression|null {
793802
// PropertyRead resolved to a variable or reference, and therefore this is a property read on
794803
// the component context itself.
795804
return ts.createIdentifier('ctx');
805+
} else if (ast instanceof BindingPipe) {
806+
const expr = tcbExpression(ast.exp, tcb, scope);
807+
let pipe: ts.Expression;
808+
if (tcb.env.config.checkTypeOfPipes) {
809+
pipe = tcb.getPipeByName(ast.name);
810+
} else {
811+
pipe = ts.createParen(ts.createAsExpression(
812+
ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)));
813+
}
814+
const args = ast.args.map(arg => tcbExpression(arg, tcb, scope));
815+
return tsCallMethod(pipe, 'transform', [expr, ...args]);
796816
} else {
797817
// This AST isn't special after all.
798818
return null;

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ export class TypeCheckFile extends Environment {
5151
'\n\n';
5252
const printer = ts.createPrinter();
5353
source += '\n';
54+
for (const stmt of this.pipeInstStatements) {
55+
source += printer.printNode(ts.EmitHint.Unspecified, stmt, this.contextFile) + '\n';
56+
}
5457
for (const stmt of this.typeCtorStatements) {
5558
source += printer.printNode(ts.EmitHint.Unspecified, stmt, this.contextFile) + '\n';
5659
}

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

Lines changed: 65 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ describe('type check blocks', () => {
5353
{{d.value}}
5454
<div dir #d="dir"></div>
5555
`;
56-
const DIRECTIVES: TestDirective[] = [{
56+
const DIRECTIVES: TestDeclaration[] = [{
57+
type: 'directive',
5758
name: 'Dir',
5859
selector: '[dir]',
5960
exportAs: ['dir'],
@@ -76,7 +77,8 @@ describe('type check blocks', () => {
7677
});
7778

7879
describe('config', () => {
79-
const DIRECTIVES: TestDirective[] = [{
80+
const DIRECTIVES: TestDeclaration[] = [{
81+
type: 'directive',
8082
name: 'Dir',
8183
selector: '[dir]',
8284
exportAs: ['dir'],
@@ -87,6 +89,7 @@ describe('type check blocks', () => {
8789
applyTemplateContextGuards: true,
8890
checkTemplateBodies: true,
8991
checkTypeOfBindings: true,
92+
checkTypeOfPipes: true,
9093
strictSafeNavigationTypes: true,
9194
};
9295

@@ -135,6 +138,26 @@ describe('type check blocks', () => {
135138
});
136139
});
137140

141+
142+
describe('config.checkTypeOfPipes', () => {
143+
const TEMPLATE = `{{a | test:b:c}}`;
144+
const PIPES: TestDeclaration[] = [{
145+
type: 'pipe',
146+
name: 'TestPipe',
147+
pipeName: 'test',
148+
}];
149+
150+
it('should check types of pipes when enabled', () => {
151+
const block = tcb(TEMPLATE, PIPES);
152+
expect(block).toContain('(null as TestPipe).transform(ctx.a, ctx.b, ctx.c);');
153+
});
154+
it('should not check types of pipes when disabled', () => {
155+
const DISABLED_CONFIG = {...BASE_CONFIG, checkTypeOfPipes: false};
156+
const block = tcb(TEMPLATE, PIPES, DISABLED_CONFIG);
157+
expect(block).toContain('(null as any).transform(ctx.a, ctx.b, ctx.c);');
158+
});
159+
});
160+
138161
describe('config.strictSafeNavigationTypes', () => {
139162
const TEMPLATE = `{{a?.b}} {{a?.method()}}`;
140163

@@ -158,6 +181,7 @@ it('should generate a circular directive reference correctly', () => {
158181
<div dir #d="dir" [input]="d"></div>
159182
`;
160183
const DIRECTIVES: TestDirective[] = [{
184+
type: 'directive',
161185
name: 'Dir',
162186
selector: '[dir]',
163187
exportAs: ['dir'],
@@ -173,12 +197,14 @@ it('should generate circular references between two directives correctly', () =>
173197
`;
174198
const DIRECTIVES: TestDirective[] = [
175199
{
200+
type: 'directive',
176201
name: 'DirA',
177202
selector: '[dir-a]',
178203
exportAs: ['dirA'],
179204
inputs: {inputA: 'inputA'},
180205
},
181206
{
207+
type: 'directive',
182208
name: 'DirB',
183209
selector: '[dir-b]',
184210
exportAs: ['dirB'],
@@ -203,42 +229,60 @@ function getClass(sf: ts.SourceFile, name: string): ClassDeclaration<ts.ClassDec
203229
// Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead.
204230
type TestDirective =
205231
Partial<Pick<TypeCheckableDirectiveMeta, Exclude<keyof TypeCheckableDirectiveMeta, 'ref'>>>&
206-
{selector: string, name: string};
232+
{selector: string, name: string, type: 'directive'};
233+
type TestPipe = {
234+
name: string,
235+
pipeName: string,
236+
type: 'pipe',
237+
};
238+
239+
type TestDeclaration = TestDirective | TestPipe;
207240

208241
function tcb(
209-
template: string, directives: TestDirective[] = [], config?: TypeCheckingConfig): string {
210-
const classes = ['Test', ...directives.map(dir => dir.name)];
242+
template: string, declarations: TestDeclaration[] = [], config?: TypeCheckingConfig): string {
243+
const classes = ['Test', ...declarations.map(decl => decl.name)];
211244
const code = classes.map(name => `class ${name}<T extends string> {}`).join('\n');
212245

213246
const sf = ts.createSourceFile('synthetic.ts', code, ts.ScriptTarget.Latest, true);
214247
const clazz = getClass(sf, 'Test');
215248
const {nodes} = parseTemplate(template, 'synthetic.html');
216249
const matcher = new SelectorMatcher();
217250

218-
for (const dir of directives) {
219-
const selector = CssSelector.parse(dir.selector);
251+
for (const decl of declarations) {
252+
if (decl.type !== 'directive') {
253+
continue;
254+
}
255+
const selector = CssSelector.parse(decl.selector);
220256
const meta: TypeCheckableDirectiveMeta = {
221-
name: dir.name,
222-
ref: new Reference(getClass(sf, dir.name)),
223-
exportAs: dir.exportAs || null,
224-
hasNgTemplateContextGuard: dir.hasNgTemplateContextGuard || false,
225-
inputs: dir.inputs || {},
226-
isComponent: dir.isComponent || false,
227-
ngTemplateGuards: dir.ngTemplateGuards || [],
228-
outputs: dir.outputs || {},
229-
queries: dir.queries || [],
257+
name: decl.name,
258+
ref: new Reference(getClass(sf, decl.name)),
259+
exportAs: decl.exportAs || null,
260+
hasNgTemplateContextGuard: decl.hasNgTemplateContextGuard || false,
261+
inputs: decl.inputs || {},
262+
isComponent: decl.isComponent || false,
263+
ngTemplateGuards: decl.ngTemplateGuards || [],
264+
outputs: decl.outputs || {},
265+
queries: decl.queries || [],
230266
};
231267
matcher.addSelectables(selector, meta);
232268
}
233269

234270
const binder = new R3TargetBinder(matcher);
235271
const boundTarget = binder.bind({template: nodes});
236272

237-
const meta: TypeCheckBlockMetadata = {boundTarget};
273+
const pipes = new Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>();
274+
for (const decl of declarations) {
275+
if (decl.type === 'pipe') {
276+
pipes.set(decl.pipeName, new Reference(getClass(sf, decl.name)));
277+
}
278+
}
279+
280+
const meta: TypeCheckBlockMetadata = {boundTarget, pipes};
238281

239282
config = config || {
240283
applyTemplateContextGuards: true,
241284
checkTypeOfBindings: true,
285+
checkTypeOfPipes: true,
242286
checkTemplateBodies: true,
243287
strictSafeNavigationTypes: true,
244288
};
@@ -257,6 +301,10 @@ class FakeEnvironment /* implements Environment */ {
257301
return ts.createPropertyAccess(ts.createIdentifier(dir.name), 'ngTypeCtor');
258302
}
259303

304+
pipeInst(ref: Reference<ClassDeclaration<ts.ClassDeclaration>>): ts.Expression {
305+
return ts.createParen(ts.createAsExpression(ts.createNull(), this.referenceType(ref)));
306+
}
307+
260308
reference(ref: Reference<ClassDeclaration<ts.ClassDeclaration>>): ts.Expression {
261309
return ref.node.name;
262310
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ const ALL_ENABLED_CONFIG: TypeCheckingConfig = {
2929
applyTemplateContextGuards: true,
3030
checkTemplateBodies: true,
3131
checkTypeOfBindings: true,
32+
checkTypeOfPipes: true,
3233
strictSafeNavigationTypes: true,
3334
};
3435

0 commit comments

Comments
 (0)