From ad547fb7193999735396d12c07124c70d9941daa Mon Sep 17 00:00:00 2001 From: Artur Androsovych Date: Fri, 18 Feb 2022 09:08:01 +0200 Subject: [PATCH] perf(module:typography): do not run change detection on `input` and `keydown` events (#7185) --- components/typography/text-edit.component.ts | 153 +++++++++++++------ components/typography/typography.spec.ts | 68 ++++++++- 2 files changed, 169 insertions(+), 52 deletions(-) diff --git a/components/typography/text-edit.component.ts b/components/typography/text-edit.component.ts index de6fceccd2..1e54ecb54c 100644 --- 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,58 +29,64 @@ import { NzAutosizeDirective } from 'ng-zorro-antd/input'; selector: 'nz-text-edit', exportAs: 'nzTextEdit', 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(); @Output() readonly endEditing = new EventEmitter(true); - @ViewChild('textarea', { static: false }) textarea!: ElementRef; + @ViewChild('textarea', { static: false }) + set textarea(textarea: ElementRef | 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 | null>(null); + constructor( - private zone: NgZone, - private host: ElementRef, + private ngZone: NgZone, + private host: ElementRef, private cdr: ChangeDetectorRef, - private i18n: NzI18nService + private i18n: NzI18nService, + private destroy$: NzDestroyService ) {} ngOnInit(): void { @@ -87,11 +94,60 @@ export class NzTextEditComponent implements OnInit, OnDestroy { this.locale = this.i18n.getLocaleData('Text'); this.cdr.markForCheck(); }); - } - ngOnDestroy(): void { - this.destroy$.next(); - this.destroy$.complete(); + const textarea$: Observable> = this.textarea$.pipe( + filter((textarea): textarea is ElementRef => 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(subscriber => + this.ngZone.runOutsideAngular(() => + fromEvent(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(subscriber => + this.ngZone.runOutsideAngular(() => + fromEvent(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 index aa603509ba..a6c4fd425d 100644 --- a/components/typography/typography.spec.ts +++ b/components/typography/typography.spec.ts @@ -1,13 +1,15 @@ -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 @@ -15,6 +17,7 @@ import { 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; + + 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('.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('.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. Ant Design