perf(module:typography): do not run change detection on input and keydown events (#7185)

arturovt · web-flow · commit ad547fb71939 · 2022-02-18T15:08:01.000+08:00
diff --git a/components/typography/text-edit.component.ts b/components/typography/text-edit.component.ts
@@ -3,6 +3,7 @@
  * found in the LICENSE file at https://github.com/NG-ZORRO/ng-zorro-antd/blob/master/LICENSE
  */
 
+import { ENTER, ESCAPE } from '@angular/cdk/keycodes';
 import {
   ChangeDetectionStrategy,
   ChangeDetectorRef,
@@ -11,15 +12,15 @@ import {
   EventEmitter,
   Input,
   NgZone,
-  OnDestroy,
   OnInit,
   Output,
   ViewChild,
   ViewEncapsulation
 } from '@angular/core';
-import { Subject } from 'rxjs';
-import { take, takeUntil } from 'rxjs/operators';
+import { BehaviorSubject, fromEvent, Observable } from 'rxjs';
+import { filter, switchMap, take, takeUntil, withLatestFrom } from 'rxjs/operators';
 
+import { NzDestroyService } from 'ng-zorro-antd/core/services';
 import { NzTSType } from 'ng-zorro-antd/core/types';
 import { NzI18nService, NzTextI18nInterface } from 'ng-zorro-antd/i18n';
 import { NzAutosizeDirective } from 'ng-zorro-antd/input';
@@ -28,70 +29,125 @@ import { NzAutosizeDirective } from 'ng-zorro-antd/input';
   selector: 'nz-text-edit',
   exportAs: 'nzTextEdit',
   template: `
-    <button
-      *ngIf="!editing"
-      nz-tooltip
-      nz-trans-button
-      class="ant-typography-edit"
-      [nzTooltipTitle]="tooltip === null ? null : tooltip || locale?.edit"
-      (click)="onClick()"
-    >
-      <ng-container *nzStringTemplateOutlet="icon; let icon">
-        <i nz-icon [nzType]="icon"></i>
-      </ng-container>
-    </button>
-    <ng-container *ngIf="editing">
-      <textarea
-        #textarea
-        nz-input
-        nzAutosize
-        (input)="onInput($event)"
-        (blur)="confirm()"
-        (keydown.esc)="onCancel()"
-        (keydown.enter)="onEnter($event)"
-      ></textarea>
+    <ng-template [ngIf]="editing" [ngIfElse]="notEditing">
+      <textarea #textarea nz-input nzAutosize (blur)="confirm()"></textarea>
       <button nz-trans-button class="ant-typography-edit-content-confirm" (click)="confirm()">
         <i nz-icon nzType="enter"></i>
       </button>
-    </ng-container>
+    </ng-template>
+
+    <ng-template #notEditing>
+      <button
+        nz-tooltip
+        nz-trans-button
+        class="ant-typography-edit"
+        [nzTooltipTitle]="tooltip === null ? null : tooltip || locale?.edit"
+        (click)="onClick()"
+      >
+        <ng-container *nzStringTemplateOutlet="icon; let icon">
+          <i nz-icon [nzType]="icon"></i>
+        </ng-container>
+      </button>
+    </ng-template>
   `,
   changeDetection: ChangeDetectionStrategy.OnPush,
   encapsulation: ViewEncapsulation.None,
-  preserveWhitespaces: false
+  preserveWhitespaces: false,
+  providers: [NzDestroyService]
 })
-export class NzTextEditComponent implements OnInit, OnDestroy {
+export class NzTextEditComponent implements OnInit {
   editing = false;
   locale!: NzTextI18nInterface;
-  private destroy$ = new Subject();
 
   @Input() text?: string;
   @Input() icon: NzTSType = 'edit';
   @Input() tooltip?: null | NzTSType;
   @Output() readonly startEditing = new EventEmitter<void>();
   @Output() readonly endEditing = new EventEmitter<string>(true);
-  @ViewChild('textarea', { static: false }) textarea!: ElementRef<HTMLTextAreaElement>;
+  @ViewChild('textarea', { static: false })
+  set textarea(textarea: ElementRef<HTMLTextAreaElement> | undefined) {
+    if (textarea) {
+      this.textarea$.next(textarea);
+    }
+  }
   @ViewChild(NzAutosizeDirective, { static: false }) autosizeDirective!: NzAutosizeDirective;
 
   beforeText?: string;
   currentText?: string;
   nativeElement = this.host.nativeElement;
+
+  // We could've saved the textarea within some private property (e.g. `_textarea`) and have a getter,
+  // but having subject makes the code more reactive and cancellable (e.g. event listeners will be
+  // automatically removed and re-added through the `switchMap` below).
+  private textarea$ = new BehaviorSubject<ElementRef<HTMLTextAreaElement> | null>(null);
+
   constructor(
-    private zone: NgZone,
-    private host: ElementRef,
+    private ngZone: NgZone,
+    private host: ElementRef<HTMLElement>,
     private cdr: ChangeDetectorRef,
-    private i18n: NzI18nService
+    private i18n: NzI18nService,
+    private destroy$: NzDestroyService
   ) {}
 
   ngOnInit(): void {
     this.i18n.localeChange.pipe(takeUntil(this.destroy$)).subscribe(() => {
       this.locale = this.i18n.getLocaleData('Text');
       this.cdr.markForCheck();
     });
-  }
 
-  ngOnDestroy(): void {
-    this.destroy$.next();
-    this.destroy$.complete();
+    const textarea$: Observable<ElementRef<HTMLTextAreaElement>> = this.textarea$.pipe(
+      filter((textarea): textarea is ElementRef<HTMLTextAreaElement> => textarea !== null)
+    );
+
+    textarea$
+      .pipe(
+        switchMap(
+          textarea =>
+            // Caretaker note: we explicitly should call `subscribe()` within the root zone.
+            // `runOutsideAngular(() => fromEvent(...))` will just create an observable within the root zone,
+            // but `addEventListener` is called when the `fromEvent` is subscribed.
+            new Observable<KeyboardEvent>(subscriber =>
+              this.ngZone.runOutsideAngular(() =>
+                fromEvent<KeyboardEvent>(textarea.nativeElement, 'keydown').subscribe(subscriber)
+              )
+            )
+        ),
+        takeUntil(this.destroy$)
+      )
+      .subscribe(event => {
+        // Caretaker note: adding modifier at the end (for instance `(keydown.esc)`) will tell Angular to add
+        // an event listener through the `KeyEventsPlugin`, which always runs `ngZone.runGuarded()` internally.
+        // We're interested only in escape and enter keyboard buttons, otherwise Angular will run change detection
+        // on any `keydown` event.
+        if (event.keyCode !== ESCAPE && event.keyCode !== ENTER) {
+          return;
+        }
+
+        this.ngZone.run(() => {
+          if (event.keyCode === ESCAPE) {
+            this.onCancel();
+          } else {
+            this.onEnter(event);
+          }
+          this.cdr.markForCheck();
+        });
+      });
+
+    textarea$
+      .pipe(
+        switchMap(
+          textarea =>
+            new Observable<KeyboardEvent>(subscriber =>
+              this.ngZone.runOutsideAngular(() =>
+                fromEvent<KeyboardEvent>(textarea.nativeElement, 'input').subscribe(subscriber)
+              )
+            )
+        ),
+        takeUntil(this.destroy$)
+      )
+      .subscribe(event => {
+        this.currentText = (event.target as HTMLTextAreaElement).value;
+      });
   }
 
   onClick(): void {
@@ -107,11 +163,6 @@ export class NzTextEditComponent implements OnInit, OnDestroy {
     this.endEditing.emit(this.currentText);
   }
 
-  onInput(event: Event): void {
-    const target = event.target as HTMLTextAreaElement;
-    this.currentText = target.value;
-  }
-
   onEnter(event: Event): void {
     event.stopPropagation();
     event.preventDefault();
@@ -124,13 +175,15 @@ export class NzTextEditComponent implements OnInit, OnDestroy {
   }
 
   focusAndSetValue(): void {
-    this.zone.onStable.pipe(take(1), takeUntil(this.destroy$)).subscribe(() => {
-      if (this.textarea?.nativeElement) {
-        this.textarea.nativeElement.focus();
-        this.textarea.nativeElement.value = this.currentText || '';
-        this.autosizeDirective.resizeToFitContent();
-        this.cdr.markForCheck();
-      }
-    });
+    this.ngZone.onStable
+      .pipe(take(1), withLatestFrom(this.textarea$), takeUntil(this.destroy$))
+      .subscribe(([, textarea]) => {
+        if (textarea) {
+          textarea.nativeElement.focus();
+          textarea.nativeElement.value = this.currentText || '';
+          this.autosizeDirective.resizeToFitContent();
+          this.cdr.markForCheck();
+        }
+      });
   }
 }
diff --git a/components/typography/typography.spec.ts b/components/typography/typography.spec.ts
@@ -1,20 +1,23 @@
-import { ENTER } from '@angular/cdk/keycodes';
+import { CAPS_LOCK, ENTER, ESCAPE, TAB } from '@angular/cdk/keycodes';
 import { OverlayContainer } from '@angular/cdk/overlay';
 import { CommonModule } from '@angular/common';
-import { Component, NgZone, ViewChild } from '@angular/core';
+import { ApplicationRef, Component, NgZone, ViewChild } from '@angular/core';
 import { ComponentFixture, fakeAsync, flush, inject, TestBed, tick } from '@angular/core/testing';
+import { By } from '@angular/platform-browser';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
 
 import {
   createKeyboardEvent,
   dispatchFakeEvent,
+  dispatchKeyboardEvent,
   dispatchMouseEvent,
   MockNgZone,
   typeInElement
 } from 'ng-zorro-antd/core/testing';
 import { NzSafeAny } from 'ng-zorro-antd/core/types';
 import { NzIconTestModule } from 'ng-zorro-antd/icon/testing';
 
+import { NzTextEditComponent } from '.';
 import { NzTypographyComponent } from './typography.component';
 import { NzTypographyModule } from './typography.module';
 
@@ -480,6 +483,67 @@ describe('typography', () => {
   });
 });
 
+// Caretaker note: this is moved to a separate `describe` block because the first `describe` block
+// mocks the `NgZone` with `MockNgZone`.
+describe('change detection behavior', () => {
+  let componentElement: HTMLElement;
+  let fixture: ComponentFixture<NzTestTypographyEditComponent>;
+
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      imports: [CommonModule, NzTypographyModule, NzIconTestModule, NoopAnimationsModule],
+      declarations: [NzTestTypographyEditComponent]
+    }).compileComponents();
+  });
+
+  beforeEach(() => {
+    fixture = TestBed.createComponent(NzTestTypographyEditComponent);
+    componentElement = fixture.debugElement.nativeElement;
+    fixture.detectChanges();
+  });
+
+  it('should not run change detection on `input` event', () => {
+    componentElement.querySelector<HTMLButtonElement>('.ant-typography-edit')!.click();
+    fixture.detectChanges();
+
+    const appRef = TestBed.inject(ApplicationRef);
+    spyOn(appRef, 'tick');
+
+    const nzTextEdit = fixture.debugElement.query(By.directive(NzTextEditComponent));
+    const textarea: HTMLTextAreaElement = nzTextEdit.nativeElement.querySelector('textarea');
+
+    textarea.value = 'some-value';
+    dispatchFakeEvent(textarea, 'input');
+
+    expect(appRef.tick).not.toHaveBeenCalled();
+    expect(nzTextEdit.componentInstance.currentText).toEqual('some-value');
+  });
+
+  it('should not run change detection on non-handled keydown events', done => {
+    componentElement.querySelector<HTMLButtonElement>('.ant-typography-edit')!.click();
+    fixture.detectChanges();
+
+    const ngZone = TestBed.inject(NgZone);
+    const appRef = TestBed.inject(ApplicationRef);
+    const spy = spyOn(appRef, 'tick');
+
+    const nzTextEdit = fixture.debugElement.query(By.directive(NzTextEditComponent));
+    const textarea: HTMLTextAreaElement = nzTextEdit.nativeElement.querySelector('textarea');
+
+    dispatchKeyboardEvent(textarea, 'keydown', TAB);
+    dispatchKeyboardEvent(textarea, 'keydown', CAPS_LOCK);
+
+    expect(spy).not.toHaveBeenCalled();
+
+    dispatchKeyboardEvent(textarea, 'keydown', ESCAPE);
+
+    ngZone.onMicrotaskEmpty.subscribe(() => {
+      expect(spy).toHaveBeenCalledTimes(1);
+      done();
+    });
+  });
+});
+
 @Component({
   template: `
     <h1 nz-typography>h1. Ant Design</h1>
