perf(docs-infra): remove usage of the NgClass directive (angular#48312)

NgClass has non-negligable performance cost, especially if a different object is provided as NgClass input (which was the case for the NavItemComponent). The perfornace problem was noticed in the scope of the INP score investigations while profiling the https://angular.io/resources page. On this particular page replacing NgClass usage with alternatives results in 20x (!) runtime perf improvement. Such big improvement is possible since we avoid unneeded CSS classes removal / adding in the DOM. Part of angular#25518 PR Close angular#48312
saeidi-dev · Dec 2, 2022 · 6706fab · 6706fab
1 parent b53e1ed
commit 6706fab
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 60 deletions.
diff --git a/aio/src/app/app.component.html b/aio/src/app/app.component.html
@@ -54,7 +54,7 @@
 
 <mat-sidenav-container class="sidenav-container" [class.no-animations]="disableAnimations" [class.has-floating-toc]="hasFloatingToc">
 
-  <mat-sidenav [ngClass]="{'collapsed': !dockSideNav}" #sidenav class="sidenav" [mode]="mode" [opened]="isOpened" (openedChange)="updateHostClasses()">
+  <mat-sidenav [class.collapsed]="!dockSideNav" #sidenav class="sidenav" [mode]="mode" [opened]="isOpened" (openedChange)="updateHostClasses()">
     <aio-nav-menu *ngIf="!showTopMenu" [nodes]="topMenuNarrowNodes" [currentNode]="currentNodes.TopBarNarrow" [isWide]="dockSideNav" navLabel="primary"></aio-nav-menu>
     <aio-nav-menu [nodes]="sideNavNodes" [currentNode]="currentNodes.SideNav" [isWide]="dockSideNav" navLabel="guides and docs"></aio-nav-menu>
 

diff --git a/aio/src/app/custom-elements/code/code-example.component.spec.ts b/aio/src/app/custom-elements/code/code-example.component.spec.ts
@@ -43,20 +43,19 @@ describe('CodeExampleComponent', () => {
   it('should change aio-code classes based on header presence', () => {
     expect(codeExampleComponent.header).toBe('Great Example');
     expect(fixture.nativeElement.querySelector('header')).toBeTruthy();
-    expect(codeExampleComponent.classes).toEqual({
-      'headed-code': true,
-      'simple-code': false
-    });
+
+    const aioCodeEl = fixture.nativeElement.querySelector('aio-code');
+
+    expect(aioCodeEl).toHaveClass('headed-code');
+    expect(aioCodeEl).not.toHaveClass('simple-code');
 
     codeExampleComponent.header = '';
     fixture.detectChanges();
 
     expect(codeExampleComponent.header).toBe('');
     expect(fixture.nativeElement.querySelector('header')).toBeFalsy();
-    expect(codeExampleComponent.classes).toEqual({
-      'headed-code': false,
-      'simple-code': true
-    });
+    expect(aioCodeEl).not.toHaveClass('headed-code');
+    expect(aioCodeEl).toHaveClass('simple-code');
   });
 
   it('should set avoidFile class if path has .avoid.', () => {

diff --git a/aio/src/app/custom-elements/code/code-example.component.ts b/aio/src/app/custom-elements/code/code-example.component.ts
@@ -22,7 +22,8 @@ import { CodeComponent } from './code.component';
 
     <header *ngIf="header">{{header}}</header>
 
-    <aio-code [ngClass]="classes"
+    <aio-code [class.headed-code]="!!this.header"
+              [class.simple-code]="!this.header"
               [language]="language"
               [linenums]="linenums"
               [path]="path"
@@ -41,16 +42,7 @@ export class CodeExampleComponent implements AfterViewInit {
 
   @Input() region: string;
 
-  @Input()
-  set header(header: string) {
-    this._header = header;
-    this.classes = {
-      'headed-code': !!this.header,
-      'simple-code': !this.header,
-    };
-  }
-  get header(): string { return this._header; }
-  private _header: string;
+  @Input() header: string;
 
   @Input()
   set path(path: string) {

diff --git a/aio/src/app/layout/nav-item/nav-item.component.html b/aio/src/app/layout/nav-item/nav-item.component.html
@@ -1,25 +1,25 @@
 <div *ngIf="!node.children">
-  <a href="{{node.url}}" [ngClass]="classes" title="{{node.tooltip}}"
+  <a href="{{node.url}}" [class]="classes" title="{{node.tooltip}}"
     class="vertical-menu-item">
     <span class="vertical-menu-item-text">{{node.title}}</span>
   </a>
 </div>
 
 <div *ngIf="node.children">
-  <a *ngIf="node.url != null" href="{{node.url}}" [ngClass]="classes" title="{{node.tooltip}}"
+  <a *ngIf="node.url != null" href="{{node.url}}" [class]="classes" title="{{node.tooltip}}"
     (click)="headerClicked()" class="vertical-menu-item heading">
     <span>{{node.title}}</span>
     <mat-icon class="rotating-icon" svgIcon="keyboard_arrow_right"></mat-icon>
   </a>
 
-  <button *ngIf="node.url == null" type="button" [ngClass]="classes" title="{{node.tooltip}}"
+  <button *ngIf="node.url == null" type="button" [class]="classes" title="{{node.tooltip}}"
     (click)="headerClicked()" class="vertical-menu-item heading"
     [attr.aria-pressed]="isExpanded">
     <span>{{node.title}}</span>
     <mat-icon class="rotating-icon" svgIcon="keyboard_arrow_right"></mat-icon>
   </button>
 
-  <div class="heading-children" [ngClass]="classes">
+  <div class="heading-children" [class]="classes">
     <aio-nav-item *ngFor="let node of nodeChildren" [level]="level + 1" [isWide]="isWide"
     [isParentExpanded]="isExpanded"
     [node]="node" [selectedNodes]="selectedNodes"></aio-nav-item>

diff --git a/aio/src/app/layout/nav-item/nav-item.component.spec.ts b/aio/src/app/layout/nav-item/nav-item.component.spec.ts
@@ -15,7 +15,6 @@ describe('NavItemComponent', () => {
     let component: NavItemComponent;
 
     let selectedNodes: NavigationNode[];
-    let setClassesSpy: jasmine.Spy;
 
     function initialize(nd: NavigationNode) {
       component.node = nd;
@@ -30,7 +29,6 @@ describe('NavItemComponent', () => {
     beforeEach(() => {
 
       component = new NavItemComponent();
-      setClassesSpy = spyOn(component, 'setClasses').and.callThrough();
 
       // Selected nodes is the selected node and its header ancestors
       selectedNodes = [
@@ -44,25 +42,19 @@ describe('NavItemComponent', () => {
     describe('should have expected classes when initialized', () => {
       it('with selected node', () => {
         initialize(selectedNodes[0]);
-        expect(component.classes).toEqual(
-          // selected node should be expanded even if is a header.
-          { 'level-1': true, collapsed: false, expanded: true, selected: true }
-        );
+        // selected node should be expanded even if is a header.
+        expect(component.classes).toBe('level-1 expanded selected');
       });
 
       it('with selected node ancestor', () => {
         initialize(selectedNodes[1]);
-        expect(component.classes).toEqual(
-          // ancestor is a header and should be expanded
-          { 'level-1': true, collapsed: false, expanded: true, selected: true }
-        );
+        // ancestor is a header and should be expanded
+        expect(component.classes).toBe('level-1 expanded selected');
       });
 
       it('with other than a selected node or ancestor', () => {
         initialize({ title: 'x' });
-        expect(component.classes).toEqual(
-          { 'level-1': true, collapsed: true, expanded: false, selected: false }
-        );
+        expect(component.classes).toBe('level-1 collapsed');
       });
     });
 
@@ -161,9 +153,10 @@ describe('NavItemComponent', () => {
         expect(component.isSelected).withContext('remains not selected').toBe(false);
       });
 
-      it('should set classes', () => {
+      it('should set expanded classes', () => {
         component.headerClicked();
-        expect(setClassesSpy).toHaveBeenCalled();
+        expect(component.classes).toContain('expanded');
+        expect(component.classes).not.toContain('collapsed');
       });
     });
   });

diff --git a/aio/src/app/layout/nav-item/nav-item.component.ts b/aio/src/app/layout/nav-item/nav-item.component.ts
@@ -1,5 +1,5 @@
-import { Component, Input, OnChanges} from '@angular/core';
-import { NavigationNode } from 'app/navigation/navigation.model';
+import {Component, Input, OnChanges} from '@angular/core';
+import {NavigationNode} from 'app/navigation/navigation.model';
 
 @Component({
   selector: 'aio-nav-item',
@@ -10,41 +10,38 @@ export class NavItemComponent implements OnChanges {
   @Input() level = 1;
   @Input() node: NavigationNode;
   @Input() isParentExpanded = true;
-  @Input() selectedNodes: NavigationNode[] | undefined;
+  @Input() selectedNodes: NavigationNode[]|undefined;
 
   isExpanded = false;
   isSelected = false;
-  classes: {[index: string]: boolean };
+  classes: string;
   nodeChildren: NavigationNode[];
 
   ngOnChanges() {
-    this.nodeChildren = this.node && this.node.children ? this.node.children.filter(n => !n.hidden) : [];
+    this.nodeChildren =
+        this.node && this.node.children ? this.node.children.filter(n => !n.hidden) : [];
 
     if (this.selectedNodes) {
       const ix = this.selectedNodes.indexOf(this.node);
-      this.isSelected = ix !== -1; // this node is the selected node or its ancestor
+      this.isSelected = ix !== -1;  // this node is the selected node or its ancestor
       this.isExpanded = this.isParentExpanded &&
-        (this.isSelected || // expand if selected or ...
-        // preserve expanded state when display is wide; collapse in mobile.
-        (this.isWide && this.isExpanded));
+          (this.isSelected ||  // expand if selected or ...
+                               // preserve expanded state when display is wide; collapse in mobile.
+           (this.isWide && this.isExpanded));
     } else {
       this.isSelected = false;
     }
 
-    this.setClasses();
-  }
-
-  setClasses() {
-    this.classes = {
-      ['level-' + this.level]: true,
-      collapsed: !this.isExpanded,
-      expanded: this.isExpanded,
-      selected: this.isSelected
-    };
+    this._updateClasses();
   }
 
   headerClicked() {
     this.isExpanded = !this.isExpanded;
-    this.setClasses();
+    this._updateClasses();
+  }
+
+  private _updateClasses() {
+    this.classes = `level-${this.level} ${this.isExpanded ? 'expanded' : 'collapsed'}${
+        this.isSelected ? ' selected' : ''}`;
   }
 }
diff --git a/aio/src/app/layout/top-menu/top-menu.component.ts b/aio/src/app/layout/top-menu/top-menu.component.ts
@@ -6,7 +6,7 @@ import { CurrentNode, NavigationNode } from 'app/navigation/navigation.service';
   template: `
     <nav aria-label="primary">
       <ul>
-        <li *ngFor="let node of nodes" [ngClass]="{selected: node.url === currentUrl}">
+        <li *ngFor="let node of nodes" [class.selected]="node.url === currentUrl">
           <a class="nav-link" [href]="node.url" [title]="node.tooltip">
             <span class="nav-link-inner">{{ node.title }}</span>
           </a>