From 3e02e379d15b2e1625e28ed83ed2c5b73a7534de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Stanimirovi=C4=87?= <markostanimirovic95@gmail.com>
Date: Mon, 12 Jul 2021 22:32:53 +0200
Subject: [PATCH] fix(component-store): accept error type in tapResponse with
 strict generic checks (#3068)

Related to #3056
---
 .../component-store/spec/tap-response.spec.ts | 50 +++++++++++++
 .../spec/types/tap-response.types.spec.ts     | 71 +++++++++++++++++++
 modules/component-store/src/tap-response.ts   |  4 +-
 3 files changed, 123 insertions(+), 2 deletions(-)
 create mode 100644 modules/component-store/spec/tap-response.spec.ts
 create mode 100644 modules/component-store/spec/types/tap-response.types.spec.ts

diff --git a/modules/component-store/spec/tap-response.spec.ts b/modules/component-store/spec/tap-response.spec.ts
new file mode 100644
index 0000000000..69924661ea
--- /dev/null
+++ b/modules/component-store/spec/tap-response.spec.ts
@@ -0,0 +1,50 @@
+import { EMPTY, noop, Observable, of, throwError } from 'rxjs';
+import { tapResponse } from '@ngrx/component-store';
+import { concatMap, finalize } from 'rxjs/operators';
+
+describe('tapResponse', () => {
+  it('should invoke next callback on next', () => {
+    const nextCallback = jest.fn<void, [number]>();
+
+    of(1, 2, 3).pipe(tapResponse(nextCallback, noop)).subscribe();
+
+    expect(nextCallback.mock.calls).toEqual([[1], [2], [3]]);
+  });
+
+  it('should invoke error callback on error', () => {
+    const errorCallback = jest.fn<void, [{ message: string }]>();
+    const error = { message: 'error' };
+
+    throwError(error).pipe(tapResponse(noop, errorCallback)).subscribe();
+
+    expect(errorCallback).toHaveBeenCalledWith(error);
+  });
+
+  it('should invoke complete callback on complete', () => {
+    const completeCallback = jest.fn<void, []>();
+
+    EMPTY.pipe(tapResponse(noop, noop, completeCallback)).subscribe();
+
+    expect(completeCallback).toHaveBeenCalledWith();
+  });
+
+  it('should not unsubscribe from outer observable on inner observable error', () => {
+    const innerCompleteCallback = jest.fn<void, []>();
+    const outerCompleteCallback = jest.fn<void, []>();
+
+    new Observable((subscriber) => subscriber.next(1))
+      .pipe(
+        concatMap(() =>
+          throwError('error').pipe(
+            tapResponse(noop, noop),
+            finalize(innerCompleteCallback)
+          )
+        ),
+        finalize(outerCompleteCallback)
+      )
+      .subscribe();
+
+    expect(innerCompleteCallback).toHaveBeenCalled();
+    expect(outerCompleteCallback).not.toHaveBeenCalled();
+  });
+});
diff --git a/modules/component-store/spec/types/tap-response.types.spec.ts b/modules/component-store/spec/types/tap-response.types.spec.ts
new file mode 100644
index 0000000000..4e36aa354e
--- /dev/null
+++ b/modules/component-store/spec/types/tap-response.types.spec.ts
@@ -0,0 +1,71 @@
+import { Expect, expecter } from 'ts-snippet';
+import { compilerOptions } from './utils';
+
+describe('tapResponse types', () => {
+  const snippetFactory = (code: string): string => `
+    import { tapResponse } from '@ngrx/component-store';
+    import { noop, of } from 'rxjs';
+
+    ${code}
+  `;
+
+  function testWith(expectSnippet: (code: string) => Expect): void {
+    it('should infer next type', () => {
+      expectSnippet(`
+        of(1).pipe(
+          tapResponse((next) => {
+            const num = next;
+          }, noop)
+        );
+      `).toInfer('num', 'number');
+    });
+
+    it('should accept error type', () => {
+      expectSnippet(`
+        of(true).pipe(
+          tapResponse(noop, (error: { message: string }) => {
+            const err = error;
+          })
+        );
+      `).toInfer('err', '{ message: string; }');
+    });
+
+    it('should use unknown as default error type', () => {
+      expectSnippet(`
+        of(true).pipe(
+          tapResponse(noop, (error) => {
+            const err = error;
+          })
+        );
+      `).toInfer('err', 'unknown');
+    });
+  }
+
+  describe('strict mode', () => {
+    const expectSnippet = expecter(snippetFactory, {
+      ...compilerOptions(),
+      strict: true,
+    });
+
+    testWith(expectSnippet);
+  });
+
+  describe('non-strict mode', () => {
+    const expectSnippet = expecter(snippetFactory, {
+      ...compilerOptions(),
+      strict: false,
+    });
+
+    testWith(expectSnippet);
+  });
+
+  describe('non-strict mode with strict generic checks', () => {
+    const expectSnippet = expecter(snippetFactory, {
+      ...compilerOptions(),
+      strict: false,
+      noStrictGenericChecks: false,
+    });
+
+    testWith(expectSnippet);
+  });
+});
diff --git a/modules/component-store/src/tap-response.ts b/modules/component-store/src/tap-response.ts
index 5f5a364a91..9b5ed029e8 100644
--- a/modules/component-store/src/tap-response.ts
+++ b/modules/component-store/src/tap-response.ts
@@ -22,9 +22,9 @@ import { catchError, tap } from 'rxjs/operators';
  *   });
  * ```
  */
-export function tapResponse<T>(
+export function tapResponse<T, E = unknown>(
   nextFn: (next: T) => void,
-  errorFn: <E = unknown>(error: E) => void,
+  errorFn: (error: E) => void,
   completeFn?: () => void
 ): (source: Observable<T>) => Observable<T> {
   return (source) =>