Passing a trackBy function as input doesn't work with new control flow

[dnbo]

Which @angular/* package(s) are the source of the bug?

compiler, core

Is this a regression?

Yes

Description

We have components that render content in an ngFor based on a structure such as

type ContentAndAdditionalData = {
    content: string;
} & {
    // Can be anything that somewhere has the correct value to track
};

where the items as well as the tracking function is passed through inputs.

This has been used like below with the old ngFor syntax

@Input() items: Array<...>;
@Input() trackByFn: () => number; 
---
*ngFor="let item of items; trackBy: trackByFn" 

With the new @for syntax, passing of a tracking function as an input is no longer possible as it yields the error
ERROR Error: trackByFn is not a function when done like

<div>
  @for(item of items; track trackByFn($index, item)) {
    <span>{{ item.id }}</span>
  }
</div>

This can be worked around by just tracking by reference as @for (item of items; track item) but I would still expect that passing a tracking function as an input should work, or is it no longer possible?

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/angular-uoqxab?file=src%2Fcontrol-flow-with-input%2Fcontrol-flow-with-input.component.html

Please provide the exception or error you saw

ERROR Error: trackByFn is not a function

Please provide the environment you discovered this bug in (run ng version)

_                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/
    

Angular CLI: 17.0.7
Node: 18.13.0
Package Manager: yarn 1.22.19
OS: darwin arm64

Angular: 17.0.7
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1700.7
@angular-devkit/build-angular   17.0.7
@angular-devkit/core            17.0.7
@angular-devkit/schematics      17.0.7
@schematics/angular             17.0.7
rxjs                            7.8.1
typescript                      5.2.2
zone.js                         0.14.2

Anything else?

No response

[JoostK]

The trackBy binding does not account for changes and is captured during creation mode, before the input has been assigned. The problem you're having can be avoided using a method that delegates to the input.

[pkozlowski-opensource]

Discussed this in the team and decided that we are not going to support it given that the case is not very common and a work-around exists. But we should detect this case and raise an error.

[pkozlowski-opensource]

Digging into it a bit more, I'm having second thoughts on the correct way of proceeding here. The thing is that it breaks due to the optimisation we've chosen to do, that is, the generated code seems to be different for the 2 following cases:

• @for(item of items; track trackFn($index, item)) will generate repeaterCreate(1, WithALoop_For_2_Template, 2, 1, "span", null, ctx.trackFn) - notice that here we pass a reference to a function assuming that it is defined on the context;
  @for(item of items; track trackFn($index, item) + 5) (notice the expression in the track by) will generate repeaterCreate(1, WithALoop_For_2_Template, 2, 1, "span", null, _forTrack0, true) where _forTrack0 is defined as:

function _forTrack0($index, $item) {
  return this.trackFn($index, $item) + 5;
}

(see https://stackblitz.com/edit/stackblitz-starters-xgaggq)

So the optimisation we've got is really good for common cases but I can see how it could get surprising. Essentially we are trading the "correctness" for perf here. I still do think that this is the right tradeoff but leaning towards keeping this issue open and see how many people bump into it.

[SuNNjek]

The trackBy binding does not account for changes and is captured during creation mode, before the input has been assigned. The problem you're having can be avoided using a method that delegates to the input.

Can you elaborate on what you mean by this, maybe give an example? I'm currently facing the same problem. I have a component that has a @for loop in its template and a TrackByFunction<T> called trackBy as an input. Inside the component code I have the following code:

@Input()
public trackBy?: TrackByFunction<T>;

protected trackItem(index: number, item: T): unknown {
    return this.trackBy?.(index, item) ?? index;
}

and in the template I call it like this:

@for (item of items; track trackItem(row, item); let lastItem = $last; let row = $index) {
    ...
}

But when I want to use the component, I just get the following error, indicating that this is undefined:

ERROR TypeError: Cannot read properties of undefined (reading 'trackBy')
    at trackItem (grid-table.component.ts:37:15)
    at createOrAttach (core.mjs:19662:73)
    at reconcile (core.mjs:19590:13)
    at Module.ɵɵrepeater (core.mjs:20517:9)
    at GridTableComponent_Template (grid-table.component.html:14:1)
    at executeTemplate (core.mjs:12159:9)
    at refreshView (core.mjs:13392:13)
    at detectChangesInView (core.mjs:13617:9)
    at detectChangesInViewIfAttached (core.mjs:13580:5)
    at detectChangesInComponent (core.mjs:13569:5)

[pkozlowski-opensource]

@SuNNjek what I was trying to explain is that with the current implementation a reference to the trackBy function must be defined when the component is created - which is much earlier, before @Inputs are set.

In your particular case your code is correct but you are bumping into a slightly separate issue - the this context is not present when the trackItem is invoked.

The combination of the two mentioned issues makes your code to fail in the end but you can work-around it by defining the trackItem function as an arrow function:

protected trackItem = <T>(index: number, item: T) => {
    return this.trackBy?.(index, item) ?? index;
  };

(live code at: https://stackblitz.com/edit/stackblitz-starters-ijejgx?file=src%2Fmain.ts)

Having said this I do recognise that this is very non-obvious behaviour and we will look into improving support for this scenario. For now the presented work-around should do the trick.