feat(operators): add mapResponse #4230

[tom9744]

PR Checklist

• The commit message follows our guidelines: https://github.com/ngrx/platform/blob/main/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Documentation has been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

export const loadAllUsers = createEffect((
  actions$ = inject(Actions),
  usersService = inject(UsersService)
) => {
  return actions$.pipe(
    ofType(UsersPageActions.opened),
    exhaustMap(() => {
      return usersService.getAll().pipe(
        map((users) => UsersApiActions.usersLoadedSuccess({ users })),
        catchError((error) =>
          of(UsersApiActions.usersLoadedFailure({ error }))
        )
      );
    })
  );
});

Closes #4230

What is the new behavior?

import { mapResponse } from '@ngrx/operators';

export const loadAllUsers = createEffect((
  actions$ = inject(Actions),
  usersService = inject(UsersService)
) => {
  return actions$.pipe(
    ofType(UsersPageActions.opened),
    exhaustMap(() => {
      return usersService.getAll().pipe(
        mapResponse({
          next: (users) => UsersApiActions.usersLoadedSuccess({ users }),
          error: (error) => UsersApiActions.usersLoadedFailure({ error }),
        })
      );
    })
  );
});

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

I am not 100% sure if I correctly understood the expected behavior of mapResponse, as @markostanimirovic suggested in #4230.

Please feel free to give me feedback.

Thanks.

[netlify]

✅ Deploy Preview for ngrx-io ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	0c4c658
🔍 Latest deploy log	https://app.netlify.com/sites/ngrx-io/deploys/662e6c86dbd3a200087cd237
😎 Deploy Preview	https://deploy-preview-4302--ngrx-io.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[markostanimirovic]

Thanks for the PR @tom9744! In addition to the suggested changes, can you please add the mapResponse section to this page: https://ngrx.io/guide/operators/operators

It should contain the description and example of using the mapResponse operator. The example can be an NgRx effect + mapResponse.

[tom9744]

I appreciate your kind feedback on my work, @markostanimirovic!

I modified the code based on your suggestions, and am currently working on writing a document for ngrx.io.

[tom9744]

I thought it would make more sense to type error as unknown because we don't know what type the error can possibly be in various use cases.

[markostanimirovic]

Please revert this change. When error is explicitly set to unknown, it's not possible to override its type:

mapResponse({
  next: (val) => val + 1,
  error: (err: { message: string }) => err, // compilation error
})

This should be allowed.

[markostanimirovic]

Thanks @tom9744! Few small suggestions 👇

[markostanimirovic]

When using expect within subscribe, it's necessary to use done. Otherwise, incorrect tests will also succeed.

Suggested change

	it('should map the thrown error using the error callback', () => {
	throwError(() => 'error')
	.pipe(
	mapResponse({
	next: noop,
	error: (error) => `mapped ${error}`,
	})
	)
	.subscribe((result) => {
	expect(result).toBe('mapped error');
	});
	});
	it('should map the thrown error using the error callback', (done) => {
	throwError(() => 'error')
	.pipe(
	mapResponse({
	next: noop,
	error: (error) => `mapped ${error}`,
	})
	)
	.subscribe((result) => {
	expect(result).toBe('mapped error');
	done();
	});
	});

[markostanimirovic]

same here:

Suggested change

	it('should map the error thrown in next callback using error callback', () => {
	function producesError() {
	throw 'error';
	}
	of(1)
	.pipe(
	mapResponse({
	next: producesError,
	error: (error) => `mapped ${error}`,
	})
	)
	.subscribe((result) => {
	expect(result).toBe('mapped error');
	});
	});
	it('should map the error thrown in next callback using error callback', (done) => {
	function producesError() {
	throw 'error';
	}
	of(1)
	.pipe(
	mapResponse({
	next: producesError,
	error: (error) => `mapped ${error}`,
	})
	)
	.subscribe((result) => {
	expect(result).toBe('mapped error');
	done();
	});
	});

[markostanimirovic]

Please revert this change. When error is explicitly set to unknown, it's not possible to override its type:

mapResponse({
  next: (val) => val + 1,
  error: (err: { message: string }) => err, // compilation error
})

This should be allowed.

[markostanimirovic]

let's use a bit more complete example:

Suggested change

	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 }
	);