From 9693f1f42b050c49031b5fc29598b4f5fce52e35 Mon Sep 17 00:00:00 2001 From: Junyoung Yang Date: Wed, 17 Apr 2024 23:22:45 +0900 Subject: [PATCH 1/9] feat(operators): add `mapResponse` #4230 --- modules/operators/spec/map-response.spec.ts | 79 +++++++++++++++++++++ modules/operators/src/index.ts | 1 + modules/operators/src/map-response.ts | 17 +++++ 3 files changed, 97 insertions(+) create mode 100644 modules/operators/spec/map-response.spec.ts create mode 100644 modules/operators/src/map-response.ts diff --git a/modules/operators/spec/map-response.spec.ts b/modules/operators/spec/map-response.spec.ts new file mode 100644 index 0000000000..aa01c0a993 --- /dev/null +++ b/modules/operators/spec/map-response.spec.ts @@ -0,0 +1,79 @@ +import { noop, Observable, of, throwError } from 'rxjs'; +import { mapResponse } from '..'; +import { concatMap, finalize } from 'rxjs/operators'; + +describe('mapResponse', () => { + it('should invoke next callback on next', () => { + const nextCallback = jest.fn(); + + of(1, 2, 3) + .pipe( + mapResponse({ + next: nextCallback, + error: noop, + }) + ) + .subscribe(); + + expect(nextCallback.mock.calls).toEqual([[1], [2], [3]]); + }); + + it('should invoke error callback on error', () => { + const errorCallback = jest.fn(); + const error = { message: 'error' }; + + throwError(() => error) + .pipe( + mapResponse({ + next: noop, + error: errorCallback, + }) + ) + .subscribe(); + + expect(errorCallback).toHaveBeenCalledWith(error); + }); + + it('should invoke error callback on the exception thrown in next', () => { + const errorCallback = jest.fn(); + const error = { message: 'error' }; + + function producesError() { + throw error; + } + + of(1) + .pipe( + mapResponse({ + next: producesError, + error: errorCallback, + }) + ) + .subscribe(); + + expect(errorCallback).toHaveBeenCalledWith(error); + }); + + it('should not unsubscribe from outer observable on inner observable error', () => { + const innerCompleteCallback = jest.fn(); + const outerCompleteCallback = jest.fn(); + + new Observable((subscriber) => subscriber.next(1)) + .pipe( + concatMap(() => + throwError(() => 'error').pipe( + mapResponse({ + next: noop, + error: noop, + }), + finalize(innerCompleteCallback) + ) + ), + finalize(outerCompleteCallback) + ) + .subscribe(); + + expect(innerCompleteCallback).toHaveBeenCalled(); + expect(outerCompleteCallback).not.toHaveBeenCalled(); + }); +}); diff --git a/modules/operators/src/index.ts b/modules/operators/src/index.ts index d4f9ce0ff7..97cf92f04e 100644 --- a/modules/operators/src/index.ts +++ b/modules/operators/src/index.ts @@ -1,2 +1,3 @@ export { concatLatestFrom } from './concat_latest_from'; +export { mapResponse } from './map-response'; export { tapResponse } from './tap-response'; diff --git a/modules/operators/src/map-response.ts b/modules/operators/src/map-response.ts new file mode 100644 index 0000000000..5e96659d49 --- /dev/null +++ b/modules/operators/src/map-response.ts @@ -0,0 +1,17 @@ +import { Observable, of } from 'rxjs'; +import { catchError, map } from 'rxjs/operators'; + +export function mapResponse(observerOrNext: { + next: (value: T) => R1; + error: (error: E) => R2; +}): (source$: Observable) => Observable { + return (source$) => + source$.pipe( + map((value) => { + return observerOrNext.next(value); + }), + catchError((error) => { + return of(observerOrNext.error(error)); + }) + ); +} From fec69e012a951428bfeb1830a344b50477a985dd Mon Sep 17 00:00:00 2001 From: Junyoung Yang Date: Wed, 17 Apr 2024 23:36:55 +0900 Subject: [PATCH 2/9] refactor(operators): add `MapResponseObserver` type --- modules/operators/src/map-response.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/modules/operators/src/map-response.ts b/modules/operators/src/map-response.ts index 5e96659d49..3d1b3d14c4 100644 --- a/modules/operators/src/map-response.ts +++ b/modules/operators/src/map-response.ts @@ -1,17 +1,21 @@ import { Observable, of } from 'rxjs'; import { catchError, map } from 'rxjs/operators'; -export function mapResponse(observerOrNext: { +type MapResponseObserver = { next: (value: T) => R1; error: (error: E) => R2; -}): (source$: Observable) => Observable { +}; + +export function mapResponse( + observer: MapResponseObserver +): (source$: Observable) => Observable { return (source$) => source$.pipe( map((value) => { - return observerOrNext.next(value); + return observer.next(value); }), catchError((error) => { - return of(observerOrNext.error(error)); + return of(observer.error(error)); }) ); } From 8065aadd3d6e5037e8c61712af9232c2443aa400 Mon Sep 17 00:00:00 2001 From: Junyoung Yang Date: Sun, 28 Apr 2024 09:58:46 +0900 Subject: [PATCH 3/9] refactor(operators): tidy up --- modules/operators/src/map-response.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/modules/operators/src/map-response.ts b/modules/operators/src/map-response.ts index 3d1b3d14c4..5e07f68a08 100644 --- a/modules/operators/src/map-response.ts +++ b/modules/operators/src/map-response.ts @@ -11,11 +11,7 @@ export function mapResponse( ): (source$: Observable) => Observable { return (source$) => source$.pipe( - map((value) => { - return observer.next(value); - }), - catchError((error) => { - return of(observer.error(error)); - }) + map((value) => observer.next(value)), + catchError((error) => of(observer.error(error))) ); } From 00e246ffbd267f0f8b980b162b9aab5af9fac33c Mon Sep 17 00:00:00 2001 From: Junyoung Yang Date: Sun, 28 Apr 2024 11:17:43 +0900 Subject: [PATCH 4/9] fix(operators): make test cases verity the results of `mapResponse` --- modules/operators/spec/map-response.spec.ts | 42 ++++++++++----------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/modules/operators/spec/map-response.spec.ts b/modules/operators/spec/map-response.spec.ts index aa01c0a993..cfa5267578 100644 --- a/modules/operators/spec/map-response.spec.ts +++ b/modules/operators/spec/map-response.spec.ts @@ -3,55 +3,51 @@ import { mapResponse } from '..'; import { concatMap, finalize } from 'rxjs/operators'; describe('mapResponse', () => { - it('should invoke next callback on next', () => { - const nextCallback = jest.fn(); + it('should map the emitted value using the next callback', () => { + const results: number[] = []; of(1, 2, 3) .pipe( mapResponse({ - next: nextCallback, + next: (value) => value + 1, error: noop, }) ) - .subscribe(); + .subscribe((result) => { + results.push(result as number); + }); - expect(nextCallback.mock.calls).toEqual([[1], [2], [3]]); + expect(results).toEqual([2, 3, 4]); }); - it('should invoke error callback on error', () => { - const errorCallback = jest.fn(); - const error = { message: 'error' }; - - throwError(() => error) + it('should map the thrown error using the error callback', () => { + throwError(() => 'error') .pipe( mapResponse({ next: noop, - error: errorCallback, + error: (error) => `mapped ${error}`, }) ) - .subscribe(); - - expect(errorCallback).toHaveBeenCalledWith(error); + .subscribe((result) => { + expect(result).toBe('mapped error'); + }); }); - it('should invoke error callback on the exception thrown in next', () => { - const errorCallback = jest.fn(); - const error = { message: 'error' }; - + it('should map the error thrown in next callback using error callback', () => { function producesError() { - throw error; + throw 'error'; } of(1) .pipe( mapResponse({ next: producesError, - error: errorCallback, + error: (error) => `mapped ${error}`, }) ) - .subscribe(); - - expect(errorCallback).toHaveBeenCalledWith(error); + .subscribe((result) => { + expect(result).toBe('mapped error'); + }); }); it('should not unsubscribe from outer observable on inner observable error', () => { From 7c4251516dee0767e81f2b9318a7bff1f39eb800 Mon Sep 17 00:00:00 2001 From: Junyoung Yang Date: Sun, 28 Apr 2024 11:29:38 +0900 Subject: [PATCH 5/9] refactor(operators): mark `error` parameter as unknown --- modules/operators/src/map-response.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/operators/src/map-response.ts b/modules/operators/src/map-response.ts index 5e07f68a08..cb4c95c9f8 100644 --- a/modules/operators/src/map-response.ts +++ b/modules/operators/src/map-response.ts @@ -1,14 +1,14 @@ import { Observable, of } from 'rxjs'; import { catchError, map } from 'rxjs/operators'; -type MapResponseObserver = { - next: (value: T) => R1; - error: (error: E) => R2; +type MapResponseObserver = { + next: (value: T) => S; + error: (error: unknown) => E; }; -export function mapResponse( - observer: MapResponseObserver -): (source$: Observable) => Observable { +export function mapResponse( + observer: MapResponseObserver +): (source$: Observable) => Observable { return (source$) => source$.pipe( map((value) => observer.next(value)), From 5f0db1a4575fa9038406e2b779da49f8da085107 Mon Sep 17 00:00:00 2001 From: Junyoung Yang Date: Mon, 29 Apr 2024 00:35:12 +0900 Subject: [PATCH 6/9] docs(operators): append `mapResponse` section --- .../content/guide/operators/operators.md | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/projects/ngrx.io/content/guide/operators/operators.md b/projects/ngrx.io/content/guide/operators/operators.md index ceb90ff437..ff94692d4a 100644 --- a/projects/ngrx.io/content/guide/operators/operators.md +++ b/projects/ngrx.io/content/guide/operators/operators.md @@ -90,3 +90,25 @@ There is also another signature of the `tapResponse` operator that accepts the o ); }); + +## mapResponse + +The `mapResponse` operator is particularly useful in scenarios where you need to transform data and handle potential errors with minimal boilerplate. + +In the example below, we use `mapResponse` within an NgRx effect to handle loading movies from an API. It demonstrates how to map successful API responses to an action indicating success, and how to handle errors by dispatching an error action. + + + loadMovies$ = createEffect(() => + this.actions$.pipe( + ofType('[Movies Page] Load Movies'), + exhaustMap(() => this.moviesService.getAll() + .pipe( + mapResponse({ + next: (movies) => ({ type: '[Movies API] Movies Loaded Success', payload: movies }), + error: () => of({ type: '[Movies API] Movies Loaded Error' }) + }) + ) + ) + ) + ); + From 997929c9ab7257398e0e96fcedca8887d0cb8a53 Mon Sep 17 00:00:00 2001 From: Junyoung Yang Date: Sat, 11 May 2024 23:33:43 +0900 Subject: [PATCH 7/9] fix(operators): call 'done' function in async test cases --- modules/operators/spec/map-response.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/operators/spec/map-response.spec.ts b/modules/operators/spec/map-response.spec.ts index cfa5267578..f43e4ffb11 100644 --- a/modules/operators/spec/map-response.spec.ts +++ b/modules/operators/spec/map-response.spec.ts @@ -20,7 +20,7 @@ describe('mapResponse', () => { expect(results).toEqual([2, 3, 4]); }); - it('should map the thrown error using the error callback', () => { + it('should map the thrown error using the error callback', (done) => { throwError(() => 'error') .pipe( mapResponse({ @@ -30,10 +30,11 @@ describe('mapResponse', () => { ) .subscribe((result) => { expect(result).toBe('mapped error'); + done(); }); }); - it('should map the error thrown in next callback using error callback', () => { + it('should map the error thrown in next callback using error callback', (done) => { function producesError() { throw 'error'; } @@ -47,6 +48,7 @@ describe('mapResponse', () => { ) .subscribe((result) => { expect(result).toBe('mapped error'); + done(); }); }); From 49b49bbf61e31af5e57c6df8a3c9bbc2fecc1f35 Mon Sep 17 00:00:00 2001 From: Junyoung Yang Date: Sat, 11 May 2024 23:36:19 +0900 Subject: [PATCH 8/9] docs(operators): add more details to the example code --- .../content/guide/operators/operators.md | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/projects/ngrx.io/content/guide/operators/operators.md b/projects/ngrx.io/content/guide/operators/operators.md index ff94692d4a..cfb41d91c9 100644 --- a/projects/ngrx.io/content/guide/operators/operators.md +++ b/projects/ngrx.io/content/guide/operators/operators.md @@ -98,17 +98,21 @@ The `mapResponse` operator is particularly useful in scenarios where you need to In the example below, we use `mapResponse` within an NgRx effect to handle loading movies from an API. It demonstrates how to map successful API responses to an action indicating success, and how to handle errors by dispatching an error action. - loadMovies$ = createEffect(() => - this.actions$.pipe( - ofType('[Movies Page] Load Movies'), - exhaustMap(() => this.moviesService.getAll() - .pipe( - mapResponse({ - next: (movies) => ({ type: '[Movies API] Movies Loaded Success', payload: movies }), - error: () => of({ type: '[Movies API] Movies Loaded Error' }) - }) + export const loadMovies = createEffect( + (actions$ = inject(Actions), moviesService = inject(MoviesService)) => { + return actions$.pipe( + ofType(MoviesPageActions.opened), + exhaustMap(() => + moviesService.getAll().pipe( + mapResponse({ + next: (movies) => MoviesApiActions.moviesLoadedSuccess({ movies }), + error: (error: { message: string }) => + MoviesApiActions.moviesLoadedFailure({ errorMsg: error.message }), + }) + ) ) - ) - ) + ); + }, + { functional: true } ); From f09ef970c94b6c0b120fdf515d3df881a4e9555e Mon Sep 17 00:00:00 2001 From: Junyoung Yang Date: Sat, 11 May 2024 23:36:55 +0900 Subject: [PATCH 9/9] Revert "refactor(operators): mark `error` parameter as unknown" This reverts commit 8142313fadd18359cc8aa675a43caa1812536e7d. --- modules/operators/src/map-response.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/operators/src/map-response.ts b/modules/operators/src/map-response.ts index cb4c95c9f8..5e07f68a08 100644 --- a/modules/operators/src/map-response.ts +++ b/modules/operators/src/map-response.ts @@ -1,14 +1,14 @@ import { Observable, of } from 'rxjs'; import { catchError, map } from 'rxjs/operators'; -type MapResponseObserver = { - next: (value: T) => S; - error: (error: unknown) => E; +type MapResponseObserver = { + next: (value: T) => R1; + error: (error: E) => R2; }; -export function mapResponse( - observer: MapResponseObserver -): (source$: Observable) => Observable { +export function mapResponse( + observer: MapResponseObserver +): (source$: Observable) => Observable { return (source$) => source$.pipe( map((value) => observer.next(value)),