Skip to content

Commit

Permalink
fix(component): capture errors from observable when using ngrxPush
Browse files Browse the repository at this point in the history
…pipe and `ngrxLet` directive

Closes #3100
  • Loading branch information
maxime1992 authored Jul 28, 2021
1 parent b67af2f commit 23c846b
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 15 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ logs
*.log
.nyc
.nyc_output
.history

# Runtime data
pids
Expand Down
23 changes: 13 additions & 10 deletions modules/component/spec/core/cd-aware/cd-aware_creator.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { OnDestroy } from '@angular/core';
import { ErrorHandler, OnDestroy } from '@angular/core';
import {
concat,
EMPTY,
Expand All @@ -9,7 +9,6 @@ import {
throwError,
Unsubscribable,
} from 'rxjs';

import {
CdAware,
createCdAware,
Expand All @@ -35,14 +34,15 @@ class CdAwareImplementation<U> implements OnDestroy {
complete: () => (this.completed = true),
};

constructor() {
constructor(errorHandler: ErrorHandler) {
this.cdAware = createCdAware<U>({
render: createRender({
ngZone: manualInstanceNgZone,
cdRef: new MockChangeDetectorRef(),
}),
updateViewContextObserver: this.updateViewContextObserver,
resetContextObserver: this.resetContextObserver,
errorHandler,
});
this.subscription = this.cdAware.subscribe();
}
Expand All @@ -53,16 +53,19 @@ class CdAwareImplementation<U> implements OnDestroy {
}

let cdAwareImplementation: CdAwareImplementation<any>;
const setupCdAwareImplementation = () => {
cdAwareImplementation = new CdAwareImplementation();
const setupCdAwareImplementation = (errorHandler: ErrorHandler) => {
cdAwareImplementation = new CdAwareImplementation(errorHandler);
cdAwareImplementation.renderedValue = undefined;
cdAwareImplementation.error = undefined;
cdAwareImplementation.completed = false;
};

describe('CdAware', () => {
let errorHandlerCb: jest.Mock;

beforeEach(() => {
setupCdAwareImplementation();
errorHandlerCb = jest.fn();
setupCdAwareImplementation({ handleError: errorHandlerCb });
});

it('should be implementable', () => {
Expand Down Expand Up @@ -145,12 +148,12 @@ describe('CdAware', () => {
});

it('error handling', () => {
cdAwareImplementation.cdAware.nextPotentialObservable(
throwError('Error!')
);
const error = 'Error!';
cdAwareImplementation.cdAware.nextPotentialObservable(throwError(error));
expect(cdAwareImplementation.renderedValue).toBe(undefined);
expect(cdAwareImplementation.error).toBe('Error!');
expect(cdAwareImplementation.error).toBe(error);
expect(cdAwareImplementation.completed).toBe(false);
expect(errorHandlerCb).toHaveBeenCalledWith(error);
});

it('completion handling', () => {
Expand Down
3 changes: 3 additions & 0 deletions modules/component/src/core/cd-aware/cd-aware_creator.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ErrorHandler } from '@angular/core';
import {
EMPTY,
from,
Expand Down Expand Up @@ -35,6 +36,7 @@ export function createCdAware<U>(cfg: {
render: () => void;
resetContextObserver: NextObserver<void>;
updateViewContextObserver: NextObserver<U | undefined | null>;
errorHandler: ErrorHandler;
}): CdAware<U | undefined | null> {
const potentialObservablesSubject: Subject<
ObservableInput<U> | undefined | null
Expand Down Expand Up @@ -72,6 +74,7 @@ export function createCdAware<U>(cfg: {
tap(cfg.updateViewContextObserver),
tap(() => cfg.render()),
catchError((e) => {
cfg.errorHandler.handleError(e);
return EMPTY;
})
);
Expand Down
7 changes: 4 additions & 3 deletions modules/component/src/let/let.directive.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import {
ChangeDetectorRef,
Directive,
ErrorHandler,
Input,
NgZone,
OnDestroy,
TemplateRef,
ViewContainerRef,
} from '@angular/core';

import { NextObserver, ObservableInput, Observer, Unsubscribable } from 'rxjs';

import { CdAware, createCdAware } from '../core/cd-aware/cd-aware_creator';
import { createRender } from '../core/cd-aware/creator_render';

Expand Down Expand Up @@ -157,12 +156,14 @@ export class LetDirective<U> implements OnDestroy {
cdRef: ChangeDetectorRef,
ngZone: NgZone,
private readonly templateRef: TemplateRef<LetViewContext<U>>,
private readonly viewContainerRef: ViewContainerRef
private readonly viewContainerRef: ViewContainerRef,
errorHandler: ErrorHandler
) {
this.cdAware = createCdAware<U>({
render: createRender({ cdRef, ngZone }),
resetContextObserver: this.resetContextObserver,
updateViewContextObserver: this.updateViewContextObserver,
errorHandler,
});
this.subscription = this.cdAware.subscribe();
}
Expand Down
9 changes: 7 additions & 2 deletions modules/component/src/push/push.pipe.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {
ChangeDetectorRef,
ErrorHandler,
NgZone,
OnDestroy,
Pipe,
PipeTransform,
} from '@angular/core';
import { NextObserver, ObservableInput, Unsubscribable } from 'rxjs';

import { CdAware, createCdAware } from '../core/cd-aware/cd-aware_creator';
import { createRender } from '../core/cd-aware/creator_render';

Expand Down Expand Up @@ -68,11 +68,16 @@ export class PushPipe<S> implements PipeTransform, OnDestroy {
next: (value: S | null | undefined) => (this.renderedValue = value),
};

constructor(cdRef: ChangeDetectorRef, ngZone: NgZone) {
constructor(
cdRef: ChangeDetectorRef,
ngZone: NgZone,
errorHandler: ErrorHandler
) {
this.cdAware = createCdAware<S>({
render: createRender({ cdRef, ngZone }),
updateViewContextObserver: this.updateViewContextObserver,
resetContextObserver: this.resetContextObserver,
errorHandler,
});
this.subscription = this.cdAware.subscribe();
}
Expand Down

0 comments on commit 23c846b

Please sign in to comment.