From 49dca36880a1c1c394533e8a94db9c5ef412ebd2 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 16 Nov 2023 06:46:09 +0100 Subject: [PATCH] fix(compiler): nested for loops incorrectly calculating computed variables (#52931) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `$first`, `$last`, `$even` and `$odd` variables in `@for` loops aren't defined on the template context of the loop, but are computed based on `$index` and `$count` (e.g. `$first` is defined as `$index === 0`). We do this calculation by looking up `$index` and `$count` when one of the variables is used. The problem is that all `@for` loop variables are available implicitly which means that when a nested loop tries to rewrite a reference to an outer loop computed variable, it finds its own `$index` and `$count` first and it doesn't look up the ones on the parent at all. This means that the calculated values will be incorrect at runtime. These changes work around the issue by defining nested-level-specific variable names that can be used for lookups (e.g. `$index` at level `2` will also be available as `ɵ$index_2`). This isn't the most elegant solution, however the `TemplatDefitinionBuilder` wasn't set up to handle shadowed variables like this and it doesn't make sense to refactor it given the upcoming template pipeline. Fixes #52917. PR Close #52931 --- .../GOLDEN_PARTIAL.js | 126 ++++++++++++++++++ .../TEST_CASES.json | 36 +++++ .../nested_for_computed_template_variables.ts | 24 ++++ ...or_computed_template_variables_template.js | 56 ++++++++ ...or_listener_computed_template_variables.ts | 27 ++++ ...er_computed_template_variables_template.js | 95 +++++++++++++ .../compiler/src/render3/view/template.ts | 96 +++++++++---- 7 files changed, 431 insertions(+), 29 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_computed_template_variables.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_computed_template_variables_template.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_listener_computed_template_variables.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_listener_computed_template_variables_template.js 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 93279976ce460..ea5163c842130 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 @@ -1800,3 +1800,129 @@ export declare class MyApp { static ɵcmp: i0.ɵɵComponentDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: nested_for_computed_template_variables.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyApp { + constructor() { + 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: ` + @for (outer of items; track outer; let outerOdd = $odd, outerEven = $even, outerFirst = $first, outerLast = $last) { + Outer vars: {{outerOdd}} {{outerEven}} {{outerFirst}} {{outerLast}} + @for (inner of items; track inner; let innerOdd = $odd, innerEven = $even, innerFirst = $first, innerLast = $last) { + Inner vars: {{innerOdd}} {{innerEven}} {{innerFirst}} {{innerLast}} +
+ Outer vars: {{outerOdd}} {{outerEven}} {{outerFirst}} {{outerLast}} + @for (innermost of items; track innermost; let innermostOdd = $odd, innermostEven = $even, innermostFirst = $first, innermostLast = $last) { + Innermost vars: {{innermostOdd}} {{innermostEven}} {{innermostFirst}} {{innermostLast}} +
+ Inner vars: {{innerOdd}} {{innerEven}} {{innerFirst}} {{innerLast}} +
+ Outer vars: {{outerOdd}} {{outerEven}} {{outerFirst}} {{outerLast}} + } + } + } + `, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{ + type: Component, + args: [{ + template: ` + @for (outer of items; track outer; let outerOdd = $odd, outerEven = $even, outerFirst = $first, outerLast = $last) { + Outer vars: {{outerOdd}} {{outerEven}} {{outerFirst}} {{outerLast}} + @for (inner of items; track inner; let innerOdd = $odd, innerEven = $even, innerFirst = $first, innerLast = $last) { + Inner vars: {{innerOdd}} {{innerEven}} {{innerFirst}} {{innerLast}} +
+ Outer vars: {{outerOdd}} {{outerEven}} {{outerFirst}} {{outerLast}} + @for (innermost of items; track innermost; let innermostOdd = $odd, innermostEven = $even, innermostFirst = $first, innermostLast = $last) { + Innermost vars: {{innermostOdd}} {{innermostEven}} {{innermostFirst}} {{innermostLast}} +
+ Inner vars: {{innerOdd}} {{innerEven}} {{innerFirst}} {{innerLast}} +
+ Outer vars: {{outerOdd}} {{outerEven}} {{outerFirst}} {{outerLast}} + } + } + } + `, + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: nested_for_computed_template_variables.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyApp { + items: never[]; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + +/**************************************************************************************************** + * PARTIAL FILE: nested_for_listener_computed_template_variables.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyApp { + constructor() { + this.items = []; + } + outerCb(...args) { } + innerCb(...args) { } + innermostCb(...args) { } +} +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: ` + @for (outer of items; track outer; let outerOdd = $odd, outerEven = $even, outerFirst = $first, outerLast = $last) { + + + @for (inner of items; track inner; let innerOdd = $odd, innerEven = $even, innerFirst = $first, innerLast = $last) { + + + + @for (innermost of items; track innermost; let innermostOdd = $odd, innermostEven = $even, innermostFirst = $first, innermostLast = $last) { + + + + } + } + } + `, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{ + type: Component, + args: [{ + template: ` + @for (outer of items; track outer; let outerOdd = $odd, outerEven = $even, outerFirst = $first, outerLast = $last) { + + + @for (inner of items; track inner; let innerOdd = $odd, innerEven = $even, innerFirst = $first, innerLast = $last) { + + + + @for (innermost of items; track innermost; let innermostOdd = $odd, innermostEven = $even, innermostFirst = $first, innermostLast = $last) { + + + + } + } + } + `, + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: nested_for_listener_computed_template_variables.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyApp { + items: never[]; + outerCb(...args: unknown[]): void; + innerCb(...args: unknown[]): void; + innermostCb(...args: unknown[]): void; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + 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 38c176dcc6feb..8e5b68d6c1d25 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 @@ -581,6 +581,42 @@ "failureMessage": "Incorrect template" } ] + }, + { + "description": "should generate computed for loop variables that depend on shadowed $index and $count", + "inputFiles": [ + "nested_for_computed_template_variables.ts" + ], + "expectations": [ + { + "files": [ + { + "expected": "nested_for_computed_template_variables_template.js", + "generated": "nested_for_computed_template_variables.js" + } + ], + "failureMessage": "Incorrect template" + } + ], + "skipForTemplatePipeline": true + }, + { + "description": "should generate computed for loop variables that depend on shadowed $index and $count inside of listeners", + "inputFiles": [ + "nested_for_listener_computed_template_variables.ts" + ], + "expectations": [ + { + "files": [ + { + "expected": "nested_for_listener_computed_template_variables_template.js", + "generated": "nested_for_listener_computed_template_variables.js" + } + ], + "failureMessage": "Incorrect template" + } + ], + "skipForTemplatePipeline": true } ] } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_computed_template_variables.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_computed_template_variables.ts new file mode 100644 index 0000000000000..1d742d6577603 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_computed_template_variables.ts @@ -0,0 +1,24 @@ +import {Component} from '@angular/core'; + +@Component({ + template: ` + @for (outer of items; track outer; let outerOdd = $odd, outerEven = $even, outerFirst = $first, outerLast = $last) { + Outer vars: {{outerOdd}} {{outerEven}} {{outerFirst}} {{outerLast}} + @for (inner of items; track inner; let innerOdd = $odd, innerEven = $even, innerFirst = $first, innerLast = $last) { + Inner vars: {{innerOdd}} {{innerEven}} {{innerFirst}} {{innerLast}} +
+ Outer vars: {{outerOdd}} {{outerEven}} {{outerFirst}} {{outerLast}} + @for (innermost of items; track innermost; let innermostOdd = $odd, innermostEven = $even, innermostFirst = $first, innermostLast = $last) { + Innermost vars: {{innermostOdd}} {{innermostEven}} {{innermostFirst}} {{innermostLast}} +
+ Inner vars: {{innerOdd}} {{innerEven}} {{innerFirst}} {{innerLast}} +
+ Outer vars: {{outerOdd}} {{outerEven}} {{outerFirst}} {{outerLast}} + } + } + } + `, +}) +export class MyApp { + items = []; +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_computed_template_variables_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_computed_template_variables_template.js new file mode 100644 index 0000000000000..6b8d5be06aa18 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_computed_template_variables_template.js @@ -0,0 +1,56 @@ +function MyApp_For_1_For_2_For_4_Template(rf, ctx) { + … + if (rf & 2) { + const $index_r14 = ctx.$index; + const $count_r16 = ctx.$count; + const ctx_r18 = $r3$.ɵɵnextContext(); + const ɵ$index_2_r9 = ctx_r18.$index; + const ɵ$count_2_r11 = ctx_r18.$count; + const ctx_r19 = $r3$.ɵɵnextContext(); + const ɵ$index_1_r3 = ctx_r19.$index; + const ɵ$count_1_r5 = ctx_r19.$count; + $r3$.ɵɵtextInterpolate4(" Innermost vars: ", $index_r14 % 2 !== 0, " ", $index_r14 % 2 === 0, " ", $index_r14 === 0, " ", $index_r14 === $count_r16 - 1, " "); + $r3$.ɵɵadvance(2); + $r3$.ɵɵtextInterpolate4(" Inner vars: ", ɵ$index_2_r9 % 2 !== 0, " ", ɵ$index_2_r9 % 2 === 0, " ", ɵ$index_2_r9 === 0, " ", ɵ$index_2_r9 === ɵ$count_2_r11 - 1, " "); + $r3$.ɵɵadvance(2); + $r3$.ɵɵtextInterpolate4(" Outer vars: ", ɵ$index_1_r3 % 2 !== 0, " ", ɵ$index_1_r3 % 2 === 0, " ", ɵ$index_1_r3 === 0, " ", ɵ$index_1_r3 === ɵ$count_1_r5 - 1, " "); + } +} + +function MyApp_For_1_For_2_Template(rf, ctx) { + … + if (rf & 2) { + const $index_r8 = ctx.$index; + const $count_r10 = ctx.$count; + const ctx_r20 = $r3$.ɵɵnextContext(); + const ɵ$index_1_r3 = ctx_r20.$index; + const ɵ$count_1_r5 = ctx_r20.$count; + const ctx_r6 = $r3$.ɵɵnextContext(); + $r3$.ɵɵtextInterpolate4(" Inner vars: ", $index_r8 % 2 !== 0, " ", $index_r8 % 2 === 0, " ", $index_r8 === 0, " ", $index_r8 === $count_r10 - 1, " "); + $r3$.ɵɵadvance(2); + $r3$.ɵɵtextInterpolate4(" Outer vars: ", ɵ$index_1_r3 % 2 !== 0, " ", ɵ$index_1_r3 % 2 === 0, " ", ɵ$index_1_r3 === 0, " ", ɵ$index_1_r3 === ɵ$count_1_r5 - 1, " "); + $r3$.ɵɵadvance(1); + $r3$.ɵɵrepeater(ctx_r6.items); + } +} +… +function MyApp_For_1_Template(rf, ctx) { + … + if (rf & 2) { + const $index_r2 = ctx.$index; + const $count_r4 = ctx.$count; + const ctx_r0 = $r3$.ɵɵnextContext(); + $r3$.ɵɵtextInterpolate4(" Outer vars: ", $index_r2 % 2 !== 0, " ", $index_r2 % 2 === 0, " ", $index_r2 === 0, " ", $index_r2 === $count_r4 - 1, " "); + $r3$.ɵɵadvance(1); + $r3$.ɵɵrepeater(ctx_r0.items); + } +} +… +function MyApp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 3, 4, null, null, $r3$.ɵɵrepeaterTrackByIdentity); + } + if (rf & 2) { + $r3$.ɵɵrepeater(ctx.items); + } +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_listener_computed_template_variables.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_listener_computed_template_variables.ts new file mode 100644 index 0000000000000..4073e334fff75 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_listener_computed_template_variables.ts @@ -0,0 +1,27 @@ +import {Component} from '@angular/core'; + +@Component({ + template: ` + @for (outer of items; track outer; let outerOdd = $odd, outerEven = $even, outerFirst = $first, outerLast = $last) { + + + @for (inner of items; track inner; let innerOdd = $odd, innerEven = $even, innerFirst = $first, innerLast = $last) { + + + + @for (innermost of items; track innermost; let innermostOdd = $odd, innermostEven = $even, innermostFirst = $first, innermostLast = $last) { + + + + } + } + } + `, +}) +export class MyApp { + items = []; + + outerCb(...args: unknown[]) {} + innerCb(...args: unknown[]) {} + innermostCb(...args: unknown[]) {} +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_listener_computed_template_variables_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_listener_computed_template_variables_template.js new file mode 100644 index 0000000000000..9bab0e095daf3 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_listener_computed_template_variables_template.js @@ -0,0 +1,95 @@ +function MyApp_For_1_For_2_For_3_Template(rf, ctx) { + if (rf & 1) { + const _r19 = $r3$.ɵɵgetCurrentView(); + $r3$.ɵɵelementStart(0, "button", 0); + $r3$.ɵɵlistener("click", function MyApp_For_1_For_2_For_3_Template_button_click_0_listener() { + const restoredCtx = $r3$.ɵɵrestoreView(_r19); + const $index_r14 = restoredCtx.$index; + const $count_r16 = restoredCtx.$count; + const ctx_r18 = $r3$.ɵɵnextContext(3); + return $r3$.ɵɵresetView(ctx_r18.innermostCb($index_r14 % 2 !== 0, $index_r14 % 2 === 0, $index_r14 === 0, $index_r14 === $count_r16 - 1)); + }); + $r3$.ɵɵelementEnd(); + $r3$.ɵɵelementStart(1, "button", 0); + $r3$.ɵɵlistener("click", function MyApp_For_1_For_2_For_3_Template_button_click_1_listener() { + $r3$.ɵɵrestoreView(_r19); + const ctx_r21 = $r3$.ɵɵnextContext(); + const ɵ$index_2_r9 = ctx_r21.$index; + const ɵ$count_2_r11 = ctx_r21.$count; + const ctx_r20 = $r3$.ɵɵnextContext(2); + return $r3$.ɵɵresetView(ctx_r20.innerCb(ɵ$index_2_r9 % 2 !== 0, ɵ$index_2_r9 % 2 === 0, ɵ$index_2_r9 === 0, ɵ$index_2_r9 === ɵ$count_2_r11 - 1)); + }); + $r3$.ɵɵelementEnd(); + $r3$.ɵɵelementStart(2, "button", 0); + $r3$.ɵɵlistener("click", function MyApp_For_1_For_2_For_3_Template_button_click_2_listener() { + $r3$.ɵɵrestoreView(_r19); + const ctx_r23 = $r3$.ɵɵnextContext(2); + const ɵ$index_1_r3 = ctx_r23.$index; + const ɵ$count_1_r5 = ctx_r23.$count; + const ctx_r22 = $r3$.ɵɵnextContext(); + return $r3$.ɵɵresetView(ctx_r22.outerCb(ɵ$index_1_r3 % 2 !== 0, ɵ$index_1_r3 % 2 === 0, ɵ$index_1_r3 === 0, ɵ$index_1_r3 === ɵ$count_1_r5 - 1)); + }); + $r3$.ɵɵelementEnd(); + } +} +… +function MyApp_For_1_For_2_Template(rf, ctx) { + if (rf & 1) { + const _r25 = $r3$.ɵɵgetCurrentView(); + $r3$.ɵɵelementStart(0, "button", 0); + $r3$.ɵɵlistener("click", function MyApp_For_1_For_2_Template_button_click_0_listener() { + const restoredCtx = $r3$.ɵɵrestoreView(_r25); + const $index_r8 = restoredCtx.$index; + const $count_r10 = restoredCtx.$count; + const ctx_r24 = $r3$.ɵɵnextContext(2); + return $r3$.ɵɵresetView(ctx_r24.innerCb($index_r8 % 2 !== 0, $index_r8 % 2 === 0, $index_r8 === 0, $index_r8 === $count_r10 - 1)); + }); + $r3$.ɵɵelementEnd(); + $r3$.ɵɵelementStart(1, "button", 0); + $r3$.ɵɵlistener("click", function MyApp_For_1_For_2_Template_button_click_1_listener() { + $r3$.ɵɵrestoreView(_r25); + const ctx_r27 = $r3$.ɵɵnextContext(); + const ɵ$index_1_r3 = ctx_r27.$index; + const ɵ$count_1_r5 = ctx_r27.$count; + const ctx_r26 = $r3$.ɵɵnextContext(); + return $r3$.ɵɵresetView(ctx_r26.outerCb(ɵ$index_1_r3 % 2 !== 0, ɵ$index_1_r3 % 2 === 0, ɵ$index_1_r3 === 0, ɵ$index_1_r3 === ɵ$count_1_r5 - 1)); + }); + $r3$.ɵɵelementEnd(); + $r3$.ɵɵrepeaterCreate(2, MyApp_For_1_For_2_For_3_Template, 3, 0, null, null, $r3$.ɵɵrepeaterTrackByIdentity); + } + if (rf & 2) { + const ctx_r6 = $r3$.ɵɵnextContext(2); + $r3$.ɵɵadvance(2); + $r3$.ɵɵrepeater(ctx_r6.items); + } +} +… +function MyApp_For_1_Template(rf, ctx) { + if (rf & 1) { + const _r29 = $r3$.ɵɵgetCurrentView(); + $r3$.ɵɵelementStart(0, "button", 0); + $r3$.ɵɵlistener("click", function MyApp_For_1_Template_button_click_0_listener() { + const restoredCtx = $r3$.ɵɵrestoreView(_r29); + const $index_r2 = restoredCtx.$index; + const $count_r4 = restoredCtx.$count; + const ctx_r28 = $r3$.ɵɵnextContext(); + return $r3$.ɵɵresetView(ctx_r28.outerCb($index_r2 % 2 !== 0, $index_r2 % 2 === 0, $index_r2 === 0, $index_r2 === $count_r4 - 1)); + }); + $r3$.ɵɵelementEnd(); + $r3$.ɵɵrepeaterCreate(1, MyApp_For_1_For_2_Template, 4, 0, null, null, $r3$.ɵɵrepeaterTrackByIdentity); + } + if (rf & 2) { + const ctx_r0 = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(1); + $r3$.ɵɵrepeater(ctx_r0.items); + } +} +… +function MyApp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 3, 0, null, null, $r3$.ɵɵrepeaterTrackByIdentity); + } + if (rf & 2) { + $r3$.ɵɵrepeater(ctx.items); + } +} diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 01da9d2fe8548..3854cadb8329a 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -266,7 +266,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver buildTemplateFunction( nodes: t.Node[], variables: t.Variable[], ngContentSelectorsOffset: number = 0, - i18n?: i18n.I18nMeta): o.FunctionExpr { + i18n?: i18n.I18nMeta, variableAliases?: Record): o.FunctionExpr { this._ngContentSelectorsOffset = ngContentSelectorsOffset; if (this._namespace !== R3.namespaceHTML) { @@ -274,7 +274,13 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } // Create variable bindings - variables.forEach(v => this.registerContextVariables(v)); + variables.forEach(v => { + const alias = variableAliases?.[v.name]; + this.registerContextVariables(v.name, v.value); + if (alias) { + this.registerContextVariables(alias, v.value); + } + }); // Initiate i18n context in case: // - this template has parent i18n context @@ -394,14 +400,14 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver return _ref; } - private registerContextVariables(variable: t.Variable) { + private registerContextVariables(name: string, value: string) { const scopedName = this._bindingScope.freshReferenceName(); const retrievalLevel = this.level; - const isDirect = variable.value === DIRECT_CONTEXT_REFERENCE; - const lhs = o.variable(variable.name + scopedName); + const isDirect = value === DIRECT_CONTEXT_REFERENCE; + const lhs = o.variable(name + scopedName); this._bindingScope.set( - retrievalLevel, variable.name, + retrievalLevel, name, scope => { // If we're at the top level and we're referring to the context variable directly, we // can do so through the implicit receiver, instead of renaming it. Note that this does @@ -439,7 +445,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver return [ // e.g. const $items$ = x(2) for direct context references and // const $item$ = x(2).$implicit for indirect ones. - lhs.set(isDirect ? rhs : rhs.prop(variable.value || IMPLICIT_REFERENCE)).toConstDecl() + lhs.set(isDirect ? rhs : rhs.prop(value || IMPLICIT_REFERENCE)).toConstDecl() ]; }); } @@ -934,7 +940,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver private prepareEmbeddedTemplateFn( children: t.Node[], contextNameSuffix: string, variables: t.Variable[] = [], - i18n?: i18n.I18nMeta) { + i18n?: i18n.I18nMeta, variableAliases?: Record) { const index = this.allocateDataSlot(); if (this.i18n && i18n) { @@ -957,7 +963,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this._nestedTemplateFns.push(() => { const templateFunctionExpr = visitor.buildTemplateFunction( children, variables, this._ngContentReservedSlots.length + this._ngContentSelectorsOffset, - i18n); + i18n, variableAliases); this.constantPool.statements.push(templateFunctionExpr.toDeclStmt(name)); if (visitor._ngContentReservedSlots.length) { this._ngContentReservedSlots.push(...visitor._ngContentReservedSlots); @@ -1531,7 +1537,16 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const {tagName, attrsExprs} = this.inferProjectionDataFromInsertionPoint(block); const primaryData = this.prepareEmbeddedTemplateFn( block.children, '_For', - [block.item, block.contextVariables.$index, block.contextVariables.$count]); + [block.item, block.contextVariables.$index, block.contextVariables.$count], undefined, { + // We need to provide level-specific versions of `$index` and `$count`, because + // they're used when deriving the remaining variables (`$odd`, `$even` etc.) while at the + // same time being available implicitly. Without these aliases, we wouldn't be able to + // access the `$index` of a parent loop from inside of a nested loop. + [block.contextVariables.$index.name]: + this.getLevelSpecificVariableName('$index', this.level + 1), + [block.contextVariables.$count.name]: + this.getLevelSpecificVariableName('$count', this.level + 1), + }); const {expression: trackByExpression, usesComponentInstance: trackByUsesComponentInstance} = this.createTrackByFunction(block); let emptyData: TemplateData|null = null; @@ -1580,26 +1595,48 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } private registerComputedLoopVariables(block: t.ForLoopBlock, bindingScope: BindingScope): void { - const indexLocalName = block.contextVariables.$index.name; - const countLocalName = block.contextVariables.$count.name; const level = bindingScope.bindingLevel; - bindingScope.set( - level, block.contextVariables.$odd.name, - scope => scope.get(indexLocalName)!.modulo(o.literal(2)).notIdentical(o.literal(0))); + bindingScope.set(level, block.contextVariables.$odd.name, (scope, retrievalLevel) => { + return this.getLevelSpecificForLoopVariable(block, scope, retrievalLevel, '$index') + .modulo(o.literal(2)) + .notIdentical(o.literal(0)); + }); + + bindingScope.set(level, block.contextVariables.$even.name, (scope, retrievalLevel) => { + return this.getLevelSpecificForLoopVariable(block, scope, retrievalLevel, '$index') + .modulo(o.literal(2)) + .identical(o.literal(0)); + }); + + bindingScope.set(level, block.contextVariables.$first.name, (scope, retrievalLevel) => { + return this.getLevelSpecificForLoopVariable(block, scope, retrievalLevel, '$index') + .identical(o.literal(0)); + }); - bindingScope.set( - level, block.contextVariables.$even.name, - scope => scope.get(indexLocalName)!.modulo(o.literal(2)).identical(o.literal(0))); + bindingScope.set(level, block.contextVariables.$last.name, (scope, retrievalLevel) => { + const index = this.getLevelSpecificForLoopVariable(block, scope, retrievalLevel, '$index'); + const count = this.getLevelSpecificForLoopVariable(block, scope, retrievalLevel, '$count'); + return index.identical(count.minus(o.literal(1))); + }); + } - bindingScope.set( - level, block.contextVariables.$first.name, - scope => scope.get(indexLocalName)!.identical(o.literal(0))); + private getLevelSpecificVariableName(name: string, level: number): string { + // We use the `ɵ` here to ensure that there are no name conflicts with user-defined variables. + return `ɵ${name}_${level}`; + } - bindingScope.set( - level, block.contextVariables.$last.name, - scope => - scope.get(indexLocalName)!.identical(scope.get(countLocalName)!.minus(o.literal(1)))); + /** + * Gets the name of a for loop variable at a specific binding level. This allows us to look + * up implicitly shadowed variables like `$index` and `$count` at a specific level. + */ + private getLevelSpecificForLoopVariable( + block: t.ForLoopBlock, scope: BindingScope, retrievalLevel: number, + name: keyof t.ForLoopBlockContext): o.Expression { + const scopeName = scope.bindingLevel === retrievalLevel ? + block.contextVariables[name].name : + this.getLevelSpecificVariableName(name, retrievalLevel); + return scope.get(scopeName)!; } private optimizeTrackByFunction(block: t.ForLoopBlock) { @@ -2223,7 +2260,7 @@ type DeclareLocalVarCallback = (scope: BindingScope, relativeLevel: number) => o * Function that is executed whenever a variable is referenced. It allows for the variable to be * renamed depending on its location. */ -type LocalVarRefCallback = (scope: BindingScope) => o.Expression; +type LocalVarRefCallback = (scope: BindingScope, retrievalLevel: number) => o.Expression; /** The prefix used to get a shared context in BindingScope's map. */ const SHARED_CONTEXT_KEY = '$$shared_ctx$$'; @@ -2302,7 +2339,7 @@ export class BindingScope implements LocalResolver { if (value.declareLocalCallback && !value.declare) { value.declare = true; } - return typeof value.lhs === 'function' ? value.lhs(this) : value.lhs; + return typeof value.lhs === 'function' ? value.lhs(this, value.retrievalLevel) : value.lhs; } current = current.parent; } @@ -2421,8 +2458,9 @@ export class BindingScope implements LocalResolver { const componentValue = this.map.get(SHARED_CONTEXT_KEY + 0)!; componentValue.declare = true; this.maybeRestoreView(); - const lhs = - typeof componentValue.lhs === 'function' ? componentValue.lhs(this) : componentValue.lhs; + const lhs = typeof componentValue.lhs === 'function' ? + componentValue.lhs(this, componentValue.retrievalLevel) : + componentValue.lhs; return name === DIRECT_CONTEXT_REFERENCE ? lhs : lhs.prop(name); }