🐞 - A tree renders all children(and children of all nested children) even if parents are not expanded.

[EricPoul]

Which @taiga-ui/* package(s) are the source of the bug?

kit

Please provide a link to a minimal reproduction of the bug

No response

Description

The tree should render children only if it's expanded. If my trees are made of an array of 3000 items, it will lead to a leak of a lot of memory and a colossal performance downgrade. Especially if I render it each time in a dropdown.

I guess the fix is as simple as it gets.

We need to add ngIf in tree.component.html

<tui-tree-item
    *tuiLet="children$ | async as children"
    #view
    [tuiTreeNode]="value"
    [isExpandable]="isExpandable"
>
    <div
        *ngIf="value !== children"
        polymorpheus-outlet
        [content]="content"
        [context]="{$implicit: value, node: view}"
    ></div>

     <ng-container *ngIf="item?.isExpanded" ngProjectAs="tui-tree">
        <tui-tree
           *ngFor="let child of children"
           [value]="child"
            [content]="content"
        ></tui-tree>
    </ng-container>
</tui-tree-item>

add a getter in tree.component.ts (because we have a handler to check whether it is expandable or not)

get isExpandable(): boolean {
    return !!this.handler(this.value).length;
  }

add an input to the tree-item.component.ts and remove @ContentChildren nested due to its needless.

Also I'd like to have an ability to pass trackBy to nested trees.

Angular version

13.1.3

Taiga UI version

2.34.0

Which browsers have you used?

• [X] Chrome
• [ ] Firefox
• [ ] Safari
• [ ] Edge

Which operating systems have you used?

• [X] macOS
• [ ] Windows
• [ ] Linux
• [ ] iOS
• [ ] Android

[EricPoul]

@waterplea do you have any thoughts about this one? I'm ready to make a pr if you have the same sight on this issue

[waterplea]

Yes, please, make a PR, this sounds like a good idea overall and the solutions looks fine. Not sure if ngProjectAs is necessary. Not a big fan of adding extra inputs but I need to take a look at the PR first :) As for trackBy, in my experience it is rather pointless with OnPush in most cases and only brings extra issues.

[EricPoul]

1. We have <ng-content select="tui-tree"></ng-content> in tui-tree-item so we need set ngProjectAs on a ng-template created by ngIf.
2. We can access a parent tui-tree via DI. I guess it's preferable.
3. We can have a situation with lots of items on the second level of the tree and we can remove only the last child(while filtering) thus we have almost the same tree and with trackBy the browser does not need to rerender its all children.

I'll get to it asap.

[waterplea]

1. It would probably still work since after ngIf turn to true it would have tui-tree inside content. Give it a shot 🙂
2. Yeah, probably. With @Optional.
3. Browser will not re-render children if they are equal by === even without trackBy. If you update your array, say, by items = items.filter(...) then it would reuse the same DOM elements and components it had for the items that remain in the array. You would only need trackBy if you reload the same items from the server request, for example, and the same objects in the array will not be equal to each other — then you could artificially tell Angular that they are considered equal if, for example, they have the same id property.

[EricPoul]

Yeah, you described it more correctly than I did. Anyway, it was just a suggestion for optimization. In most of the cases, it'll be excess, not everyone has 3k items in a tree.😄

[EricPoul]

It doesn't work without ngProjectAs. But we can make the next

 readonly children$ = this.check$.pipe(
        startWith(null),
        // was map(() => this.handler(this.value)),
        map(() => this.isExpandable ? this.handler(this.value) : []),
        distinctUntilChanged(),
    );

in this case, we do not add ng-container over ngFor and it works as before

[waterplea]

If it works I like it better :)

[waterplea]

I believe you meant this.isExpanded?

[EricPoul]

it's a new getter that shows whether it has children or not

[waterplea]

But it only returns true when there are children and false when there are not, the ternary doesn't make sense in this case 🤔

[EricPoul]

Oh, I messed up my previous changes, sorry, you're right

[EricPoul]

that`s the case

readonly children$ = this.check$.pipe(
        startWith(null),
        map(() => this.item?.isExpanded ? this.handler(this.value) : []),
        distinctUntilChanged(),
    );

[waterplea]

Go ahead with the PR and we will continue discussion there 🙂