Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(component): use RenderScheduler as a service #3493

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions modules/component/spec/core/render-scheduler.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,40 @@
import { createRenderScheduler } from '../../src/core/render-scheduler';
import {
createRenderScheduler,
RenderScheduler,
} from '../../src/core/render-scheduler';
import { NoopTickScheduler } from '../../src/core/tick-scheduler';
import { MockChangeDetectorRef } from '../fixtures/fixtures';
import { TestBed } from '@angular/core/testing';
import { ChangeDetectorRef, Injectable } from '@angular/core';

describe('createRenderScheduler', () => {
it('should initialize within injection context', () => {
@Injectable({ providedIn: 'root' })
class Service {
readonly renderScheduler = createRenderScheduler();
}

TestBed.configureTestingModule({
providers: [
{ provide: ChangeDetectorRef, useClass: MockChangeDetectorRef },
],
});

const renderScheduler = TestBed.inject(Service).renderScheduler;
expect(renderScheduler instanceof RenderScheduler).toBe(true);
markostanimirovic marked this conversation as resolved.
Show resolved Hide resolved
});

it('should throw an error out of injection context', () => {
expect(() => createRenderScheduler()).toThrowError();
});
});

describe('RenderScheduler', () => {
function setup() {
const cdRef = new MockChangeDetectorRef();
const tickScheduler = new NoopTickScheduler();
jest.spyOn(tickScheduler, 'schedule');
const renderScheduler = createRenderScheduler({ cdRef, tickScheduler });
const renderScheduler = new RenderScheduler(cdRef, tickScheduler);

return { cdRef, renderScheduler, tickScheduler };
}
Expand Down
6 changes: 3 additions & 3 deletions modules/component/spec/types/push.pipe.types.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ describe('PushPipe', () => {
import { Observable } from 'rxjs';
import { PushPipe } from '@ngrx/component';

const anyVal = {} as any;
const pushPipe = new PushPipe(anyVal, anyVal, anyVal);
const value = pushPipe.transform(anyVal as ${potentialObservableType});
const unknownVal: unknown = {};
const pushPipe = unknownVal as PushPipe;
const value = pushPipe.transform(unknownVal as ${potentialObservableType});
`
);

Expand Down
29 changes: 13 additions & 16 deletions modules/component/src/core/render-scheduler.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
import { ChangeDetectorRef } from '@angular/core';
import { ChangeDetectorRef, inject, Injectable } from '@angular/core';
import { TickScheduler } from './tick-scheduler';

export interface RenderScheduler {
schedule(): void;
}

export interface RenderSchedulerConfig {
cdRef: ChangeDetectorRef;
tickScheduler: TickScheduler;
}
@Injectable()
export class RenderScheduler {
constructor(
private readonly cdRef: ChangeDetectorRef,
private readonly tickScheduler: TickScheduler
) {}

export function createRenderScheduler(
config: RenderSchedulerConfig
): RenderScheduler {
function schedule(): void {
config.cdRef.markForCheck();
config.tickScheduler.schedule();
schedule(): void {
this.cdRef.markForCheck();
this.tickScheduler.schedule();
}
}

return { schedule };
export function createRenderScheduler(): RenderScheduler {
return new RenderScheduler(inject(ChangeDetectorRef), inject(TickScheduler));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

18 changes: 7 additions & 11 deletions modules/component/src/let/let.directive.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
ChangeDetectorRef,
Directive,
ErrorHandler,
Input,
Expand All @@ -13,9 +12,8 @@ import {
ObservableOrPromise,
PotentialObservable,
} from '../core/potential-observable';
import { createRenderScheduler } from '../core/render-scheduler';
import { RenderScheduler } 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 @@ -103,7 +101,10 @@ export interface LetViewContext<PO> {
*
* @publicApi
*/
@Directive({ selector: '[ngrxLet]' })
@Directive({
selector: '[ngrxLet]',
providers: [RenderScheduler],
})
export class LetDirective<PO> implements OnInit, OnDestroy {
private isMainViewCreated = false;
private isSuspenseViewCreated = false;
Expand All @@ -114,10 +115,6 @@ export class LetDirective<PO> implements OnInit, OnDestroy {
$complete: false,
$suspense: true,
};
private readonly renderScheduler = createRenderScheduler({
cdRef: this.cdRef,
tickScheduler: this.tickScheduler,
});
private readonly renderEventManager = createRenderEventManager<
LetViewContextValue<PO>
>({
Expand Down Expand Up @@ -182,11 +179,10 @@ export class LetDirective<PO> implements OnInit, OnDestroy {
>;

constructor(
private readonly cdRef: ChangeDetectorRef,
private readonly tickScheduler: TickScheduler,
private readonly mainTemplateRef: TemplateRef<LetViewContext<PO>>,
private readonly viewContainerRef: ViewContainerRef,
private readonly errorHandler: ErrorHandler
private readonly errorHandler: ErrorHandler,
private readonly renderScheduler: RenderScheduler
) {}

static ngTemplateContextGuard<PO>(
Expand Down
20 changes: 3 additions & 17 deletions modules/component/src/push/push.pipe.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
import {
ChangeDetectorRef,
ErrorHandler,
OnDestroy,
Pipe,
PipeTransform,
} from '@angular/core';
import { ErrorHandler, OnDestroy, Pipe, PipeTransform } from '@angular/core';
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 @@ -39,10 +32,7 @@ type PushPipeResult<PO> = PO extends ObservableOrPromise<infer R>
@Pipe({ name: 'ngrxPush', pure: false })
export class PushPipe implements PipeTransform, OnDestroy {
private renderedValue: unknown;
private readonly renderScheduler = createRenderScheduler({
cdRef: this.cdRef,
tickScheduler: this.tickScheduler,
});
private readonly renderScheduler = createRenderScheduler();
private readonly renderEventManager = createRenderEventManager({
suspense: (event) => this.setRenderedValue(undefined, event.synchronous),
next: (event) => this.setRenderedValue(event.value, event.synchronous),
Expand All @@ -60,11 +50,7 @@ export class PushPipe implements PipeTransform, OnDestroy {
});
private readonly subscription: Unsubscribable;

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