diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts index 0815589d850..5e942e9bdb0 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts @@ -573,7 +573,7 @@ runInEachFileSystem(() => { }); it('should retrieve a symbol for the track expression', () => { - const userSymbol = templateTypeChecker.getSymbolOfNode(forLoopNode.trackBy.ast, cmp)!; + const userSymbol = templateTypeChecker.getSymbolOfNode(forLoopNode.trackBy!.ast, cmp)!; expectUserSymbol(userSymbol); }); diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index fe4860f9c99..44addc76d16 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -6573,6 +6573,32 @@ suppress `Type 'number' is not assignable to type 'string'.`, ]); }); + + it('should type check a @for loop without a `track` expression', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: \` + @for (item of items) { + {{does_not_exist}} + } + \`, + }) + export class Main { + items = []; + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.map((d) => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([ + `Property 'does_not_exist' does not exist on type 'Main'.`, + `@for loop must have a "track" expression`, + ]); + }); }); describe('control flow content projection diagnostics', () => { diff --git a/packages/compiler/src/render3/r3_ast.ts b/packages/compiler/src/render3/r3_ast.ts index bd6cb2988d9..8c7fa4a7909 100644 --- a/packages/compiler/src/render3/r3_ast.ts +++ b/packages/compiler/src/render3/r3_ast.ts @@ -489,8 +489,8 @@ export class ForLoopBlock extends BlockNode implements Node { constructor( public item: Variable, public expression: ASTWithSource, - public trackBy: ASTWithSource, - public trackKeywordSpan: ParseSourceSpan, + public trackBy: ASTWithSource | null, + public trackKeywordSpan: ParseSourceSpan | null, public contextVariables: Variable[], public children: Node[], public empty: ForLoopBlockEmpty | null, diff --git a/packages/compiler/src/render3/r3_control_flow.ts b/packages/compiler/src/render3/r3_control_flow.ts index 0f37877ffb0..98ea387c7e3 100644 --- a/packages/compiler/src/render3/r3_control_flow.ts +++ b/packages/compiler/src/render3/r3_control_flow.ts @@ -183,35 +183,40 @@ export function createForLoop( } if (params !== null) { + // The `for` block has a main span that includes the `empty` branch. For only the span of the + // main `for` body, use `mainSourceSpan`. + const endSpan = empty?.endSourceSpan ?? ast.endSourceSpan; + const sourceSpan = new ParseSourceSpan( + ast.sourceSpan.start, + endSpan?.end ?? ast.sourceSpan.end, + ); + let trackExpression: ASTWithSource | null; + let trackKeywordSpan: ParseSourceSpan | null; + if (params.trackBy === null) { - // TODO: We should not fail here, and instead try to produce some AST for the language - // service. + trackExpression = trackKeywordSpan = null; errors.push(new ParseError(ast.startSourceSpan, '@for loop must have a "track" expression')); } else { - // The `for` block has a main span that includes the `empty` branch. For only the span of the - // main `for` body, use `mainSourceSpan`. - const endSpan = empty?.endSourceSpan ?? ast.endSourceSpan; - const sourceSpan = new ParseSourceSpan( - ast.sourceSpan.start, - endSpan?.end ?? ast.sourceSpan.end, - ); + trackExpression = params.trackBy.expression; + trackKeywordSpan = params.trackBy.keywordSpan; validateTrackByExpression(params.trackBy.expression, params.trackBy.keywordSpan, errors); - node = new t.ForLoopBlock( - params.itemName, - params.expression, - params.trackBy.expression, - params.trackBy.keywordSpan, - params.context, - html.visitAll(visitor, ast.children, ast.children), - empty, - sourceSpan, - ast.sourceSpan, - ast.startSourceSpan, - endSpan, - ast.nameSpan, - ast.i18n, - ); } + + node = new t.ForLoopBlock( + params.itemName, + params.expression, + trackExpression, + trackKeywordSpan, + params.context, + html.visitAll(visitor, ast.children, ast.children), + empty, + sourceSpan, + ast.sourceSpan, + ast.startSourceSpan, + endSpan, + ast.nameSpan, + ast.i18n, + ); } return {node, errors}; diff --git a/packages/compiler/src/render3/view/t2_binder.ts b/packages/compiler/src/render3/view/t2_binder.ts index b6eb3a18bb3..38d23acefdb 100644 --- a/packages/compiler/src/render3/view/t2_binder.ts +++ b/packages/compiler/src/render3/view/t2_binder.ts @@ -994,7 +994,7 @@ class TemplateBinder extends CombinedRecursiveAstVisitor { } else if (nodeOrNodes instanceof ForLoopBlock) { this.visitNode(nodeOrNodes.item); nodeOrNodes.contextVariables.forEach((v) => this.visitNode(v)); - nodeOrNodes.trackBy.visit(this); + nodeOrNodes.trackBy?.visit(this); nodeOrNodes.children.forEach(this.visitNode); this.nestingLevel.set(nodeOrNodes, this.level); } else if (nodeOrNodes instanceof DeferredBlock) { diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index 74ab2a5e179..f4cb0d9f1ed 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -935,8 +935,19 @@ function ingestForBlock(unit: ViewCompilationUnit, forBlock: t.ForLoopBlock): vo } } - const sourceSpan = convertSourceSpan(forBlock.trackBy.span, forBlock.sourceSpan); - const track = convertAst(forBlock.trackBy, unit.job, sourceSpan); + let track: o.Expression; + + if (forBlock.trackBy === null) { + // `@for` without a `track` is invalid and it produces a parser error. + // Put a placeholder here so we don't need to account for it throughout the pipeline. + track = o.variable('$index'); + } else { + track = convertAst( + forBlock.trackBy, + unit.job, + convertSourceSpan(forBlock.trackBy.span, forBlock.sourceSpan), + ); + } ingestNodes(repeaterView, forBlock.children); diff --git a/packages/compiler/src/typecheck/ops/for_block.ts b/packages/compiler/src/typecheck/ops/for_block.ts index 34b9ae52772..b5850f103c9 100644 --- a/packages/compiler/src/typecheck/ops/for_block.ts +++ b/packages/compiler/src/typecheck/ops/for_block.ts @@ -48,11 +48,21 @@ export class TcbForOfOp extends TcbOp { const expression = new TcbExpr( `${tcbExpression(this.block.expression, this.tcb, this.scope).print()}!`, ); - const trackTranslator = new TcbForLoopTrackTranslator(this.tcb, loopScope, this.block); - const trackExpression = trackTranslator.translate(this.block.trackBy); - const block = getStatementsBlock([...loopScope.render(), trackExpression]); + + let statements: TcbExpr[]; + + if (this.block.trackBy === null) { + statements = loopScope.render(); + } else { + const trackTranslator = new TcbForLoopTrackTranslator(this.tcb, loopScope, this.block); + const trackExpression = trackTranslator.translate(this.block.trackBy); + statements = [...loopScope.render(), trackExpression]; + } + this.scope.addStatement( - new TcbExpr(`for (${initializer.print()} of ${expression.print()}) {\n${block} }`), + new TcbExpr( + `for (${initializer.print()} of ${expression.print()}) {\n${getStatementsBlock(statements)} }`, + ), ); return null; } diff --git a/packages/compiler/test/render3/r3_template_transform_spec.ts b/packages/compiler/test/render3/r3_template_transform_spec.ts index 4f969463059..b30b5ee1023 100644 --- a/packages/compiler/test/render3/r3_template_transform_spec.ts +++ b/packages/compiler/test/render3/r3_template_transform_spec.ts @@ -119,7 +119,10 @@ class R3AstHumanizer implements t.Visitor { } visitForLoopBlock(block: t.ForLoopBlock): void { - const result: any[] = ['ForLoopBlock', unparse(block.expression), unparse(block.trackBy)]; + const result: any[] = ['ForLoopBlock', unparse(block.expression)]; + if (block.trackBy !== null) { + result.push(unparse(block.trackBy)); + } this.result.push(result); this.visitAll([[block.item], block.contextVariables, block.children]); block.empty?.visit(this); diff --git a/packages/core/schematics/migrations/safe-optional-chaining/migration.ts b/packages/core/schematics/migrations/safe-optional-chaining/migration.ts index 1a72bf5975e..b116a5e1132 100644 --- a/packages/core/schematics/migrations/safe-optional-chaining/migration.ts +++ b/packages/core/schematics/migrations/safe-optional-chaining/migration.ts @@ -703,7 +703,7 @@ class TmplVisitor extends TmplAstRecursiveVisitor { override visitForLoopBlock(block: TmplAstForLoopBlock) { block.expression.visit(this.exprMigrator, false); - block.trackBy.visit(this.exprMigrator, false); + block.trackBy?.visit(this.exprMigrator, false); super.visitForLoopBlock(block); } diff --git a/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/template_reference_visitor.ts b/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/template_reference_visitor.ts index c22b6d2c0f2..e33e71658ba 100644 --- a/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/template_reference_visitor.ts +++ b/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/template_reference_visitor.ts @@ -175,7 +175,11 @@ export class TemplateReferenceVisitor< override visitForLoopBlock(block: TmplAstForLoopBlock): void { this.checkExpressionForReferencedFields(block, block.expression); - this.checkExpressionForReferencedFields(block, block.trackBy); + + if (block.trackBy !== null) { + this.checkExpressionForReferencedFields(block, block.trackBy); + } + super.visitForLoopBlock(block); } diff --git a/packages/language-service/src/quick_info_built_ins.ts b/packages/language-service/src/quick_info_built_ins.ts index 4da50627212..9996b6b1e66 100644 --- a/packages/language-service/src/quick_info_built_ins.ts +++ b/packages/language-service/src/quick_info_built_ins.ts @@ -127,6 +127,7 @@ export function createQuickInfoForBuiltIn( partSpan = node.nameSpan; } else if ( node instanceof TmplAstForLoopBlock && + node.trackKeywordSpan !== null && isWithin(cursorPositionInTemplate, node.trackKeywordSpan) ) { partSpan = node.trackKeywordSpan; diff --git a/packages/language-service/src/template_target.ts b/packages/language-service/src/template_target.ts index f4fffd8eb6f..17282274f64 100644 --- a/packages/language-service/src/template_target.ts +++ b/packages/language-service/src/template_target.ts @@ -679,7 +679,7 @@ class TemplateTargetVisitor implements TmplAstVisitor { this.visit(block.item); this.visitAll(block.contextVariables); this.visitBinding(block.expression); - this.visitBinding(block.trackBy); + block.trackBy && this.visitBinding(block.trackBy); this.visitAll(block.children); block.empty && this.visit(block.empty); }