Description
OS: Windows 10
Browsers: Chrome 75.0.3770.142 64-bit, Firefox 68.0.1
Angular 7.2.12
After upgrading to CoreUI 2.4.5 from an old v1.x version, I am experiencing significant performance issues across our entire app. The observed behavior is that typing fast in a text input is laggy; keeping a key pressed shows noticeable hiccups every 5 to 10 characters. All in all, it just gives the feeling of bad UX.
When profiling with Chrome, I noticed that a good deal of time is spent in a "Recalculate Style" step:
Switching to Event Log tab in Chrome Debugger while running on a debug version of our app, I noticed that every keypress event triggers a style recalculation. One can find the source of style changes by clicking Reveal link:
.
Source is AppSidebarNavLinkComponent; our app has 20+ sidebar link items and on every keypress, all of them schedule a style recalculation by removing and re-adding the same CSS classes:
Looking at the code, I believe the root cause is the refactoring done in this commit, as well as this older commit. Before those changes, the ngClass was bound to an inline expression, and it was converted to function calls e.g. getLinkClass or helper.getBadgeClass(item). All of these functions always return a new object reference when called, which makes Angular think - during every change detection cycle - that the ngClass attribute has changed. It then proceeds to remove all classes and re-add them back, even if the object structure is the same; verified this by debugging.
A possible solution is to maintain string arrays in each nav item component, and have the getLinkClass/getBadgeClass etc. functions return the same array instance every time is called. NgClass directive has an IterableDiffer to detect changes in the array elements, so clearing the array and re-adding the same items all over again should be okay as long as we keep the same array reference. I verified this and it does get rid of the Recalculate Style, solving the issue.
For example, in app-sidebar-nav-link.component.ts:
public getLinkClass() {
const disabled = this.isDisabled();
const classes = {
'nav-link': true,
'disabled': disabled,
'btn-link': disabled
};
if (this.hasVariant()) {
const variant = `nav-link-${this.item.variant}`;
classes[variant] = true;
}
return classes;
}
becomes:
private linkClasses: string[] = [];
public getLinkClass() {
this.linkClasses.splice(0);
this.linkClasses.push('nav-link');
if (this.isDisabled()) {
this.linkClasses.push('disabled');
this.linkClasses.push('btn-link');
}
if (this.hasVariant()) {
const variant = `nav-link-${this.item.variant}`;
this.linkClasses.push(variant);
}
return this.linkClasses;
}
You would also have to add array variables for badge/icon classes as well, and change SidebarNavHelper to return an array of class names, for the HTML to bind to:
public getIconClass() {
this.iconClasses.splice(0);
for (const iconClass of this.helper.getIconClass(this.item)) {
this.iconClasses.push(iconClass);
}
return this.iconClasses;
}
It kinda breaks the SidebarNavHelper idea and results in more code; other alternatives might be to introduce an inheritance tree for the app-sidebar-nav-xxx.component classes or to change the SidebarNavHelper to receive the classes array as input.
Let me know what you think. I am willing to fork and submit a pull request if the above changes look acceptable.