siemens / ix

Siemens Industrial Experience is a design system for designers and developers, to consistently create the perfect digital experience for industrial software products.
https://ix.siemens.io/
MIT License
207 stars 68 forks source link

Angular change detection and ix-tabs #846

Closed viktorhajer closed 1 year ago

viktorhajer commented 1 year ago

What happened?

It looks like the change detection mechanizm is triggered by the ix-tabs component continuously.

My comp.html: <ix-tabs></ix-tabs> <div *ngIf="isTabActive()"></div>

My comp.ts: @Component({ ... }) export class Component { isTabActive() { console.log(1); return true; } }

It starts writing 1 to the console continuously. Could you check it?

What type of frontend frameware are you seeing the problem on?

Angular

Which version of iX do you use?

v2.0.3

Code to produce this issue.

<ix-tabs></ix-tabs>
<div *ngIf="isTabActive()"></div>
danielleroux commented 1 year ago

You should not use function expression inside the angular template (https://medium.com/showpad-engineering/why-you-should-never-use-function-calls-in-angular-template-expressions-e1a50f9c0496).

tl;dr:

Because Angular cannot predict whether the return value of fullName() has changed, it needs to execute the function every time change detection runs.

Our angular wrappers uses the onPush mode https://github.com/siemens/ix/blob/main/packages/angular/src/components.ts#L1951

viktorhajer commented 1 year ago

Thank you, that article is very useful. But I'm still a little bit puzzled.

I removed the <ix-tabs> and after that I only see 30 logs in the console. But when I turn the <ix-tabs> on then the logs flow is endless.

So again: without ix-tabs it is triggered so many times, but it is okay. Angular is strong and optimized framework and we can also fine-tune it (e.g. using models instead of functions - although in some cases we simple can't...). BUT when we use ix-tabs the triggering is endless.

The updated html:
<button (click)="showTab = true;">Toggle: {{showTab}}</button> <ix-tabs *ngIf="showTab"></ix-tabs> <div *ngIf="isTabActive()"></div>

So it is crystal clear your component does something which triggers the change cycle.

As for the onPush: this applies to the component where it is set, but it can trigger the parent as I tested it. If we set the changeDetection to onPush in our components as well, it will solve the problem but in some cases it is just not working for us - the default behavior is needed...

If you have a special action which is continuously create events, you can use ngZone: https://angular.io/api/core/NgZone

danielleroux commented 1 year ago

Okay will have a look on it, but in your updated example you are using a function expression again in the angular template.

Maybe a found already a point which cannot be handled well by angular

danielleroux commented 1 year ago

Found the problem the component us the arequestAnimationFrame which leets in more change detections. Good call!

https://github.com/siemens/ix/blob/main/packages/core/src/components/tabs/tabs.tsx#L247

danielleroux commented 1 year ago

beta version for testing -> https://github.com/siemens/ix/releases/tag/v2.1.0-beta.0

viktorhajer commented 1 year ago

I couldn't repro the problem above with this version, so it looks fine for me, thank you :-)