diff --git a/modules/component/spec/core/render-event/handlers.spec.ts b/modules/component/spec/core/render-event/handlers.spec.ts index 64f1c2b91a..65b7ce3128 100644 --- a/modules/component/spec/core/render-event/handlers.spec.ts +++ b/modules/component/spec/core/render-event/handlers.spec.ts @@ -30,6 +30,7 @@ describe('combineRenderEventHandlers', () => { const suspenseEvent: SuspenseRenderEvent = { type: 'suspense', reset: true, + synchronous: true, }; testRenderEvent(suspenseEvent); @@ -37,6 +38,7 @@ describe('combineRenderEventHandlers', () => { type: 'next', value: 1, reset: true, + synchronous: false, }; testRenderEvent(nextEvent); @@ -44,12 +46,14 @@ describe('combineRenderEventHandlers', () => { type: 'error', error: 'ERROR!', reset: false, + synchronous: true, }; testRenderEvent(errorEvent); const completeEvent: CompleteRenderEvent = { type: 'complete', reset: false, + synchronous: false, }; testRenderEvent(completeEvent); }); diff --git a/modules/component/spec/core/render-event/manager.spec.ts b/modules/component/spec/core/render-event/manager.spec.ts index 061c34c766..d345446ad0 100644 --- a/modules/component/spec/core/render-event/manager.spec.ts +++ b/modules/component/spec/core/render-event/manager.spec.ts @@ -3,6 +3,7 @@ import { BehaviorSubject, delay, EMPTY, + merge, NEVER, Observable, of, @@ -45,10 +46,30 @@ describe('createRenderEventManager', () => { type: 'next', value: 'ngrx', reset: true, + synchronous: true, }); }); - it('should call next handler without reset flag when another value is emitted', () => { + it('should call next handler with synchronous and without reset flag when another value is emitted synchronously', () => { + const { renderEventManager, nextHandler } = setup(); + + renderEventManager.nextPotentialObservable(of('angular', 'ngrx')); + expect(nextHandler.mock.calls[0][0]).toEqual({ + type: 'next', + value: 'angular', + reset: true, + synchronous: true, + }); + expect(nextHandler.mock.calls[1][0]).toEqual({ + type: 'next', + value: 'ngrx', + reset: false, + synchronous: true, + }); + expect(nextHandler).toHaveBeenCalledTimes(2); + }); + + it('should call next handler without reset and synchronous flags when another value is emitted asynchronously', () => { const { renderEventManager, nextHandler } = setup(); const subject = new BehaviorSubject('ngrx'); @@ -59,16 +80,39 @@ describe('createRenderEventManager', () => { type: 'next', value: 'ngrx', reset: true, + synchronous: true, }); expect(nextHandler.mock.calls[1][0]).toEqual({ type: 'next', value: 'angular', reset: false, + synchronous: false, }); expect(nextHandler).toHaveBeenCalledTimes(2); }); - it('should not call next handler second time when same value is emitted twice', fakeAsync(() => { + it('should not call next handler second time when same value is emitted twice synchronously', () => { + const { renderEventManager, nextHandler } = setup(); + + renderEventManager.nextPotentialObservable( + of('ngrx', 'component', 'component') + ); + expect(nextHandler.mock.calls[0][0]).toEqual({ + type: 'next', + value: 'ngrx', + reset: true, + synchronous: true, + }); + expect(nextHandler.mock.calls[1][0]).toEqual({ + type: 'next', + value: 'component', + reset: false, + synchronous: true, + }); + expect(nextHandler).toHaveBeenCalledTimes(2); + }); + + it('should not call next handler second time when same value is emitted twice asynchronously', fakeAsync(() => { const { renderEventManager, nextHandler } = setup(); const subject = new BehaviorSubject('angular'); @@ -82,16 +126,44 @@ describe('createRenderEventManager', () => { type: 'next', value: 'angular', reset: true, + synchronous: true, }); expect(nextHandler.mock.calls[1][0]).toEqual({ type: 'next', value: 'ngrx/component', reset: false, + synchronous: false, + }); + expect(nextHandler).toHaveBeenCalledTimes(2); + })); + + it('should not call next handler second time when same value is emitted first synchronously then asynchronously', fakeAsync(() => { + const { renderEventManager, nextHandler } = setup(); + + renderEventManager.nextPotentialObservable( + merge( + of('ngrx', 'component'), + timer(10).pipe(switchMap(() => of('component'))) + ) + ); + tick(10); + + expect(nextHandler.mock.calls[0][0]).toEqual({ + type: 'next', + value: 'ngrx', + reset: true, + synchronous: true, + }); + expect(nextHandler.mock.calls[1][0]).toEqual({ + type: 'next', + value: 'component', + reset: false, + synchronous: true, }); expect(nextHandler).toHaveBeenCalledTimes(2); })); - it('should call error handler with reset flag when error is emitted', () => { + it('should call error handler with reset and synchronous flags when error is emitted', () => { const { renderEventManager, errorHandler } = setup(); renderEventManager.nextPotentialObservable(throwError(() => 'ERROR!')); @@ -99,10 +171,39 @@ describe('createRenderEventManager', () => { type: 'error', error: 'ERROR!', reset: true, + synchronous: true, }); }); - it('should call error handler without reset flag when error is emitted as second event', () => { + it('should call error handler with synchronous and without reset flag when error is emitted synchronously as second event', () => { + const { renderEventManager, errorHandler, nextHandler } = + setup(); + + renderEventManager.nextPotentialObservable( + new Observable((subscriber) => { + subscriber.next(1); + subscriber.error('ERROR!!!'); + }) + ); + + expect(nextHandler).toHaveBeenCalledWith({ + type: 'next', + value: 1, + reset: true, + synchronous: true, + }); + expect(errorHandler).toHaveBeenCalledWith({ + type: 'error', + error: 'ERROR!!!', + reset: false, + synchronous: true, + }); + + expect(nextHandler).toHaveBeenCalledTimes(1); + expect(errorHandler).toHaveBeenCalledTimes(1); + }); + + it('should call error handler without reset and synchronous flags when error is emitted asynchronously as second event', () => { const { renderEventManager, errorHandler, nextHandler } = setup(); const subject = new BehaviorSubject(100); @@ -114,28 +215,53 @@ describe('createRenderEventManager', () => { type: 'next', value: 100, reset: true, + synchronous: true, }); expect(errorHandler).toHaveBeenCalledWith({ type: 'error', error: 'ERROR!', reset: false, + synchronous: false, }); expect(nextHandler).toHaveBeenCalledTimes(1); expect(errorHandler).toHaveBeenCalledTimes(1); }); - it('should call complete handler with reset flag when complete is emitted', () => { + it('should call complete handler with reset and synchronous flags when complete is emitted', () => { const { renderEventManager, completeHandler } = setup(); renderEventManager.nextPotentialObservable(EMPTY); expect(completeHandler).toHaveBeenCalledWith({ type: 'complete', reset: true, + synchronous: true, + }); + }); + + it('should call complete handler with synchronous and without reset flag when complete is emitted synchronously as second event', () => { + const { renderEventManager, nextHandler, completeHandler } = + setup(); + + renderEventManager.nextPotentialObservable(of(100)); + + expect(nextHandler).toHaveBeenCalledWith({ + type: 'next', + value: 100, + reset: true, + synchronous: true, }); + expect(completeHandler).toHaveBeenCalledWith({ + type: 'complete', + reset: false, + synchronous: true, + }); + + expect(nextHandler).toHaveBeenCalledTimes(1); + expect(completeHandler).toHaveBeenCalledTimes(1); }); - it('should call complete handler without reset flag when complete is emitted as second event', () => { + it('should call complete handler without reset and synchronous flags when complete is emitted asynchronously as second event', () => { const { renderEventManager, nextHandler, completeHandler } = setup(); const subject = new BehaviorSubject(true); @@ -147,10 +273,12 @@ describe('createRenderEventManager', () => { type: 'next', value: true, reset: true, + synchronous: true, }); expect(completeHandler).toHaveBeenCalledWith({ type: 'complete', reset: false, + synchronous: false, }); expect(nextHandler).toHaveBeenCalledTimes(1); @@ -168,6 +296,7 @@ describe('createRenderEventManager', () => { expect(suspenseHandler).toHaveBeenCalledWith({ type: 'suspense', reset: true, + synchronous: true, }); expect(nextHandler).not.toHaveBeenCalled(); @@ -177,6 +306,7 @@ describe('createRenderEventManager', () => { type: 'next', value: 'ngrx', reset: false, + synchronous: false, }); })); @@ -191,6 +321,7 @@ describe('createRenderEventManager', () => { expect(suspenseHandler).toHaveBeenCalledWith({ type: 'suspense', reset: true, + synchronous: true, }); expect(errorHandler).not.toHaveBeenCalled(); @@ -200,10 +331,11 @@ describe('createRenderEventManager', () => { type: 'error', error: 'ERROR!', reset: false, + synchronous: false, }); })); - it('should call complete handler immediately and error handler when error is emitted', fakeAsync(() => { + it('should call suspense handler immediately and complete handler when complete is emitted', fakeAsync(() => { const { renderEventManager, suspenseHandler, completeHandler } = setup(); @@ -214,6 +346,7 @@ describe('createRenderEventManager', () => { expect(suspenseHandler).toHaveBeenCalledWith({ type: 'suspense', reset: true, + synchronous: true, }); expect(completeHandler).not.toHaveBeenCalled(); @@ -222,6 +355,7 @@ describe('createRenderEventManager', () => { expect(completeHandler).toHaveBeenCalledWith({ type: 'complete', reset: false, + synchronous: false, }); })); }); @@ -234,6 +368,7 @@ describe('createRenderEventManager', () => { expect(suspenseHandler).toHaveBeenCalledWith({ type: 'suspense', reset: true, + synchronous: true, }); }); }); @@ -248,7 +383,7 @@ describe('createRenderEventManager', () => { } describe('that emits first event synchronously', () => { - it('should call next handler with reset flag when first value is emitted', () => { + it('should call next handler with reset and synchronous flags when first value is emitted', () => { const { renderEventManager, nextHandler, @@ -263,6 +398,7 @@ describe('createRenderEventManager', () => { type: 'error', error: 'ERROR!', reset: true, + synchronous: true, }); // next observable @@ -270,10 +406,12 @@ describe('createRenderEventManager', () => { type: 'next', value: 100, reset: true, + synchronous: true, }); expect(completeHandler).toHaveBeenCalledWith({ type: 'complete', reset: false, + synchronous: true, }); expect(nextHandler).toHaveBeenCalledTimes(1); @@ -281,7 +419,44 @@ describe('createRenderEventManager', () => { expect(completeHandler).toHaveBeenCalledTimes(1); }); - it('should call next handler without reset flag when another value is emitted', () => { + it('should call next handler with synchronous and without reset flag when another value is emitted synchronously', () => { + const { renderEventManager, nextHandler } = withNextObservableSetup( + new BehaviorSubject(1) + ); + + renderEventManager.nextPotentialObservable( + new Observable((subscriber) => { + subscriber.next(10); + subscriber.next(100); + }) + ); + + // first observable + expect(nextHandler.mock.calls[0][0]).toEqual({ + type: 'next', + value: 1, + reset: true, + synchronous: true, + }); + + // next observable + expect(nextHandler.mock.calls[1][0]).toEqual({ + type: 'next', + value: 10, + reset: true, + synchronous: true, + }); + expect(nextHandler.mock.calls[2][0]).toEqual({ + type: 'next', + value: 100, + reset: false, + synchronous: true, + }); + + expect(nextHandler).toHaveBeenCalledTimes(3); + }); + + it('should call next handler without reset and synchronous flags when another value is emitted asynchronously', () => { const { renderEventManager, nextHandler } = withNextObservableSetup( new BehaviorSubject(1) ); @@ -295,6 +470,7 @@ describe('createRenderEventManager', () => { type: 'next', value: 1, reset: true, + synchronous: true, }); // next observable @@ -302,11 +478,13 @@ describe('createRenderEventManager', () => { type: 'next', value: 10, reset: true, + synchronous: true, }); expect(nextHandler.mock.calls[2][0]).toEqual({ type: 'next', value: 100, reset: false, + synchronous: false, }); expect(nextHandler).toHaveBeenCalledTimes(3); @@ -323,11 +501,12 @@ describe('createRenderEventManager', () => { type: 'next', value: 100, reset: true, + synchronous: true, }); expect(nextHandler).toHaveBeenCalledTimes(1); }); - it('should call error handler with reset flag when error is emitted', () => { + it('should call error handler with reset and synchronous flags when error is emitted', () => { const { renderEventManager, nextHandler, @@ -342,10 +521,12 @@ describe('createRenderEventManager', () => { type: 'next', value: 200, reset: true, + synchronous: true, }); expect(completeHandler).toHaveBeenCalledWith({ type: 'complete', reset: false, + synchronous: true, }); // next observable @@ -353,6 +534,7 @@ describe('createRenderEventManager', () => { type: 'error', error: 'ERROR!', reset: true, + synchronous: true, }); expect(nextHandler).toHaveBeenCalledTimes(1); @@ -372,6 +554,7 @@ describe('createRenderEventManager', () => { type: 'error', error: 'SAME_ERROR!', reset: true, + synchronous: true, }); expect(errorHandler).toHaveBeenCalledTimes(1); }); @@ -387,16 +570,19 @@ describe('createRenderEventManager', () => { type: 'next', value: 1, reset: true, + synchronous: true, }); expect(completeHandler.mock.calls[0][0]).toEqual({ type: 'complete', reset: false, + synchronous: true, }); // next observable expect(completeHandler.mock.calls[1][0]).toEqual({ type: 'complete', reset: true, + synchronous: true, }); expect(completeHandler).toHaveBeenCalledTimes(2); @@ -411,6 +597,7 @@ describe('createRenderEventManager', () => { expect(completeHandler).toHaveBeenCalledWith({ type: 'complete', reset: true, + synchronous: true, }); expect(completeHandler).toHaveBeenCalledTimes(1); }); @@ -430,12 +617,14 @@ describe('createRenderEventManager', () => { type: 'next', value: 'ngrx', reset: true, + synchronous: true, }); // next observable expect(suspenseHandler).toHaveBeenCalledWith({ type: 'suspense', reset: true, + synchronous: true, }); expect(nextHandler.mock.calls[1]).not.toBeDefined(); @@ -445,6 +634,7 @@ describe('createRenderEventManager', () => { type: 'next', value: 'component', reset: false, + synchronous: false, }); expect(suspenseHandler).toHaveBeenCalledTimes(1); @@ -468,12 +658,14 @@ describe('createRenderEventManager', () => { type: 'next', value: 'ngrx/component', reset: true, + synchronous: true, }); // next observable expect(suspenseHandler).toHaveBeenCalledWith({ type: 'suspense', reset: true, + synchronous: true, }); expect(errorHandler).not.toHaveBeenCalled(); @@ -483,6 +675,7 @@ describe('createRenderEventManager', () => { type: 'error', error: 'ERROR!', reset: false, + synchronous: false, }); expect(suspenseHandler).toHaveBeenCalledTimes(1); @@ -490,7 +683,7 @@ describe('createRenderEventManager', () => { expect(errorHandler).toHaveBeenCalledTimes(1); })); - it('should call complete handler immediately and error handler when error is emitted', fakeAsync(() => { + it('should call suspense handler immediately and complete handler when complete is emitted', fakeAsync(() => { const { renderEventManager, suspenseHandler, @@ -507,12 +700,14 @@ describe('createRenderEventManager', () => { type: 'next', value: false, reset: true, + synchronous: true, }); // next observable expect(suspenseHandler).toHaveBeenCalledWith({ type: 'suspense', reset: true, + synchronous: true, }); expect(completeHandler).not.toHaveBeenCalled(); @@ -521,6 +716,7 @@ describe('createRenderEventManager', () => { expect(completeHandler).toHaveBeenCalledWith({ type: 'complete', reset: false, + synchronous: false, }); expect(suspenseHandler).toHaveBeenCalledTimes(1); @@ -539,10 +735,12 @@ describe('createRenderEventManager', () => { type: 'next', value: 10, reset: true, + synchronous: true, }); expect(suspenseHandler).toHaveBeenCalledWith({ type: 'suspense', reset: true, + synchronous: true, }); }); @@ -555,6 +753,7 @@ describe('createRenderEventManager', () => { expect(suspenseHandler).toHaveBeenCalledWith({ type: 'suspense', reset: true, + synchronous: true, }); expect(suspenseHandler).toHaveBeenCalledTimes(1); }); diff --git a/modules/component/src/core/render-event/manager.ts b/modules/component/src/core/render-event/manager.ts index 5bcdd7c713..3f1f5d3e26 100644 --- a/modules/component/src/core/render-event/manager.ts +++ b/modules/component/src/core/render-event/manager.ts @@ -40,27 +40,29 @@ function switchMapToRenderEvent(): ( switchMap((potentialObservable) => { const observable$ = fromPotentialObservable(potentialObservable); let reset = true; + let synchronous = true; return new Observable>((subscriber) => { const subscription = observable$.subscribe({ next(value) { - subscriber.next({ type: 'next', value, reset }); + subscriber.next({ type: 'next', value, reset, synchronous }); reset = false; }, error(error) { - subscriber.next({ type: 'error', error, reset }); + subscriber.next({ type: 'error', error, reset, synchronous }); reset = false; }, complete() { - subscriber.next({ type: 'complete', reset }); + subscriber.next({ type: 'complete', reset, synchronous }); reset = false; }, }); if (reset) { - subscriber.next({ type: 'suspense', reset }); + subscriber.next({ type: 'suspense', reset, synchronous: true }); reset = false; } + synchronous = false; return subscription; }); diff --git a/modules/component/src/core/render-event/models.ts b/modules/component/src/core/render-event/models.ts index b805a4f04e..9947c316df 100644 --- a/modules/component/src/core/render-event/models.ts +++ b/modules/component/src/core/render-event/models.ts @@ -1,10 +1,18 @@ interface BaseRenderEvent { + /** + * true if the event is emitted by a new source + */ reset: boolean; + /** + * true if the synchronous event is emitted + */ + synchronous: boolean; } export interface SuspenseRenderEvent extends BaseRenderEvent { type: 'suspense'; reset: true; + synchronous: true; } export interface NextRenderEvent extends BaseRenderEvent { diff --git a/modules/component/src/let/let.directive.ts b/modules/component/src/let/let.directive.ts index 5d3b261056..9270c19f5d 100644 --- a/modules/component/src/let/let.directive.ts +++ b/modules/component/src/let/let.directive.ts @@ -140,7 +140,7 @@ export class LetDirective implements OnInit, OnDestroy { this.viewContext.$complete = false; } - this.renderMainView(); + this.renderMainView(event.synchronous); }, error: (event) => { this.viewContext.$error = event.error; @@ -152,7 +152,7 @@ export class LetDirective implements OnInit, OnDestroy { this.viewContext.$complete = false; } - this.renderMainView(); + this.renderMainView(event.synchronous); this.errorHandler.handleError(event.error); }, complete: (event) => { @@ -165,7 +165,7 @@ export class LetDirective implements OnInit, OnDestroy { this.viewContext.$error = undefined; } - this.renderMainView(); + this.renderMainView(event.synchronous); }, }); private readonly subscription = new Subscription(); @@ -206,7 +206,7 @@ export class LetDirective implements OnInit, OnDestroy { this.subscription.unsubscribe(); } - private renderMainView(): void { + private renderMainView(isSyncEvent: boolean): void { if (this.isSuspenseViewCreated) { this.isSuspenseViewCreated = false; this.viewContainerRef.clear(); @@ -220,7 +220,9 @@ export class LetDirective implements OnInit, OnDestroy { ); } - this.renderScheduler.schedule(); + if (!isSyncEvent) { + this.renderScheduler.schedule(); + } } private renderSuspenseView(): void { @@ -233,9 +235,5 @@ export class LetDirective implements OnInit, OnDestroy { this.isSuspenseViewCreated = true; this.viewContainerRef.createEmbeddedView(this.suspenseTemplateRef); } - - if (this.isMainViewCreated || this.isSuspenseViewCreated) { - this.renderScheduler.schedule(); - } } } diff --git a/modules/component/src/push/push.pipe.ts b/modules/component/src/push/push.pipe.ts index 21b5d59a11..7a7280c091 100644 --- a/modules/component/src/push/push.pipe.ts +++ b/modules/component/src/push/push.pipe.ts @@ -44,17 +44,17 @@ export class PushPipe implements PipeTransform, OnDestroy { cdRef: this.cdRef, }); private readonly renderEventManager = createRenderEventManager({ - suspense: () => this.setRenderedValue(undefined), - next: (event) => this.setRenderedValue(event.value), + suspense: (event) => this.setRenderedValue(undefined, event.synchronous), + next: (event) => this.setRenderedValue(event.value, event.synchronous), error: (event) => { if (event.reset) { - this.setRenderedValue(undefined); + this.setRenderedValue(undefined, event.synchronous); } this.errorHandler.handleError(event.error); }, complete: (event) => { if (event.reset) { - this.setRenderedValue(undefined); + this.setRenderedValue(undefined, event.synchronous); } }, }); @@ -79,10 +79,13 @@ export class PushPipe implements PipeTransform, OnDestroy { this.subscription.unsubscribe(); } - private setRenderedValue(value: unknown): void { + private setRenderedValue(value: unknown, isSyncEvent: boolean): void { if (value !== this.renderedValue) { this.renderedValue = value; - this.renderScheduler.schedule(); + + if (!isSyncEvent) { + this.renderScheduler.schedule(); + } } } }