fix(compiler): invoke method-based tracking function with context (an…

…gular#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 angular#53628.

PR Close angular#54960

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_track_method_nested_template.js

@@ -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);

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_track_method_root_template.js

@@ -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);

packages/compiler/src/template/pipeline/src/phases/track_fn_optimization.ts

@@ -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

packages/core/test/acceptance/control_flow_for_spec.ts

@@ -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', () => {