From eb625d37839c3b9f20a2ffb3af06426f9910c8ac Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 19 Mar 2024 13:35:48 +0100 Subject: [PATCH] fix(compiler): declare for loop aliases in addition to new name (#54942) Currently when aliasing a `for` loop variable with `let`, we replace the variable's old name with the new one. Since users have found this to be confusing, these changes switch to a model where the variable is available both under the original name and the new one. Fixes #52528. PR Close #54942 --- .../src/ngtsc/indexer/src/template.ts | 2 +- .../src/ngtsc/typecheck/extended/api/api.ts | 2 +- .../src/ngtsc/typecheck/src/oob.ts | 8 +- .../ngtsc/typecheck/src/type_check_block.ts | 25 ++-- .../typecheck/test/type_check_block_spec.ts | 5 +- ...ecker__get_symbol_of_template_node_spec.ts | 14 ++- .../GOLDEN_PARTIAL.js | 69 +++++++++++ .../TEST_CASES.json | 15 +++ ...ed_template_variables_template.pipeline.js | 8 +- ...for_both_aliased_and_original_variables.ts | 28 +++++ ...aliased_and_original_variables_template.js | 31 +++++ ...te_variables_listener_template.pipeline.js | 2 +- ...plate_variables_scope_template.pipeline.js | 2 +- ...or_template_variables_template.pipeline.js | 2 +- .../test/ngtsc/template_typecheck_spec.ts | 32 +++--- packages/compiler/src/render3/r3_ast.ts | 10 +- .../compiler/src/render3/r3_control_flow.ts | 36 +++--- .../compiler/src/render3/view/t2_binder.ts | 6 +- .../template/pipeline/ir/src/ops/create.ts | 7 +- .../src/template/pipeline/src/ingest.ts | 107 ++++++++++-------- .../pipeline/src/phases/track_variables.ts | 2 +- .../test/render3/r3_ast_spans_spec.ts | 8 +- .../render3/r3_template_transform_spec.ts | 73 +++++++++++- .../compiler/test/render3/util/expression.ts | 2 +- .../test/acceptance/control_flow_for_spec.ts | 34 ++++-- .../language-service/src/template_target.ts | 2 +- 26 files changed, 389 insertions(+), 143 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_both_aliased_and_original_variables.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_both_aliased_and_original_variables_template.js diff --git a/packages/compiler-cli/src/ngtsc/indexer/src/template.ts b/packages/compiler-cli/src/ngtsc/indexer/src/template.ts index df723c830aa43..7e224cc3353d9 100644 --- a/packages/compiler-cli/src/ngtsc/indexer/src/template.ts +++ b/packages/compiler-cli/src/ngtsc/indexer/src/template.ts @@ -271,7 +271,7 @@ class TemplateVisitor extends TmplAstRecursiveVisitor { override visitForLoopBlock(block: TmplAstForLoopBlock): void { block.item.visit(this); - this.visitAll(Object.values(block.contextVariables)); + this.visitAll(block.contextVariables); this.visitExpression(block.expression); this.visitAll(block.children); block.empty?.visit(this); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/api/api.ts index 3962b89949434..5e74553fc3f8a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/api/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/api/api.ts @@ -195,7 +195,7 @@ class TemplateVisitor extends RecursiveAstVisitor implem visitForLoopBlock(block: TmplAstForLoopBlock): void { block.item.visit(this); - this.visitAllNodes(Object.values(block.contextVariables)); + this.visitAllNodes(block.contextVariables); this.visitAst(block.expression); this.visitAllNodes(block.children); block.empty?.visit(this); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts index f8862501949bd..686c29da94683 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts @@ -353,11 +353,13 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor throw new Error(`Assertion failure: no SourceLocation found for property read.`); } + const messageVars = [block.item, ...block.contextVariables.filter(v => v.value === '$index')] + .map(v => `'${v.name}'`) + .join(', '); const message = `Cannot access '${access.name}' inside of a track expression. ` + - `Only '${block.item.name}', '${ - block.contextVariables.$index - .name}' and properties on the containing component are available to this expression.`; + `Only ${ + messageVars} and properties on the containing component are available to this expression.`; this._diagnostics.push(makeTemplateDiagnostic( templateId, this.resolver.getSourceMapping(templateId), sourceSpan, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index 6c2f236a8c969..528fa32cb8a76 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -1816,12 +1816,13 @@ class Scope { addParseSpanInfo(loopInitializer, scopedNode.item.sourceSpan); scope.varMap.set(scopedNode.item, loopInitializer); - for (const [name, variable] of Object.entries(scopedNode.contextVariables)) { - if (!this.forLoopContextVariableTypes.has(name)) { - throw new Error(`Unrecognized for loop context variable ${name}`); + for (const variable of scopedNode.contextVariables) { + if (!this.forLoopContextVariableTypes.has(variable.value)) { + throw new Error(`Unrecognized for loop context variable ${variable.name}`); } - const type = ts.factory.createKeywordTypeNode(this.forLoopContextVariableTypes.get(name)!); + const type = + ts.factory.createKeywordTypeNode(this.forLoopContextVariableTypes.get(variable.value)!); this.registerVariable( scope, variable, new TcbBlockImplicitVariableOp(tcb, scope, type, variable)); } @@ -2761,18 +2762,26 @@ class TcbEventHandlerTranslator extends TcbExpressionTranslator { } class TcbForLoopTrackTranslator extends TcbExpressionTranslator { + private allowedVariables: Set; + constructor(tcb: Context, scope: Scope, private block: TmplAstForLoopBlock) { super(tcb, scope); + + // Tracking expressions are only allowed to read the `$index`, + // the item and properties off the component instance. + this.allowedVariables = new Set([block.item]); + for (const variable of block.contextVariables) { + if (variable.value === '$index') { + this.allowedVariables.add(variable); + } + } } protected override resolve(ast: AST): ts.Expression|null { if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver) { const target = this.tcb.boundTarget.getExpressionTarget(ast); - // Tracking expressions are only allowed to read the `$index`, - // the item and properties off the component instance. - if (target !== null && target !== this.block.item && - target !== this.block.contextVariables.$index) { + if (target !== null && !this.allowedVariables.has(target)) { this.tcb.oobRecorder.illegalForLoopTrackAccess(this.tcb.id, this.block, ast); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index 79ef478a3851e..73dced2cc86ea 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -1710,7 +1710,7 @@ describe('type check blocks', () => { expect(result).toContain('"" + (_t2) + (_t3) + (_t4) + (_t5) + (_t6) + (_t7)'); }); - it('should read an implicit variable from the component scope if it is aliased', () => { + it('should read both implicit variables and their alias at the same time', () => { const TEMPLATE = ` @for (item of items; track item; let i = $index) { {{$index}} {{i}} } `; @@ -1718,7 +1718,8 @@ describe('type check blocks', () => { const result = tcb(TEMPLATE); expect(result).toContain('for (const _t1 of ((this).items)!) {'); expect(result).toContain('var _t2 = null! as number;'); - expect(result).toContain('"" + (((this).$index)) + (_t2)'); + expect(result).toContain('var _t3 = null! as number;'); + expect(result).toContain('"" + (_t2) + (_t3)'); }); it('should read variable from a parent for loop', () => { 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 58b0c6b2779fb..4d77fad4272dd 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 @@ -477,9 +477,10 @@ runInEachFileSystem(() => { }); it('finds symbols for $index variable', () => { - const iVar = forLoopNode.contextVariables.$index; + const iVar = forLoopNode.contextVariables.find(v => v.name === '$index')!; const iSymbol = templateTypeChecker.getSymbolOfNode(iVar, cmp)!; - expectIndexSymbol(iSymbol); + expect(iVar).toBeTruthy(); + expectIndexSymbol(iSymbol, '$index'); }); it('finds symbol when using the index in the body', () => { @@ -487,7 +488,7 @@ runInEachFileSystem(() => { onlyAstElements((forLoopNode.children[0] as TmplAstElement).children); const indexSymbol = templateTypeChecker.getSymbolOfNode(innerElementNodes[0].inputs[0].value, cmp)!; - expectIndexSymbol(indexSymbol); + expectIndexSymbol(indexSymbol, 'i'); }); function expectUserSymbol(userSymbol: Symbol) { @@ -497,12 +498,15 @@ runInEachFileSystem(() => { expect((userSymbol).declaration).toEqual(forLoopNode.item); } - function expectIndexSymbol(indexSymbol: Symbol) { + function expectIndexSymbol(indexSymbol: Symbol, localName: string) { + const indexVar = + forLoopNode.contextVariables.find(v => v.value === '$index' && v.name === localName)!; assertVariableSymbol(indexSymbol); + expect(indexVar).toBeTruthy(); expect(indexSymbol.tsSymbol) .toBeNull(); // implicit variable doesn't have a TS definition location expect(program.getTypeChecker().typeToString(indexSymbol.tsType!)).toEqual('number'); - expect((indexSymbol).declaration).toEqual(forLoopNode.contextVariables.$index); + expect((indexSymbol).declaration).toEqual(indexVar); } }); }); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js index efe1cfc191f92..a12c5b2dfa10d 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js @@ -1143,6 +1143,75 @@ export declare class MyApp { static ɵcmp: i0.ɵɵComponentDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: for_both_aliased_and_original_variables.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyApp { + constructor() { + this.message = 'hello'; + this.items = []; + } +} +MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: ` +
+ {{message}} + @for (item of items; track item; let idx = $index, f = $first; let l = $last, ev = $even, o = $odd; let co = $count) { + Original index: {{$index}} + Original first: {{$first}} + Original last: {{$last}} + Original even: {{$even}} + Original odd: {{$odd}} + Original count: {{$count}} +
+ Aliased index: {{idx}} + Aliased first: {{f}} + Aliased last: {{l}} + Aliased even: {{ev}} + Aliased odd: {{o}} + Aliased count: {{co}} + } +
+ `, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{ + type: Component, + args: [{ + template: ` +
+ {{message}} + @for (item of items; track item; let idx = $index, f = $first; let l = $last, ev = $even, o = $odd; let co = $count) { + Original index: {{$index}} + Original first: {{$first}} + Original last: {{$last}} + Original even: {{$even}} + Original odd: {{$odd}} + Original count: {{$count}} +
+ Aliased index: {{idx}} + Aliased first: {{f}} + Aliased last: {{l}} + Aliased even: {{ev}} + Aliased odd: {{o}} + Aliased count: {{co}} + } +
+ `, + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: for_both_aliased_and_original_variables.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyApp { + message: string; + items: never[]; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + /**************************************************************************************************** * PARTIAL FILE: nested_for_template_variables.js ****************************************************************************************************/ diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json index 149cf84654c54..480b2b8fa543d 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json @@ -301,6 +301,21 @@ } ] }, + { + "description": "should generate a for block with references both to original and aliased template variables", + "inputFiles": ["for_both_aliased_and_original_variables.ts"], + "expectations": [ + { + "files": [ + { + "expected": "for_both_aliased_and_original_variables_template.js", + "generated": "for_both_aliased_and_original_variables.js" + } + ], + "failureMessage": "Incorrect template" + } + ] + }, { "description": "should be able to refer to aliased template variables in nested for blocks", "inputFiles": ["nested_for_template_variables.ts"], diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_aliased_template_variables_template.pipeline.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_aliased_template_variables_template.pipeline.js index 114f5acc08e8e..2038e433f9273 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_aliased_template_variables_template.pipeline.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_aliased_template_variables_template.pipeline.js @@ -3,11 +3,9 @@ function MyApp_For_3_Template(rf, ctx) { $r3$.ɵɵtext(0); } if (rf & 2) { - const $idx_r2$ = ctx.$index; - const $co_r3$ = ctx.$count; - const $idx_2_r2$ = ctx.$index; - const $co_2_r3$ = ctx.$count; - $r3$.ɵɵtextInterpolate6(" Index: ", $idx_r2$, " First: ", $idx_2_r2$ === 0, " Last: ", $idx_2_r2$ === $co_2_r3$ - 1, " Even: ", $idx_2_r2$ % 2 === 0, " Odd: ", $idx_2_r2$ % 2 !== 0, " Count: ", $co_r3$, " "); + const $idx_r2$ = ctx.$index; + const $co_r2$ = ctx.$count; + $r3$.ɵɵtextInterpolate6(" Index: ", $idx_r2$, " First: ", $idx_r2$ === 0, " Last: ", $idx_r2$ === $co_r2$ - 1, " Even: ", $idx_r2$ % 2 === 0, " Odd: ", $idx_r2$ % 2 !== 0, " Count: ", $co_r2$, " "); } } … diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_both_aliased_and_original_variables.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_both_aliased_and_original_variables.ts new file mode 100644 index 0000000000000..66b8c80835df1 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_both_aliased_and_original_variables.ts @@ -0,0 +1,28 @@ +import {Component} from '@angular/core'; + +@Component({ + template: ` +
+ {{message}} + @for (item of items; track item; let idx = $index, f = $first; let l = $last, ev = $even, o = $odd; let co = $count) { + Original index: {{$index}} + Original first: {{$first}} + Original last: {{$last}} + Original even: {{$even}} + Original odd: {{$odd}} + Original count: {{$count}} +
+ Aliased index: {{idx}} + Aliased first: {{f}} + Aliased last: {{l}} + Aliased even: {{ev}} + Aliased odd: {{o}} + Aliased count: {{co}} + } +
+ `, +}) +export class MyApp { + message = 'hello'; + items = []; +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_both_aliased_and_original_variables_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_both_aliased_and_original_variables_template.js new file mode 100644 index 0000000000000..c66c48fe54598 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_both_aliased_and_original_variables_template.js @@ -0,0 +1,31 @@ +function MyApp_For_3_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtext(0); + $r3$.ɵɵelement(1, "hr"); + $r3$.ɵɵtext(2); + } + if (rf & 2) { + const $index_r1$ = ctx.$index; + const $index_4_r2$ = ctx.$index; + const $count_r3$ = ctx.$count; + const $count_4_r4$ = ctx.$count; + $r3$.ɵɵtextInterpolate6(" Original index: ", $index_r1$, " Original first: ", $index_4_r2$ === 0, " Original last: ", $index_4_r2$ === $count_4_r4$ - 1, " Original even: ", $index_4_r2$ % 2 === 0, " Original odd: ", $index_4_r2$ % 2 !== 0, " Original count: ", $count_r3$, " "); + $r3$.ɵɵadvance(2); + $r3$.ɵɵtextInterpolate6(" Aliased index: ", $index_4_r2$, " Aliased first: ", $index_4_r2$ === 0, " Aliased last: ", $index_4_r2$ === $count_4_r4$ - 1, " Aliased even: ", $index_4_r2$ % 2 === 0, " Aliased odd: ", $index_4_r2$ % 2 !== 0, " Aliased count: ", $count_4_r4$, " "); + } +} +… +function MyApp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵelementStart(0, "div"); + $r3$.ɵɵtext(1); + $r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 3, 12, null, null, $r3$.ɵɵrepeaterTrackByIdentity); + $r3$.ɵɵelementEnd(); + } + if (rf & 2) { + $r3$.ɵɵadvance(); + $r3$.ɵɵtextInterpolate1(" ", ctx.message, " "); + $r3$.ɵɵadvance(); + $r3$.ɵɵrepeater(ctx.items); + } +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_listener_template.pipeline.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_listener_template.pipeline.js index ee614f018cb3c..268540e377e17 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_listener_template.pipeline.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_listener_template.pipeline.js @@ -5,8 +5,8 @@ function MyApp_For_3_Template(rf, ctx) { $r3$.ɵɵlistener("click", function MyApp_For_3_Template_div_click_0_listener() { const $restoredCtx$ = $r3$.ɵɵrestoreView($_r5$); const $index_r2$ = $restoredCtx$.$index; - const $count_r3$ = $restoredCtx$.$count; const $index_2_r2$ = $restoredCtx$.$index; + const $count_r3$ = $restoredCtx$.$count; const $ctx_r4$ = $r3$.ɵɵnextContext(); return $r3$.ɵɵresetView($ctx_r4$.log($index_r2$, $index_2_r2$ % 2 === 0, $index_2_r2$ === 0, $count_r3$)); }); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_scope_template.pipeline.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_scope_template.pipeline.js index dce1add3cb75c..c88e1141926da 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_scope_template.pipeline.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_scope_template.pipeline.js @@ -4,8 +4,8 @@ function MyApp_For_2_Template(rf, ctx) { } if (rf & 2) { const $index_r2$ = ctx.$index; - const $count_r3$ = ctx.$count; const $index_2_r2$ = ctx.$index; + const $count_r3$ = ctx.$count; const $count_2_r3$ = ctx.$count; $r3$.ɵɵtextInterpolate4(" ", $index_r2$, " ", $count_r3$, " ", $index_2_r2$ === 0, " ", $index_2_r2$ === $count_2_r3$ - 1, " "); } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_template.pipeline.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_template.pipeline.js index 4b02fdaba049b..a416c2a95b843 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_template.pipeline.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_template.pipeline.js @@ -4,8 +4,8 @@ function MyApp_For_3_Template(rf, ctx) { } if (rf & 2) { const $index_r2$ = ctx.$index; - const $count_r3$ = ctx.$count; const $index_4_r2$ = ctx.$index; + const $count_r3$ = ctx.$count; const $count_4_r2$ = ctx.$count; $r3$.ɵɵtextInterpolate6(" Index: ", $index_r2$, " First: ", $index_4_r2$ === 0, " Last: ", $index_4_r2$ === $count_4_r2$ - 1, " Even: ", $index_4_r2$ % 2 === 0, " Odd: ", $index_4_r2$ % 2 !== 0, " Count: ", $count_r3$, " "); } diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 21fc68c1c758f..4879be625f8fd 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -4828,23 +4828,25 @@ suppress ]); }); - it('should not expose variables under their implicit name if they are aliased', () => { - env.write('test.ts', ` - import {Component} from '@angular/core'; + it('should continue exposing implicit loop variables under their old names when they are aliased', + () => { + env.write('test.ts', ` + import {Component} from '@angular/core'; - @Component({ - template: '@for (item of items; track item; let alias = $index) { {{$index}} {{$count}} }', - }) - export class Main { - items = []; - } - `); + @Component({ + template: '@for (item of items; track item; let alias = $index) { {{acceptsString($index)}} }', + }) + export class Main { + items = []; + acceptsString(str: string) {} + } + `); - const diags = env.driveDiagnostics(); - expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([ - `Property '$index' does not exist on type 'Main'.`, - ]); - }); + const diags = env.driveDiagnostics(); + expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([ + `Argument of type 'number' is not assignable to parameter of type 'string'.`, + ]); + }); it('should not be able to write to loop template variables', () => { env.write('test.ts', ` diff --git a/packages/compiler/src/render3/r3_ast.ts b/packages/compiler/src/render3/r3_ast.ts index 35e3c5a2f4181..5f26e1b9b16ce 100644 --- a/packages/compiler/src/render3/r3_ast.ts +++ b/packages/compiler/src/render3/r3_ast.ts @@ -300,16 +300,10 @@ export class SwitchBlockCase extends BlockNode implements Node { } } -// Note: this is a weird way to define the properties, but we do it so that we can -// get strong typing when the context is passed through `Object.values`. -/** Context variables that can be used inside a `ForLoopBlock`. */ -export type ForLoopBlockContext = - Record<'$index'|'$first'|'$last'|'$even'|'$odd'|'$count', Variable>; - export class ForLoopBlock extends BlockNode implements Node { constructor( public item: Variable, public expression: ASTWithSource, public trackBy: ASTWithSource, - public trackKeywordSpan: ParseSourceSpan, public contextVariables: ForLoopBlockContext, + public trackKeywordSpan: ParseSourceSpan, public contextVariables: Variable[], public children: Node[], public empty: ForLoopBlockEmpty|null, sourceSpan: ParseSourceSpan, public mainBlockSpan: ParseSourceSpan, startSourceSpan: ParseSourceSpan, endSourceSpan: ParseSourceSpan|null, nameSpan: ParseSourceSpan, public i18n?: I18nMeta) { @@ -496,7 +490,7 @@ export class RecursiveVisitor implements Visitor { visitAll(this, block.children); } visitForLoopBlock(block: ForLoopBlock): void { - const blockItems = [block.item, ...Object.values(block.contextVariables), ...block.children]; + const blockItems = [block.item, ...block.contextVariables, ...block.children]; block.empty && blockItems.push(block.empty); visitAll(this, blockItems); } diff --git a/packages/compiler/src/render3/r3_control_flow.ts b/packages/compiler/src/render3/r3_control_flow.ts index 27fcdd02909d2..005764090ae65 100644 --- a/packages/compiler/src/render3/r3_control_flow.ts +++ b/packages/compiler/src/render3/r3_control_flow.ts @@ -36,7 +36,7 @@ const CHARACTERS_IN_SURROUNDING_WHITESPACE_PATTERN = /(\s*)(\S+)(\s*)/; /** Names of variables that are allowed to be used in the `let` expression of a `for` loop. */ const ALLOWED_FOR_LOOP_LET_VARIABLES = - new Set(['$index', '$first', '$last', '$even', '$odd', '$count']); + new Set(['$index', '$first', '$last', '$even', '$odd', '$count']); /** * Predicate function that determines if a block with @@ -234,7 +234,16 @@ function parseForLoopParameters( itemName: new t.Variable(itemName, '$implicit', variableSpan, variableSpan), trackBy: null as {expression: ASTWithSource, keywordSpan: ParseSourceSpan} | null, expression: parseBlockParameterToBinding(expressionParam, bindingParser, rawExpression), - context: {} as t.ForLoopBlockContext, + context: Array.from( + ALLOWED_FOR_LOOP_LET_VARIABLES, + variableName => { + // Give ambiently-available context variables empty spans at the end of + // the start of the `for` block, since they are not explicitly defined. + const emptySpanAfterForBlockStart = + new ParseSourceSpan(block.startSourceSpan.end, block.startSourceSpan.end); + return new t.Variable( + variableName, variableName, emptySpanAfterForBlockStart, emptySpanAfterForBlockStart); + }), }; for (const param of secondaryParams) { @@ -270,32 +279,19 @@ function parseForLoopParameters( new ParseError(param.sourceSpan, `Unrecognized @for loop paramater "${param.expression}"`)); } - // Fill out any variables that haven't been defined explicitly. - for (const variableName of ALLOWED_FOR_LOOP_LET_VARIABLES) { - if (!result.context.hasOwnProperty(variableName)) { - // Give ambiently-available context variables empty spans at the end of the start of the `for` - // block, since they are not explicitly defined. - const emptySpanAfterForBlockStart = - new ParseSourceSpan(block.startSourceSpan.end, block.startSourceSpan.end); - result.context[variableName] = new t.Variable( - variableName, variableName, emptySpanAfterForBlockStart, emptySpanAfterForBlockStart); - } - } - return result; } /** Parses the `let` parameter of a `for` loop block. */ function parseLetParameter( - sourceSpan: ParseSourceSpan, expression: string, span: ParseSourceSpan, - context: t.ForLoopBlockContext, errors: ParseError[]): void { + sourceSpan: ParseSourceSpan, expression: string, span: ParseSourceSpan, context: t.Variable[], + errors: ParseError[]): void { const parts = expression.split(','); let startSpan = span.start; for (const part of parts) { const expressionParts = part.split('='); const name = expressionParts.length === 2 ? expressionParts[0].trim() : ''; - const variableName = (expressionParts.length === 2 ? expressionParts[1].trim() : '') as - keyof t.ForLoopBlockContext; + const variableName = expressionParts.length === 2 ? expressionParts[1].trim() : ''; if (name.length === 0 || variableName.length === 0) { errors.push(new ParseError( @@ -306,7 +302,7 @@ function parseLetParameter( sourceSpan, `Unknown "let" parameter variable "${variableName}". The allowed variables are: ${ Array.from(ALLOWED_FOR_LOOP_LET_VARIABLES).join(', ')}`)); - } else if (context.hasOwnProperty(variableName)) { + } else if (context.some(v => v.name === name)) { errors.push( new ParseError(sourceSpan, `Duplicate "let" parameter variable "${variableName}"`)); } else { @@ -333,7 +329,7 @@ function parseLetParameter( undefined; } const sourceSpan = new ParseSourceSpan(keySpan.start, valueSpan?.end ?? keySpan.end); - context[variableName] = new t.Variable(name, variableName, sourceSpan, keySpan, valueSpan); + context.push(new t.Variable(name, variableName, sourceSpan, keySpan, valueSpan)); } startSpan = startSpan.moveBy(part.length + 1 /* add 1 to move past the comma */); } diff --git a/packages/compiler/src/render3/view/t2_binder.ts b/packages/compiler/src/render3/view/t2_binder.ts index 40f5ab6ba302e..d188216a3156d 100644 --- a/packages/compiler/src/render3/view/t2_binder.ts +++ b/packages/compiler/src/render3/view/t2_binder.ts @@ -117,7 +117,7 @@ class Scope implements Visitor { nodeOrNodes.children.forEach(node => node.visit(this)); } else if (nodeOrNodes instanceof ForLoopBlock) { this.visitVariable(nodeOrNodes.item); - Object.values(nodeOrNodes.contextVariables).forEach(v => this.visitVariable(v)); + nodeOrNodes.contextVariables.forEach(v => this.visitVariable(v)); nodeOrNodes.children.forEach(node => node.visit(this)); } else if ( nodeOrNodes instanceof SwitchBlockCase || nodeOrNodes instanceof ForLoopBlockEmpty || @@ -424,7 +424,7 @@ class DirectiveBinder implements Visitor { visitForLoopBlock(block: ForLoopBlock) { block.item.visit(this); - Object.values(block.contextVariables).forEach(v => v.visit(this)); + block.contextVariables.forEach(v => v.visit(this)); block.children.forEach(node => node.visit(this)); block.empty?.visit(this); } @@ -543,7 +543,7 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor { this.nestingLevel.set(nodeOrNodes, this.level); } else if (nodeOrNodes instanceof ForLoopBlock) { this.visitNode(nodeOrNodes.item); - Object.values(nodeOrNodes.contextVariables).forEach(v => this.visitNode(v)); + nodeOrNodes.contextVariables.forEach(v => this.visitNode(v)); nodeOrNodes.trackBy.visit(this); nodeOrNodes.children.forEach(this.visitNode); this.nestingLevel.set(nodeOrNodes, this.level); diff --git a/packages/compiler/src/template/pipeline/ir/src/ops/create.ts b/packages/compiler/src/template/pipeline/ir/src/ops/create.ts index 34c24f6d9a077..4e9b451f5d76b 100644 --- a/packages/compiler/src/template/pipeline/ir/src/ops/create.ts +++ b/packages/compiler/src/template/pipeline/ir/src/ops/create.ts @@ -305,12 +305,7 @@ export interface RepeaterCreateOp extends ElementOpBase, ConsumesVarsTrait { // TODO: add source spans? export interface RepeaterVarNames { - $index: string; - $count: string; - $first: string; - $last: string; - $even: string; - $odd: string; + $index: Set; $implicit: string; } diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index 5f88058c030da..a5847e4abf48f 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -598,48 +598,35 @@ function ingestIcu(unit: ViewCompilationUnit, icu: t.Icu) { function ingestForBlock(unit: ViewCompilationUnit, forBlock: t.ForLoopBlock): void { const repeaterView = unit.job.allocateView(unit.xref); + // We copy TemplateDefinitionBuilder's scheme of creating names for `$count` and `$index` + // that are suffixed with special information, to disambiguate which level of nested loop + // the below aliases refer to. + // TODO: We should refactor Template Pipeline's variable phases to gracefully handle + // shadowing, and arbitrarily many levels of variables depending on each other. + const indexName = `ɵ$index_${repeaterView.xref}`; + const countName = `ɵ$count_${repeaterView.xref}`; + const indexVarNames = new Set(); + // Set all the context variables and aliases available in the repeater. repeaterView.contextVariables.set(forBlock.item.name, forBlock.item.value); - repeaterView.contextVariables.set( - forBlock.contextVariables.$index.name, forBlock.contextVariables.$index.value); - repeaterView.contextVariables.set( - forBlock.contextVariables.$count.name, forBlock.contextVariables.$count.value); - - // We copy TemplateDefinitionBuilder's scheme of creating names for `$count` and `$index` that are - // suffixed with special information, to disambiguate which level of nested loop the below aliases - // refer to. - // TODO: We should refactor Template Pipeline's variable phases to gracefully handle shadowing, - // and arbitrarily many levels of variables depending on each other. - const indexName = `ɵ${forBlock.contextVariables.$index.name}_${repeaterView.xref}`; - const countName = `ɵ${forBlock.contextVariables.$count.name}_${repeaterView.xref}`; - repeaterView.contextVariables.set(indexName, forBlock.contextVariables.$index.value); - repeaterView.contextVariables.set(countName, forBlock.contextVariables.$count.value); - - repeaterView.aliases.add({ - kind: ir.SemanticVariableKind.Alias, - name: null, - identifier: forBlock.contextVariables.$first.name, - expression: new ir.LexicalReadExpr(indexName).identical(o.literal(0)) - }); - repeaterView.aliases.add({ - kind: ir.SemanticVariableKind.Alias, - name: null, - identifier: forBlock.contextVariables.$last.name, - expression: new ir.LexicalReadExpr(indexName).identical( - new ir.LexicalReadExpr(countName).minus(o.literal(1))) - }); - repeaterView.aliases.add({ - kind: ir.SemanticVariableKind.Alias, - name: null, - identifier: forBlock.contextVariables.$even.name, - expression: new ir.LexicalReadExpr(indexName).modulo(o.literal(2)).identical(o.literal(0)) - }); - repeaterView.aliases.add({ - kind: ir.SemanticVariableKind.Alias, - name: null, - identifier: forBlock.contextVariables.$odd.name, - expression: new ir.LexicalReadExpr(indexName).modulo(o.literal(2)).notIdentical(o.literal(0)) - }); + + for (const variable of forBlock.contextVariables) { + if (variable.value === '$index') { + indexVarNames.add(variable.name); + } + if (variable.name === '$index') { + repeaterView.contextVariables.set('$index', variable.value).set(indexName, variable.value); + } else if (variable.name === '$count') { + repeaterView.contextVariables.set('$count', variable.value).set(countName, variable.value); + } else { + repeaterView.aliases.add({ + kind: ir.SemanticVariableKind.Alias, + name: null, + identifier: variable.name, + expression: getComputedForLoopVariableExpression(variable, indexName, countName) + }); + } + } const sourceSpan = convertSourceSpan(forBlock.trackBy.span, forBlock.sourceSpan); const track = convertAst(forBlock.trackBy, unit.job, sourceSpan); @@ -655,12 +642,7 @@ function ingestForBlock(unit: ViewCompilationUnit, forBlock: t.ForLoopBlock): vo } const varNames: ir.RepeaterVarNames = { - $index: forBlock.contextVariables.$index.name, - $count: forBlock.contextVariables.$count.name, - $first: forBlock.contextVariables.$first.name, - $last: forBlock.contextVariables.$last.name, - $even: forBlock.contextVariables.$even.name, - $odd: forBlock.contextVariables.$odd.name, + $index: indexVarNames, $implicit: forBlock.item.name, }; @@ -688,6 +670,39 @@ function ingestForBlock(unit: ViewCompilationUnit, forBlock: t.ForLoopBlock): vo unit.update.push(repeater); } +/** + * Gets an expression that represents a variable in an `@for` loop. + * @param variable AST representing the variable. + * @param indexName Loop-specific name for `$index`. + * @param countName Loop-specific name for `$count`. + */ +function getComputedForLoopVariableExpression( + variable: t.Variable, indexName: string, countName: string): o.Expression { + switch (variable.value) { + case '$index': + return new ir.LexicalReadExpr(indexName); + + case '$count': + return new ir.LexicalReadExpr(countName); + + case '$first': + return new ir.LexicalReadExpr(indexName).identical(o.literal(0)); + + case '$last': + return new ir.LexicalReadExpr(indexName).identical( + new ir.LexicalReadExpr(countName).minus(o.literal(1))); + + case '$even': + return new ir.LexicalReadExpr(indexName).modulo(o.literal(2)).identical(o.literal(0)); + + case '$odd': + return new ir.LexicalReadExpr(indexName).modulo(o.literal(2)).notIdentical(o.literal(0)); + + default: + throw new Error(`AssertionError: unknown @for loop variable ${variable.value}`); + } +} + /** * Convert a template AST expression into an output AST expression. */ diff --git a/packages/compiler/src/template/pipeline/src/phases/track_variables.ts b/packages/compiler/src/template/pipeline/src/phases/track_variables.ts index 9b4a484fb233d..a8466ee40eaa0 100644 --- a/packages/compiler/src/template/pipeline/src/phases/track_variables.ts +++ b/packages/compiler/src/template/pipeline/src/phases/track_variables.ts @@ -25,7 +25,7 @@ export function generateTrackVariables(job: CompilationJob): void { op.track = ir.transformExpressionsInExpression(op.track, expr => { if (expr instanceof ir.LexicalReadExpr) { - if (expr.name === op.varNames.$index) { + if (op.varNames.$index.has(expr.name)) { return o.variable('$index'); } else if (expr.name === op.varNames.$implicit) { return o.variable('$item'); diff --git a/packages/compiler/test/render3/r3_ast_spans_spec.ts b/packages/compiler/test/render3/r3_ast_spans_spec.ts index a7badf063aaa4..8b7c70b8f782c 100644 --- a/packages/compiler/test/render3/r3_ast_spans_spec.ts +++ b/packages/compiler/test/render3/r3_ast_spans_spec.ts @@ -133,7 +133,7 @@ class R3AstSourceSpans implements t.Visitor { humanizeSpan(block.endSourceSpan) ]); this.visitVariable(block.item); - this.visitAll([Object.values(block.contextVariables)]); + this.visitAll([block.contextVariables]); this.visitAll([block.children]); block.empty?.visit(this); } @@ -702,12 +702,14 @@ describe('R3 AST source spans', () => { '@for (item of items.foo.bar; track item.id; let i = $index, _o_d_d_ = $odd) {', '}' ], ['Variable', 'item', 'item', ''], - ['Variable', 'i = $index', 'i', '$index'], - ['Variable', '_o_d_d_ = $odd', '_o_d_d_', '$odd'], ['Variable', '', '', ''], ['Variable', '', '', ''], ['Variable', '', '', ''], ['Variable', '', '', ''], + ['Variable', '', '', ''], + ['Variable', '', '', ''], + ['Variable', 'i = $index', 'i', '$index'], + ['Variable', '_o_d_d_ = $odd', '_o_d_d_', '$odd'], ['Element', '

{{ item }}

', '

', '

'], ['BoundText', '{{ item }}'], ['ForLoopBlockEmpty', '@empty {There were no items in the list.}', '@empty {'], diff --git a/packages/compiler/test/render3/r3_template_transform_spec.ts b/packages/compiler/test/render3/r3_template_transform_spec.ts index d51fe06a2d786..2dfdcf5a4d336 100644 --- a/packages/compiler/test/render3/r3_template_transform_spec.ts +++ b/packages/compiler/test/render3/r3_template_transform_spec.ts @@ -108,8 +108,7 @@ class R3AstHumanizer implements t.Visitor { visitForLoopBlock(block: t.ForLoopBlock): void { const result: any[] = ['ForLoopBlock', unparse(block.expression), unparse(block.trackBy)]; this.result.push(result); - const explicitVariables = Object.values(block.contextVariables).filter(v => v.name !== v.value); - this.visitAll([[block.item], explicitVariables, block.children]); + this.visitAll([[block.item], block.contextVariables, block.children]); block.empty?.visit(this); } @@ -1567,6 +1566,12 @@ describe('R3 template transform', () => { `).toEqual([ ['ForLoopBlock', 'items.foo.bar', 'item.id'], ['Variable', 'item', '$implicit'], + ['Variable', '$index', '$index'], + ['Variable', '$first', '$first'], + ['Variable', '$last', '$last'], + ['Variable', '$even', '$even'], + ['Variable', '$odd', '$odd'], + ['Variable', '$count', '$count'], ['BoundText', ' {{ item }} '], ['ForLoopBlockEmpty'], ['Text', ' There were no items in the list. '], @@ -1581,6 +1586,12 @@ describe('R3 template transform', () => { `).toEqual([ ['ForLoopBlock', 'items.foo.bar', 'item.id'], ['Variable', 'item', '$implicit'], + ['Variable', '$index', '$index'], + ['Variable', '$first', '$first'], + ['Variable', '$last', '$last'], + ['Variable', '$even', '$even'], + ['Variable', '$odd', '$odd'], + ['Variable', '$count', '$count'], ['BoundText', ' {{ item }} '], ]); @@ -1591,6 +1602,12 @@ describe('R3 template transform', () => { `).toEqual([ ['ForLoopBlock', 'items.foo.bar()', 'item.id'], ['Variable', 'item', '$implicit'], + ['Variable', '$index', '$index'], + ['Variable', '$first', '$first'], + ['Variable', '$last', '$last'], + ['Variable', '$even', '$even'], + ['Variable', '$odd', '$odd'], + ['Variable', '$count', '$count'], ['BoundText', ' {{ item }} '], ]); @@ -1601,6 +1618,12 @@ describe('R3 template transform', () => { `).toEqual([ ['ForLoopBlock', 'items.foo.bar()', 'item.id'], ['Variable', 'item', '$implicit'], + ['Variable', '$index', '$index'], + ['Variable', '$first', '$first'], + ['Variable', '$last', '$last'], + ['Variable', '$even', '$even'], + ['Variable', '$odd', '$odd'], + ['Variable', '$count', '$count'], ['BoundText', ' {{ item }} '], ]); }); @@ -1613,6 +1636,12 @@ describe('R3 template transform', () => { `).toEqual([ ['ForLoopBlock', 'items.foo.bar', 'item.id'], ['Variable', 'item', '$implicit'], + ['Variable', '$index', '$index'], + ['Variable', '$first', '$first'], + ['Variable', '$last', '$last'], + ['Variable', '$even', '$even'], + ['Variable', '$odd', '$odd'], + ['Variable', '$count', '$count'], ['Variable', 'idx', '$index'], ['Variable', 'f', '$first'], ['Variable', 'c', '$count'], @@ -1631,6 +1660,12 @@ describe('R3 template transform', () => { `).toEqual([ ['ForLoopBlock', 'items.foo.bar', 'item.id'], ['Variable', 'item', '$implicit'], + ['Variable', '$index', '$index'], + ['Variable', '$first', '$first'], + ['Variable', '$last', '$last'], + ['Variable', '$even', '$even'], + ['Variable', '$odd', '$odd'], + ['Variable', '$count', '$count'], ['Variable', 'idx', '$index'], ['Variable', 'f', '$first'], ['Variable', 'c', '$count'], @@ -1655,10 +1690,22 @@ describe('R3 template transform', () => { `).toEqual([ ['ForLoopBlock', 'items.foo.bar', 'item.id'], ['Variable', 'item', '$implicit'], + ['Variable', '$index', '$index'], + ['Variable', '$first', '$first'], + ['Variable', '$last', '$last'], + ['Variable', '$even', '$even'], + ['Variable', '$odd', '$odd'], + ['Variable', '$count', '$count'], ['BoundText', ' {{ item }} '], ['Element', 'div'], ['ForLoopBlock', 'item.items', 'subitem.id'], ['Variable', 'subitem', '$implicit'], + ['Variable', '$index', '$index'], + ['Variable', '$first', '$first'], + ['Variable', '$last', '$last'], + ['Variable', '$even', '$even'], + ['Variable', '$odd', '$odd'], + ['Variable', '$count', '$count'], ['Element', 'h1'], ['BoundText', '{{ subitem }}'], ['ForLoopBlockEmpty'], @@ -1674,6 +1721,12 @@ describe('R3 template transform', () => { `).toEqual([ ['ForLoopBlock', 'items.foo.bar', 'trackBy(item.id, 123)'], ['Variable', 'item', '$implicit'], + ['Variable', '$index', '$index'], + ['Variable', '$first', '$first'], + ['Variable', '$last', '$last'], + ['Variable', '$even', '$even'], + ['Variable', '$odd', '$odd'], + ['Variable', '$count', '$count'], ['BoundText', ' {{ item }} '], ]); }); @@ -1682,6 +1735,12 @@ describe('R3 template transform', () => { const expectedResult = [ ['ForLoopBlock', 'items.foo.bar', 'item.id + foo'], ['Variable', 'item', '$implicit'], + ['Variable', '$index', '$index'], + ['Variable', '$first', '$first'], + ['Variable', '$last', '$last'], + ['Variable', '$even', '$even'], + ['Variable', '$odd', '$odd'], + ['Variable', '$count', '$count'], ['BoundText', '{{ item }}'], ]; @@ -1704,6 +1763,12 @@ describe('R3 template transform', () => { `).toEqual([ ['ForLoopBlock', '[{id: 1}, {id: 2}]', 'item.id'], ['Variable', 'item', '$implicit'], + ['Variable', '$index', '$index'], + ['Variable', '$first', '$first'], + ['Variable', '$last', '$last'], + ['Variable', '$even', '$even'], + ['Variable', '$odd', '$odd'], + ['Variable', '$count', '$count'], ['BoundText', ' {{ item }} '], ]); }); @@ -1811,7 +1876,9 @@ describe('R3 template transform', () => { it('should report duplicate `let` parameter variables', () => { expect( () => parse( - `@for (item of items.foo.bar; track item.id; let i = $index, f = $first, in = $index) {}`)) + `@for (item of items.foo.bar; track item.id; let i = $index, f = $first, i = $index) {}`)) + .toThrowError(/Duplicate "let" parameter variable "\$index"/); + expect(() => parse(`@for (item of items.foo.bar; track item.id; let $index = $index) {}`)) .toThrowError(/Duplicate "let" parameter variable "\$index"/); }); }); diff --git a/packages/compiler/test/render3/util/expression.ts b/packages/compiler/test/render3/util/expression.ts index c3209f71d0f5d..0b590527627d2 100644 --- a/packages/compiler/test/render3/util/expression.ts +++ b/packages/compiler/test/render3/util/expression.ts @@ -182,7 +182,7 @@ class ExpressionSourceHumanizer extends e.RecursiveAstVisitor implements t.Visit visitForLoopBlock(block: t.ForLoopBlock) { block.item.visit(this); - t.visitAll(this, Object.values(block.contextVariables)); + t.visitAll(this, block.contextVariables); block.expression.visit(this); t.visitAll(this, block.children); block.empty?.visit(this); diff --git a/packages/core/test/acceptance/control_flow_for_spec.ts b/packages/core/test/acceptance/control_flow_for_spec.ts index fea77449c6f03..9fa35d2c31553 100644 --- a/packages/core/test/acceptance/control_flow_for_spec.ts +++ b/packages/core/test/acceptance/control_flow_for_spec.ts @@ -155,6 +155,24 @@ describe('control flow - for', () => { expect(fixture.nativeElement.textContent.trim()).toBe('2'); }); + it('should expose variables both under their real names and aliases', () => { + @Component({ + template: + '@for ((item of items); track item; let idx = $index) {{{item}}({{$index}}/{{idx}})|}', + }) + class TestComponent { + items = [1, 2, 3]; + } + + const fixture = TestBed.createComponent(TestComponent); + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toBe('1(0/0)|2(1/1)|3(2/2)|'); + + fixture.componentInstance.items.splice(1, 1); + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toBe('1(0/0)|3(1/1)|'); + }); + describe('trackBy', () => { it('should have access to the host context in the track function', () => { let offsetReads = 0; @@ -212,14 +230,14 @@ describe('control flow - for', () => { @Component({ template: ` - @if (true) { - @if (true) { - @if (true) { - @for ((item of items); track trackingFn(item, compProp)) {{{item}}} - } - } - } - `, + @if (true) { + @if (true) { + @if (true) { + @for ((item of items); track trackingFn(item, compProp)) {{{item}}} + } + } + } + `, }) class TestComponent { items = ['a', 'b']; diff --git a/packages/language-service/src/template_target.ts b/packages/language-service/src/template_target.ts index f9ec79b508217..20f709512418f 100644 --- a/packages/language-service/src/template_target.ts +++ b/packages/language-service/src/template_target.ts @@ -519,7 +519,7 @@ class TemplateTargetVisitor implements TmplAstVisitor { visitForLoopBlock(block: TmplAstForLoopBlock) { this.visit(block.item); - this.visitAll(Object.values(block.contextVariables)); + this.visitAll(block.contextVariables); this.visitBinding(block.expression); this.visitBinding(block.trackBy); this.visitAll(block.children);