fix(compiler): correctly intercept index in loop tracking function (#53604)

The for loop tracking function doesn't allow references to local template variables, aside from `$index` and the item which are passed in as parameters. We enforce this by rewriting all variable references to the components scope.

The problem is that the logic that rewrites the references first walks the view tree and then checks if the variable is `$index` or the item. This is problematic in nested for loops, because it'll find the `$index` of the parent.

These changes resolve the issue by checking for `$index` and the item first.

Fixes #53600.

PR Close #53604
This commit is contained in:
Kristiyan Kostadinov
2023-12-16 19:24:16 +01:00
committed by Jessica Janiuk
parent 513fee871e
commit de5c9ca8e9
5 changed files with 137 additions and 6 deletions
@@ -1926,3 +1926,54 @@ export declare class MyApp {
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, false, never>;
}
/****************************************************************************************************
* PARTIAL FILE: nested_for_tracking_function.js
****************************************************************************************************/
import { Component } from '@angular/core';
import * as i0 from "@angular/core";
export class MyApp {
constructor() {
this.items = [];
this.trackByGrandparent = (item, index) => index;
this.trackByParent = (item, index) => index;
this.trackByChild = (item, index) => index;
}
}
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 (grandparent of items; track trackByGrandparent(grandparent, $index)) {
@for (parent of grandparent.items; track trackByParent(parent, $index)) {
@for (child of parent.items; track trackByChild(child, $index)) {
}
}
}
`, isInline: true });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
type: Component,
args: [{
template: `
@for (grandparent of items; track trackByGrandparent(grandparent, $index)) {
@for (parent of grandparent.items; track trackByParent(parent, $index)) {
@for (child of parent.items; track trackByChild(child, $index)) {
}
}
}
`,
}]
}] });
/****************************************************************************************************
* PARTIAL FILE: nested_for_tracking_function.d.ts
****************************************************************************************************/
import * as i0 from "@angular/core";
export declare class MyApp {
items: any[];
trackByGrandparent: (item: any, index: number) => number;
trackByParent: (item: any, index: number) => number;
trackByChild: (item: any, index: number) => number;
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, false, never>;
}
@@ -617,6 +617,23 @@
}
],
"skipForTemplatePipeline": true
},
{
"description": "should generate tracking function in a nested for loop",
"inputFiles": [
"nested_for_tracking_function.ts"
],
"expectations": [
{
"files": [
{
"expected": "nested_for_tracking_function_template.js",
"generated": "nested_for_tracking_function.js"
}
],
"failureMessage": "Incorrect template"
}
]
}
]
}
@@ -0,0 +1,19 @@
import {Component} from '@angular/core';
@Component({
template: `
@for (grandparent of items; track trackByGrandparent(grandparent, $index)) {
@for (parent of grandparent.items; track trackByParent(parent, $index)) {
@for (child of parent.items; track trackByChild(child, $index)) {
}
}
}
`,
})
export class MyApp {
items: any[] = [];
trackByGrandparent = (item: any, index: number) => index;
trackByParent = (item: any, index: number) => index;
trackByChild = (item: any, index: number) => index;
}
@@ -0,0 +1,44 @@
function _forTrack0($index, $item) {
return this.trackByGrandparent($item, $index);
}
function _forTrack1($index, $item) {
return this.trackByParent($item, $index);
}
function _forTrack2($index, $item) {
return this.trackByChild($item, $index);
}
function MyApp_For_1_For_1_For_1_Template(rf, ctx) {}
function MyApp_For_1_For_1_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_For_1_For_1_Template, 0, 0, null, null, _forTrack2, true);
}
if (rf & 2) {
const $parent_r7$ = ctx.$implicit;
$r3$.ɵɵrepeater($parent_r7$.items);
}
}
function MyApp_For_1_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_For_1_Template, 2, 0, null, null, _forTrack1, true);
}
if (rf & 2) {
const $grandparent_r1$ = ctx.$implicit;
$r3$.ɵɵrepeater($grandparent_r1$.items);
}
}
…
function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 2, 0, null, null, _forTrack0, true);
}
if (rf & 2) {
$r3$.ɵɵrepeater(ctx.items);
}
}
@@ -2550,11 +2550,16 @@ export class BindingScope implements LocalResolver {
class TrackByBindingScope extends BindingScope {
private componentAccessCount = 0;
constructor(parentScope: BindingScope, private globalAliases: Record<string, string>) {
constructor(parentScope: BindingScope, private globalOverrides: Record<string, string>) {
super(parentScope.bindingLevel + 1, parentScope);
}
override get(name: string): o.Expression|null {
// Intercept any overridden globals.
if (this.globalOverrides.hasOwnProperty(name)) {
return o.variable(this.globalOverrides[name]);
}
let current: BindingScope|null = this.parent;
// Prevent accesses of template variables outside the `for` loop.
@@ -2565,11 +2570,6 @@ class TrackByBindingScope extends BindingScope {
current = current.parent;
}
// Intercept any aliased globals.
if (this.globalAliases[name]) {
return o.variable(this.globalAliases[name]);
}
// When the component scope is accessed, we redirect it through `this`.
this.componentAccessCount++;
return o.variable('this').prop(name);