refactor: align tap sigs with subscribe (#6125)

* refactor: align tap sigs with subscribe * chore: forgot api_guardian ... again * chore: remove line-length comments * refactor: simplify and deprecate multi-arg sigs * chore: update api_guardian
ReactiveX · Mar 16, 2021 · f8a3da5 · f8a3da5
1 parent ddab510
commit f8a3da5
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 30 deletions.
diff --git a/api_guard/dist/types/index.d.ts b/api_guard/dist/types/index.d.ts
@@ -261,10 +261,8 @@ export declare class Observable<T> implements Subscribable<T> {
     pipe<A, B, C, D, E, F, G, H, I>(op1: OperatorFunction<T, A>, op2: OperatorFunction<A, B>, op3: OperatorFunction<B, C>, op4: OperatorFunction<C, D>, op5: OperatorFunction<D, E>, op6: OperatorFunction<E, F>, op7: OperatorFunction<F, G>, op8: OperatorFunction<G, H>, op9: OperatorFunction<H, I>): Observable<I>;
     pipe<A, B, C, D, E, F, G, H, I>(op1: OperatorFunction<T, A>, op2: OperatorFunction<A, B>, op3: OperatorFunction<B, C>, op4: OperatorFunction<C, D>, op5: OperatorFunction<D, E>, op6: OperatorFunction<E, F>, op7: OperatorFunction<F, G>, op8: OperatorFunction<G, H>, op9: OperatorFunction<H, I>, ...operations: OperatorFunction<any, any>[]): Observable<unknown>;
     subscribe(observer?: Partial<Observer<T>>): Subscription;
-    subscribe(next: null | undefined, error: null | undefined, complete: () => void): Subscription;
-    subscribe(next: null | undefined, error: (error: any) => void, complete?: () => void): Subscription;
-    subscribe(next: (value: T) => void, error: null | undefined, complete: () => void): Subscription;
-    subscribe(next?: (value: T) => void, error?: (error: any) => void, complete?: () => void): Subscription;
+    subscribe(next: (value: T) => void): Subscription;
+    subscribe(next?: ((value: T) => void) | null, error?: ((error: any) => void) | null, complete?: (() => void) | null): Subscription;
     toPromise(): Promise<T | undefined>;
     toPromise(PromiseCtor: typeof Promise): Promise<T | undefined>;
     toPromise(PromiseCtor: PromiseConstructorLike): Promise<T | undefined>;

diff --git a/api_guard/dist/types/operators/index.d.ts b/api_guard/dist/types/operators/index.d.ts
@@ -280,11 +280,9 @@ export declare function takeWhile<T, S extends T>(predicate: (value: T, index: n
 export declare function takeWhile<T, S extends T>(predicate: (value: T, index: number) => value is S, inclusive: false): OperatorFunction<T, S>;
 export declare function takeWhile<T>(predicate: (value: T, index: number) => boolean, inclusive?: boolean): MonoTypeOperatorFunction<T>;
 
-export declare function tap<T>(next: null | undefined, error: null | undefined, complete: () => void): MonoTypeOperatorFunction<T>;
-export declare function tap<T>(next: null | undefined, error: (error: any) => void, complete?: () => void): MonoTypeOperatorFunction<T>;
-export declare function tap<T>(next: (value: T) => void, error: null | undefined, complete: () => void): MonoTypeOperatorFunction<T>;
-export declare function tap<T>(next?: (x: T) => void, error?: (e: any) => void, complete?: () => void): MonoTypeOperatorFunction<T>;
-export declare function tap<T>(observer: PartialObserver<T>): MonoTypeOperatorFunction<T>;
+export declare function tap<T>(observer?: Partial<Observer<T>>): MonoTypeOperatorFunction<T>;
+export declare function tap<T>(next: (value: T) => void): MonoTypeOperatorFunction<T>;
+export declare function tap<T>(next?: ((value: T) => void) | null, error?: ((error: any) => void) | null, complete?: (() => void) | null): MonoTypeOperatorFunction<T>;
 
 export declare function throttle<T>(durationSelector: (value: T) => ObservableInput<any>, { leading, trailing }?: ThrottleConfig): MonoTypeOperatorFunction<T>;
 

diff --git a/spec-dtslint/Observable-spec.ts b/spec-dtslint/Observable-spec.ts
@@ -131,3 +131,27 @@ describe('pipe', () => {
     const o = of('foo').toPromise(); // $ExpectType Promise<string | undefined>
   });
 });
+
+describe('subscribe', () => {
+  it('should deprecate the multi-argument usage', () => {
+    const next = (value: number) => {};
+    const error = (error: any) => {};
+    const complete = () => {};
+    const o = of(42);
+    o.subscribe(); // $ExpectNoDeprecation
+    o.subscribe({ next }); // $ExpectNoDeprecation
+    o.subscribe({ next, error }); // $ExpectNoDeprecation
+    o.subscribe({ next, complete }); // $ExpectNoDeprecation
+    o.subscribe({ next, error, complete }); // $ExpectNoDeprecation
+    o.subscribe({ error }); // $ExpectNoDeprecation
+    o.subscribe({ error, complete }); // $ExpectNoDeprecation
+    o.subscribe({ complete }); // $ExpectNoDeprecation
+    o.subscribe(next); // $ExpectNoDeprecation
+    o.subscribe(null, error); // $ExpectDeprecation
+    o.subscribe(undefined, error); // $ExpectDeprecation
+    o.subscribe(null, error, complete); // $ExpectDeprecation
+    o.subscribe(undefined, error, complete); // $ExpectDeprecation
+    o.subscribe(null, null, complete); // $ExpectDeprecation
+    o.subscribe(undefined, undefined, complete); // $ExpectDeprecation
+  });
+});
diff --git a/spec-dtslint/operators/tap-spec.ts b/spec-dtslint/operators/tap-spec.ts
@@ -11,10 +11,28 @@ it('should accept partial observer', () => {
   const c = of(1, 2, 3).pipe(tap({ complete: () => { } })); // $ExpectType Observable<number>
 });
 
-it('should not accept empty observer', () => {
-  const a = of(1, 2, 3).pipe(tap({})); // $ExpectError
-});
-
 it('should enforce type for next observer function', () => {
   const a = of(1, 2, 3).pipe(tap({ next: (x: string) => { } })); // $ExpectError
 });
+
+it('should deprecate the multi-argument usage', () => {
+  const next = (value: number) => {};
+  const error = (error: any) => {};
+  const complete = () => {};
+  const o = of(42);
+  o.pipe(tap()); // $ExpectNoDeprecation
+  o.pipe(tap({ next })); // $ExpectNoDeprecation
+  o.pipe(tap({ next, error })); // $ExpectNoDeprecation
+  o.pipe(tap({ next, complete })); // $ExpectNoDeprecation
+  o.pipe(tap({ next, error, complete })); // $ExpectNoDeprecation
+  o.pipe(tap({ error })); // $ExpectNoDeprecation
+  o.pipe(tap({ error, complete })); // $ExpectNoDeprecation
+  o.pipe(tap({ complete })); // $ExpectNoDeprecation
+  o.pipe(tap(next)); // $ExpectNoDeprecation
+  o.pipe(tap(null, error)); // $ExpectDeprecation
+  o.pipe(tap(undefined, error)); // $ExpectDeprecation
+  o.pipe(tap(null, error, complete)); // $ExpectDeprecation
+  o.pipe(tap(undefined, error, complete)); // $ExpectDeprecation
+  o.pipe(tap(null, null, complete)); // $ExpectDeprecation
+  o.pipe(tap(undefined, undefined, complete)); // $ExpectDeprecation
+});
diff --git a/src/internal/Observable.ts b/src/internal/Observable.ts
@@ -69,13 +69,9 @@ export class Observable<T> implements Subscribable<T> {
   }
 
   subscribe(observer?: Partial<Observer<T>>): Subscription;
+  subscribe(next: (value: T) => void): Subscription;
   /** @deprecated Use an observer instead of a complete callback, Details: https://rxjs.dev/deprecations/subscribe-arguments */
-  subscribe(next: null | undefined, error: null | undefined, complete: () => void): Subscription;
-  /** @deprecated Use an observer instead of an error callback, Details: https://rxjs.dev/deprecations/subscribe-arguments */
-  subscribe(next: null | undefined, error: (error: any) => void, complete?: () => void): Subscription;
-  /** @deprecated Use an observer instead of a complete callback, Details: https://rxjs.dev/deprecations/subscribe-arguments */
-  subscribe(next: (value: T) => void, error: null | undefined, complete: () => void): Subscription;
-  subscribe(next?: (value: T) => void, error?: (error: any) => void, complete?: () => void): Subscription;
+  subscribe(next?: ((value: T) => void) | null, error?: ((error: any) => void) | null, complete?: (() => void) | null): Subscription;
   /**
    * Invokes an execution of an Observable and registers Observer handlers for notifications it will emit.
    *

diff --git a/src/internal/operators/tap.ts b/src/internal/operators/tap.ts
@@ -1,19 +1,17 @@
-import { MonoTypeOperatorFunction, PartialObserver } from '../types';
+import { MonoTypeOperatorFunction, Observer } from '../types';
 import { isFunction } from '../util/isFunction';
 import { operate } from '../util/lift';
 import { OperatorSubscriber } from './OperatorSubscriber';
 import { identity } from '../util/identity';
 
-/* tslint:disable:max-line-length */
+export function tap<T>(observer?: Partial<Observer<T>>): MonoTypeOperatorFunction<T>;
+export function tap<T>(next: (value: T) => void): MonoTypeOperatorFunction<T>;
 /** @deprecated Use an observer instead of a complete callback, Details: https://rxjs.dev/deprecations/subscribe-arguments */
-export function tap<T>(next: null | undefined, error: null | undefined, complete: () => void): MonoTypeOperatorFunction<T>;
-/** @deprecated Use an observer instead of an error callback, Details: https://rxjs.dev/deprecations/subscribe-arguments */
-export function tap<T>(next: null | undefined, error: (error: any) => void, complete?: () => void): MonoTypeOperatorFunction<T>;
-/** @deprecated Use an observer instead of a complete callback, Details: https://rxjs.dev/deprecations/subscribe-arguments */
-export function tap<T>(next: (value: T) => void, error: null | undefined, complete: () => void): MonoTypeOperatorFunction<T>;
-export function tap<T>(next?: (x: T) => void, error?: (e: any) => void, complete?: () => void): MonoTypeOperatorFunction<T>;
-export function tap<T>(observer: PartialObserver<T>): MonoTypeOperatorFunction<T>;
-/* tslint:enable:max-line-length */
+export function tap<T>(
+  next?: ((value: T) => void) | null,
+  error?: ((error: any) => void) | null,
+  complete?: (() => void) | null
+): MonoTypeOperatorFunction<T>;
 
 /**
  * Used to perform side-effects for notifications from the source observable
@@ -106,7 +104,7 @@ export function tap<T>(observer: PartialObserver<T>): MonoTypeOperatorFunction<T
  * @param complete A completion handler
  */
 export function tap<T>(
-  observerOrNext?: PartialObserver<T> | ((value: T) => void) | null,
+  observerOrNext?: Partial<Observer<T>> | ((value: T) => void) | null,
   error?: ((e: any) => void) | null,
   complete?: (() => void) | null
 ): MonoTypeOperatorFunction<T> {