Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(compiler): nested for loops incorrectly calculating computed vari…
…ables (#52931) 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
- Loading branch information
1 parent
2580ecc
commit 49dca36
Showing
7 changed files
with
431 additions
and
29 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
24 changes: 24 additions & 0 deletions
24
...liance/test_cases/r3_view_compiler_control_flow/nested_for_computed_template_variables.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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}} | ||
| <br> | ||
| 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}} | ||
| <br> | ||
| Inner vars: {{innerOdd}} {{innerEven}} {{innerFirst}} {{innerLast}} | ||
| <br> | ||
| Outer vars: {{outerOdd}} {{outerEven}} {{outerFirst}} {{outerLast}} | ||
| } | ||
| } | ||
| } | ||
| `, | ||
| }) | ||
| export class MyApp { | ||
| items = []; | ||
| } |
56 changes: 56 additions & 0 deletions
56
...st_cases/r3_view_compiler_control_flow/nested_for_computed_template_variables_template.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| } | ||
| } |
27 changes: 27 additions & 0 deletions
27
...st_cases/r3_view_compiler_control_flow/nested_for_listener_computed_template_variables.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) { | ||
| <button (click)="outerCb(outerOdd, outerEven, outerFirst, outerLast)"></button> | ||
| @for (inner of items; track inner; let innerOdd = $odd, innerEven = $even, innerFirst = $first, innerLast = $last) { | ||
| <button (click)="innerCb(innerOdd, innerEven, innerFirst, innerLast)"></button> | ||
| <button (click)="outerCb(outerOdd, outerEven, outerFirst, outerLast)"></button> | ||
| @for (innermost of items; track innermost; let innermostOdd = $odd, innermostEven = $even, innermostFirst = $first, innermostLast = $last) { | ||
| <button (click)="innermostCb(innermostOdd, innermostEven, innermostFirst, innermostLast)"></button> | ||
| <button (click)="innerCb(innerOdd, innerEven, innerFirst, innerLast)"></button> | ||
| <button (click)="outerCb(outerOdd, outerEven, outerFirst, outerLast)"></button> | ||
| } | ||
| } | ||
| } | ||
| `, | ||
| }) | ||
| export class MyApp { | ||
| items = []; | ||
|
|
||
| outerCb(...args: unknown[]) {} | ||
| innerCb(...args: unknown[]) {} | ||
| innermostCb(...args: unknown[]) {} | ||
| } |
95 changes: 95 additions & 0 deletions
95
...r3_view_compiler_control_flow/nested_for_listener_computed_template_variables_template.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| } | ||
| } |
Oops, something went wrong.