fix(compiler): invoke method-based tracking function with context (#54960)

Previously we assumed that if a `for` loop tracking function is in the form of `someMethod($index, $item)`, it will be pure so we didn't pass the parameter to bind the context to it. This appears to be risky, because we don't know if the method is trying to access `this`.

These changes play it safe by always binding method-based tracking functions.

Fixes #53628.

PR Close #54960
This commit is contained in:
Kristiyan Kostadinov 2024-03-20 09:03:08 +01:00 committed by Dylan Hunn
parent 78d4ad20e6
commit 2b7bad5151
4 changed files with 26 additions and 2 deletions

View file

@ -1 +1 @@
$r3$.ɵɵrepeaterCreate(0, MyApp_ng_template_2_For_1_Template, 0, 0, null, null, $r3$.ɵɵcomponentInstance().trackFn);
$r3$.ɵɵrepeaterCreate(0, MyApp_ng_template_2_For_1_Template, 0, 0, null, null, $r3$.ɵɵcomponentInstance().trackFn, true);

View file

@ -1 +1 @@
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 0, 0, null, null, ctx.trackFn);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 0, 0, null, null, ctx.trackFn, true);

View file

@ -31,6 +31,10 @@ export function optimizeTrackFns(job: CompilationJob): void {
// Top-level access of the item uses the built in `repeaterTrackByIdentity`.
op.trackByFn = o.importExpr(Identifiers.repeaterTrackByIdentity);
} else if (isTrackByFunctionCall(job.root.xref, op.track)) {
// Mark the function as using the component instance to play it safe
// since the method might be using `this` internally (see #53628).
op.usesComponentInstance = true;
// Top-level method calls in the form of `fn($index, item)` can be passed in directly.
if (op.track.receiver.receiver.view === unit.xref) {
// TODO: this may be wrong

View file

@ -235,6 +235,26 @@ describe('control flow - for', () => {
fixture.detectChanges();
expect([...calls].sort()).toEqual(['a:hello', 'b:hello']);
});
it('should invoke method tracking function with the correct context', () => {
let context = null as TestComponent | null;
@Component({
template: `@for (item of items; track trackingFn($index, item)) {{{item}}}`,
})
class TestComponent {
items = ['a', 'b'];
trackingFn(_index: number, item: string) {
context = this;
return item;
}
}
const fixture = TestBed.createComponent(TestComponent);
fixture.detectChanges();
expect(context).toBe(fixture.componentInstance);
});
});
describe('list diffing and view operations', () => {