Skip to content

Commit

Permalink
feat(component): replace markDirty with custom TickScheduler (#3488)
Browse files Browse the repository at this point in the history
  • Loading branch information
markostanimirovic authored Aug 2, 2022
1 parent bb9071c commit 3fcd8af
Show file tree
Hide file tree
Showing 9 changed files with 172 additions and 63 deletions.
36 changes: 10 additions & 26 deletions modules/component/spec/core/render-scheduler.spec.ts
Original file line number Diff line number Diff line change
@@ -1,40 +1,24 @@
import * as angular from '@angular/core';
import { noop } from 'rxjs';
import { createRenderScheduler } from '../../src/core/render-scheduler';
import {
manualInstanceNgZone,
manualInstanceNoopNgZone,
MockChangeDetectorRef,
} from '../fixtures/fixtures';
import { NoopTickScheduler } from '../../src/core/tick-scheduler';
import { MockChangeDetectorRef } from '../fixtures/fixtures';

describe('createRenderScheduler', () => {
function setup(ngZone: angular.NgZone) {
function setup() {
const cdRef = new MockChangeDetectorRef();
const renderScheduler = createRenderScheduler({ ngZone, cdRef });
jest.spyOn(angular, 'ɵmarkDirty').mockImplementation(noop);
const tickScheduler = new NoopTickScheduler();
jest.spyOn(tickScheduler, 'schedule');
const renderScheduler = createRenderScheduler({ cdRef, tickScheduler });

return { cdRef, renderScheduler, markDirty: angular.ɵmarkDirty };
return { cdRef, renderScheduler, tickScheduler };
}

describe('schedule', () => {
it('should call markForCheck in zone-full mode', () => {
const { cdRef, renderScheduler, markDirty } = setup(manualInstanceNgZone);
it('should call cdRef.markForCheck and tickScheduler.schedule', () => {
const { cdRef, renderScheduler, tickScheduler } = setup();
renderScheduler.schedule();

expect(markDirty).toHaveBeenCalledTimes(0);
expect(cdRef.markForCheck).toHaveBeenCalledTimes(1);
});

it('should call markDirty in zone-less mode', () => {
const { cdRef, renderScheduler, markDirty } = setup(
manualInstanceNoopNgZone
);
renderScheduler.schedule();

expect(markDirty).toHaveBeenCalledWith(
(cdRef as unknown as { context: object }).context
);
expect(cdRef.markForCheck).toHaveBeenCalledTimes(0);
expect(tickScheduler.schedule).toHaveBeenCalledTimes(1);
});
});
});
90 changes: 90 additions & 0 deletions modules/component/spec/core/tick-scheduler.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import {
fakeAsync,
flushMicrotasks,
TestBed,
tick,
} from '@angular/core/testing';
import { ApplicationRef, NgZone } from '@angular/core';
import {
AnimationFrameTickScheduler,
NoopTickScheduler,
TickScheduler,
} from '../../src/core/tick-scheduler';
import { ngZoneMock, noopNgZoneMock } from '../fixtures/fixtures';

describe('TickScheduler', () => {
function setup(ngZone: unknown) {
TestBed.configureTestingModule({
providers: [{ provide: NgZone, useValue: ngZone }],
});
const tickScheduler = TestBed.inject(TickScheduler);
const appRef = TestBed.inject(ApplicationRef);
jest.spyOn(appRef, 'tick');

return { tickScheduler, appRef };
}

describe('when NgZone is provided', () => {
it('should initialize NoopTickScheduler', () => {
const { tickScheduler } = setup(ngZoneMock);
expect(tickScheduler instanceof NoopTickScheduler).toBe(true);
});
});

describe('when NgZone is not provided', () => {
// `fakeAsync` uses 16ms as `requestAnimationFrame` delay
const animationFrameDelay = 16;

it('should initialize AnimationFrameTickScheduler', () => {
const { tickScheduler } = setup(noopNgZoneMock);
expect(tickScheduler instanceof AnimationFrameTickScheduler).toBe(true);
});

it('should schedule tick using the animationFrameScheduler', fakeAsync(() => {
const { tickScheduler, appRef } = setup(noopNgZoneMock);

tickScheduler.schedule();

expect(appRef.tick).toHaveBeenCalledTimes(0);
tick(animationFrameDelay / 2);
expect(appRef.tick).toHaveBeenCalledTimes(0);
tick(animationFrameDelay / 2);
expect(appRef.tick).toHaveBeenCalledTimes(1);
}));

it('should coalesce multiple synchronous schedule calls', fakeAsync(() => {
const { tickScheduler, appRef } = setup(noopNgZoneMock);

tickScheduler.schedule();
tickScheduler.schedule();
tickScheduler.schedule();

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

it('should coalesce multiple schedule calls that are queued to the microtask queue', fakeAsync(() => {
const { tickScheduler, appRef } = setup(noopNgZoneMock);

queueMicrotask(() => tickScheduler.schedule());
queueMicrotask(() => tickScheduler.schedule());
queueMicrotask(() => tickScheduler.schedule());

flushMicrotasks();
expect(appRef.tick).toHaveBeenCalledTimes(0);
tick(animationFrameDelay);
expect(appRef.tick).toHaveBeenCalledTimes(1);
}));

it('should schedule multiple ticks for multiple asynchronous schedule calls', fakeAsync(() => {
const { tickScheduler, appRef } = setup(noopNgZoneMock);

setTimeout(() => tickScheduler.schedule(), 100);
setTimeout(() => tickScheduler.schedule(), 200);
setTimeout(() => tickScheduler.schedule(), 300);

tick(300 + animationFrameDelay);
expect(appRef.tick).toHaveBeenCalledTimes(3);
}));
});
});
12 changes: 12 additions & 0 deletions modules/component/spec/core/zone-helpers.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { isNgZone } from '../../src/core/zone-helpers';
import { ngZoneMock, noopNgZoneMock } from '../fixtures/fixtures';

describe('isNgZone', () => {
it('should return true with NgZone instance', () => {
expect(isNgZone(ngZoneMock)).toBe(true);
});

it('should return false with NoopNgZone instance', () => {
expect(isNgZone(noopNgZoneMock)).toBe(false);
});
});
9 changes: 2 additions & 7 deletions modules/component/spec/fixtures/fixtures.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
import { NgZone } from '@angular/core';
import { MockNoopNgZone } from './mock-noop-ng-zone';

/**
* this is not exposed as NgZone should never be exposed to get miss matched with the real one
*/
class NoopNgZone extends MockNoopNgZone {}

export const manualInstanceNgZone = new NgZone({
export const ngZoneMock = new NgZone({
enableLongStackTrace: false,
shouldCoalesceEventChangeDetection: false,
});
export const manualInstanceNoopNgZone = new NoopNgZone({
export const noopNgZoneMock = new MockNoopNgZone({
enableLongStackTrace: false,
shouldCoalesceEventChangeDetection: false,
});
Expand Down
29 changes: 5 additions & 24 deletions modules/component/src/core/render-scheduler.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,22 @@
import {
ChangeDetectorRef,
NgZone,
ɵmarkDirty as markDirty,
} from '@angular/core';
import { ChangeDetectorRef } from '@angular/core';
import { TickScheduler } from './tick-scheduler';

export interface RenderScheduler {
schedule(): void;
}

export interface RenderSchedulerConfig {
ngZone: NgZone;
cdRef: ChangeDetectorRef;
tickScheduler: TickScheduler;
}

export function createRenderScheduler(
config: RenderSchedulerConfig
): RenderScheduler {
function schedule(): void {
if (hasZone(config.ngZone)) {
config.cdRef.markForCheck();
} else {
const context = getCdRefContext(config.cdRef);
markDirty(context);
}
config.cdRef.markForCheck();
config.tickScheduler.schedule();
}

return { schedule };
}

/**
* @description
* Determines if the application uses `NgZone` or `NgNoopZone` as ngZone service instance.
*/
function hasZone(z: NgZone): boolean {
return z instanceof NgZone;
}

function getCdRefContext(cdRef: ChangeDetectorRef): object {
return (cdRef as unknown as { context: object }).context;
}
42 changes: 42 additions & 0 deletions modules/component/src/core/tick-scheduler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { ApplicationRef, inject, Injectable, NgZone } from '@angular/core';
import { animationFrameScheduler } from 'rxjs';
import { isNgZone } from './zone-helpers';

@Injectable({
providedIn: 'root',
useFactory: () => {
const zone = inject(NgZone);
return isNgZone(zone)
? new NoopTickScheduler()
: inject(AnimationFrameTickScheduler);
},
})
export abstract class TickScheduler {
abstract schedule(): void;
}

@Injectable({
providedIn: 'root',
})
export class AnimationFrameTickScheduler extends TickScheduler {
private isScheduled = false;

constructor(private readonly appRef: ApplicationRef) {
super();
}

schedule(): void {
if (!this.isScheduled) {
this.isScheduled = true;
animationFrameScheduler.schedule(() => {
this.appRef.tick();
this.isScheduled = false;
});
}
}
}

export class NoopTickScheduler extends TickScheduler {
// eslint-disable-next-line @typescript-eslint/no-empty-function
schedule(): void {}
}
5 changes: 5 additions & 0 deletions modules/component/src/core/zone-helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { NgZone } from '@angular/core';

export function isNgZone(zone: unknown): zone is NgZone {
return zone instanceof NgZone;
}
6 changes: 3 additions & 3 deletions modules/component/src/let/let.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {
Directive,
ErrorHandler,
Input,
NgZone,
OnDestroy,
OnInit,
TemplateRef,
Expand All @@ -16,6 +15,7 @@ import {
} from '../core/potential-observable';
import { createRenderScheduler } from '../core/render-scheduler';
import { createRenderEventManager } from '../core/render-event/manager';
import { TickScheduler } from '../core/tick-scheduler';

type LetViewContextValue<PO> = PO extends ObservableOrPromise<infer V> ? V : PO;

Expand Down Expand Up @@ -115,8 +115,8 @@ export class LetDirective<PO> implements OnInit, OnDestroy {
$suspense: true,
};
private readonly renderScheduler = createRenderScheduler({
ngZone: this.ngZone,
cdRef: this.cdRef,
tickScheduler: this.tickScheduler,
});
private readonly renderEventManager = createRenderEventManager<
LetViewContextValue<PO>
Expand Down Expand Up @@ -183,7 +183,7 @@ export class LetDirective<PO> implements OnInit, OnDestroy {

constructor(
private readonly cdRef: ChangeDetectorRef,
private readonly ngZone: NgZone,
private readonly tickScheduler: TickScheduler,
private readonly mainTemplateRef: TemplateRef<LetViewContext<PO>>,
private readonly viewContainerRef: ViewContainerRef,
private readonly errorHandler: ErrorHandler
Expand Down
6 changes: 3 additions & 3 deletions modules/component/src/push/push.pipe.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {
ChangeDetectorRef,
ErrorHandler,
NgZone,
OnDestroy,
Pipe,
PipeTransform,
Expand All @@ -10,6 +9,7 @@ import { Unsubscribable } from 'rxjs';
import { ObservableOrPromise } from '../core/potential-observable';
import { createRenderScheduler } from '../core/render-scheduler';
import { createRenderEventManager } from '../core/render-event/manager';
import { TickScheduler } from '../core/tick-scheduler';

type PushPipeResult<PO> = PO extends ObservableOrPromise<infer R>
? R | undefined
Expand Down Expand Up @@ -40,8 +40,8 @@ type PushPipeResult<PO> = PO extends ObservableOrPromise<infer R>
export class PushPipe implements PipeTransform, OnDestroy {
private renderedValue: unknown;
private readonly renderScheduler = createRenderScheduler({
ngZone: this.ngZone,
cdRef: this.cdRef,
tickScheduler: this.tickScheduler,
});
private readonly renderEventManager = createRenderEventManager({
suspense: (event) => this.setRenderedValue(undefined, event.synchronous),
Expand All @@ -62,7 +62,7 @@ export class PushPipe implements PipeTransform, OnDestroy {

constructor(
private readonly cdRef: ChangeDetectorRef,
private readonly ngZone: NgZone,
private readonly tickScheduler: TickScheduler,
private readonly errorHandler: ErrorHandler
) {
this.subscription = this.renderEventManager
Expand Down

0 comments on commit 3fcd8af

Please sign in to comment.