Skip to content

Commit

Permalink
Merge pull request #371 from arturovt/fix/error-observer
Browse files Browse the repository at this point in the history
Fix: Do not run change detection if `error` does not have observers.
  • Loading branch information
pomek authored Jun 26, 2023
2 parents e4bf808 + 12370bb commit b82ce11
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 5 deletions.
85 changes: 84 additions & 1 deletion src/ckeditor/ckeditor.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { ComponentFixture } from '@angular/core/testing';
import { TestBed } from '@angular/core/testing';
import { CKEditorComponent } from './ckeditor.component';
import AngularEditor from '../../ckeditor/build/ckeditor';
import { SimpleChange } from '@angular/core';
import { ApplicationRef, Component, SimpleChange, ViewChild } from '@angular/core';

describe( 'CKEditorComponent', () => {
let component: CKEditorComponent;
Expand Down Expand Up @@ -493,6 +493,89 @@ describe( 'CKEditorComponent', () => {
} );
} );

describe( 'change detection', () => {
it( 'should NOT run change detection if `error` does not have listeners', async () => {
window.onerror = null;

@Component( {
template: '<ckeditor [editor]="editor"></ckeditor>'
} )
class TestComponent {
public editor = AngularEditor;

@ViewChild( CKEditorComponent, { static: true } ) public ckEditorComponent!: CKEditorComponent;
}

TestBed.configureTestingModule( {
declarations: [ TestComponent, CKEditorComponent ]
} );

const appRef = TestBed.inject( ApplicationRef );

const fixture = TestBed.createComponent( TestComponent );
fixture.detectChanges();

await waitCycle();

spyOn( appRef, 'tick' );

const oldEditor = fixture.componentInstance.ckEditorComponent.editorInstance;

const error: any = new Error( 'foo' );
error.is = () => true;
error.context = oldEditor;
Promise.resolve().then( () => {
window.dispatchEvent( new ErrorEvent( 'error', { error } ) );
} );

await waitCycle();

expect( appRef.tick ).toHaveBeenCalledTimes( 1 );
} );

it( 'should run change detection if `error` has listeners', async () => {
window.onerror = null;

@Component( {
template: '<ckeditor [editor]="editor" (error)="onError()"></ckeditor>'
} )
class TestComponent {
public editor = AngularEditor;

@ViewChild( CKEditorComponent, { static: true } ) public ckEditorComponent!: CKEditorComponent;

// eslint-disable-next-line @typescript-eslint/no-empty-function
public onError(): void {}
}

TestBed.configureTestingModule( {
declarations: [ TestComponent, CKEditorComponent ]
} );

const appRef = TestBed.inject( ApplicationRef );

const fixture = TestBed.createComponent( TestComponent );
fixture.detectChanges();

await waitCycle();

spyOn( appRef, 'tick' );

const oldEditor = fixture.componentInstance.ckEditorComponent.editorInstance;

const error: any = new Error( 'foo' );
error.is = () => true;
error.context = oldEditor;
Promise.resolve().then( () => {
window.dispatchEvent( new ErrorEvent( 'error', { error } ) );
} );

await waitCycle();

expect( appRef.tick ).toHaveBeenCalledTimes( 2 );
} );
} );

function waitCycle( time?: number ) {
return new Promise( res => {
setTimeout( res, time );
Expand Down
22 changes: 18 additions & 4 deletions src/ckeditor/ckeditor.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,13 @@ export class CKEditorComponent<TEditor extends Editor = Editor> implements After
};

const emitError = () => {
this.ngZone.run( () => {
this.error.emit();
} );
// Do not run change detection by re-entering the Angular zone if the `error`
// emitter doesn't have any subscribers.
// Subscribers are pushed onto the list whenever `error` is listened inside the template:
// `<ckeditor (error)="onError(...)"></ckeditor>`.
if ( hasObservers( this.error ) ) {
this.ngZone.run( () => this.error.emit() );
}
};

const element = document.createElement( this.tagName );
Expand Down Expand Up @@ -407,7 +411,11 @@ export class CKEditorComponent<TEditor extends Editor = Editor> implements After

this.editorWatchdog = editorWatchdog;

this.editorWatchdog.create( element, config );
this.ngZone.runOutsideAngular( () => {
// Note: must be called outside of the Angular zone too because `create` is calling
// `_startErrorHandling` within a microtask which sets up `error` listener on the window.
editorWatchdog.create( element, config );
} );
}
}

Expand Down Expand Up @@ -469,3 +477,9 @@ export class CKEditorComponent<TEditor extends Editor = Editor> implements After
} );
}
}

function hasObservers<T>( emitter: EventEmitter<T> ): boolean {
// Cast to `any` because `observed` property is available in RxJS >= 7.2.0.
// Fallback to checking `observers` list if this property is not defined.
return ( emitter as any ).observed ?? emitter.observers.length > 0;
}

0 comments on commit b82ce11

Please sign in to comment.