feat(subscribe): Removed deprecated fn, fn, fn call pattern. (#6757)

* feat(subscribe): Removed deprecated fn, fn, fn call pattern. BREAKING CHANGE: `subscribe(fn, fn, fn)` is no longer a valid call for `subscribe`. Instead, use an observer: `subscribe({ next: fn, error: fn, complete: fn })`. * chore: remove unnecessary deprecation tests
ReactiveX · Jan 13, 2022 · ef59c7e · ef59c7e
1 parent 1967811
commit ef59c7e
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 89 deletions.
diff --git a/api_guard/dist/types/index.d.ts b/api_guard/dist/types/index.d.ts
@@ -415,9 +415,7 @@ export declare class Observable<T> implements Subscribable<T> {
     pipe<A, B, C, D, E, F, G, H>(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>): Observable<H>;
     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: (value: T) => void): Subscription;
-    subscribe(next?: ((value: T) => void) | null, error?: ((error: any) => void) | null, complete?: (() => void) | null): Subscription;
+    subscribe(observerOrNext?: Partial<Observer<T>> | ((value: T) => void) | null): Subscription;
 }
 
 export declare type ObservableInput<T> = Observable<T> | InteropObservable<T> | AsyncIterable<T> | PromiseLike<T> | ArrayLike<T> | Iterable<T> | ReadableStreamLike<T>;

diff --git a/spec-dtslint/Observable-spec.ts b/spec-dtslint/Observable-spec.ts
@@ -126,28 +126,4 @@ describe('pipe', () => {
     const customOperator = () => <T>(a: Observable<T>) => a;
     const o = of('foo').pipe(customOperator()); // $ExpectType Observable<string>
   });
-});
-
-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/Subject-spec.ts b/spec/Subject-spec.ts
@@ -421,7 +421,7 @@ describe('Subject', () => {
     expect(Subject.create).to.be.a('function');
     const source = of(1, 2, 3, 4, 5);
     const nexts: number[] = [];
-    const output: number[] = [];
+    const output: any[] = [];
 
     let error: any;
     let complete = false;
@@ -442,17 +442,17 @@ describe('Subject', () => {
       },
     };
 
-    const sub = Subject.create(destination, source);
+    // TODO(benlesh): This needs to go away, the types here are all wrong.
+    const sub: Subject<number | string> = Subject.create(destination, source);
 
-    sub.subscribe(
-      function (x: number) {
+    sub.subscribe({
+      next: function (x) {
         output.push(x);
       },
-      null,
-      () => {
+      complete: () => {
         outputComplete = true;
       }
-    );
+    });
 
     sub.next('a');
     sub.next('b');
@@ -492,17 +492,16 @@ describe('Subject', () => {
       },
     };
 
-    const sub = Subject.create(destination, source);
+    const sub: Subject<any> = Subject.create(destination, source);
 
-    sub.subscribe(
-      function (x: number) {
+    sub.subscribe({
+      next: function (x) {
         output.push(x);
       },
-      null,
-      () => {
+      complete: () => {
         outputComplete = true;
       }
-    );
+    });
 
     sub.next('a');
     sub.next('b');

diff --git a/spec/operators/min-spec.ts b/spec/operators/min-spec.ts
@@ -91,45 +91,45 @@ describe('min', () => {
   });
 
   it('should min a range() source observable', (done) => {
-    (<any>range(1, 10000)).pipe(min()).subscribe(
-      (value: number) => {
-        expect(value).to.equal(1);
-      },
-      (x: any) => {
-        done(new Error('should not be called'));
-      },
-      () => {
-        done();
-      }
-    );
+    range(1, 10000)
+      .pipe(min())
+      .subscribe({
+        next: (value) => {
+          expect(value).to.equal(1);
+        },
+        error: () => {
+          done(new Error('should not be called'));
+        },
+        complete: done,
+      });
   });
 
   it('should min a range().skip(1) source observable', (done) => {
-    (<any>range(1, 10)).pipe(skip(1), min()).subscribe(
-      (value: number) => {
-        expect(value).to.equal(2);
-      },
-      (x: any) => {
-        done(new Error('should not be called'));
-      },
-      () => {
-        done();
-      }
-    );
+    range(1, 10)
+      .pipe(skip(1), min())
+      .subscribe({
+        next: (value) => {
+          expect(value).to.equal(2);
+        },
+        error: () => {
+          done(new Error('should not be called'));
+        },
+        complete: done,
+      });
   });
 
   it('should min a range().take(1) source observable', (done) => {
-    (<any>range(1, 10)).pipe(take(1), min()).subscribe(
-      (value: number) => {
-        expect(value).to.equal(1);
-      },
-      (x: any) => {
-        done(new Error('should not be called'));
-      },
-      () => {
-        done();
-      }
-    );
+    range(1, 10)
+      .pipe(take(1), min())
+      .subscribe({
+        next: (value) => {
+          expect(value).to.equal(1);
+        },
+        error: () => {
+          done(new Error('should not be called'));
+        },
+        complete: done,
+      });
   });
 
   it('should work with error', () => {

diff --git a/src/internal/Observable.ts b/src/internal/Observable.ts
@@ -54,10 +54,6 @@ export class Observable<T> implements Subscribable<T> {
     return observable;
   }
 
-  subscribe(observer?: Partial<Observer<T>>): Subscription;
-  subscribe(next: (value: T) => void): Subscription;
-  /** @deprecated Instead of passing separate callback arguments, use an observer argument. Signatures taking separate callback arguments will be removed in v8. Details: https://rxjs.dev/deprecations/subscribe-arguments */
-  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.
    *
@@ -194,12 +190,8 @@ export class Observable<T> implements Subscribable<T> {
    * @return {Subscription} a subscription reference to the registered handlers
    * @method subscribe
    */
-  subscribe(
-    observerOrNext?: Partial<Observer<T>> | ((value: T) => void) | null,
-    error?: ((error: any) => void) | null,
-    complete?: (() => void) | null
-  ): Subscription {
-    const subscriber = isSubscriber(observerOrNext) ? observerOrNext : new SafeSubscriber(observerOrNext, error, complete);
+  subscribe(observerOrNext?: Partial<Observer<T>> | ((value: T) => void) | null): Subscription {
+    const subscriber = isSubscriber(observerOrNext) ? observerOrNext : new SafeSubscriber(observerOrNext);
 
     const { operator, source } = this;
     subscriber.add(

diff --git a/src/internal/Subscriber.ts b/src/internal/Subscriber.ts
@@ -34,7 +34,7 @@ export class Subscriber<T> extends Subscription implements Observer<T> {
    * If you have a specific use case, please file an issue.
    */
   static create<T>(next?: (x?: T) => void, error?: (e?: any) => void, complete?: () => void): Subscriber<T> {
-    return new SafeSubscriber(next, error, complete);
+    return new SafeSubscriber({ next, error, complete });
   }
 
   /** @deprecated Internal implementation detail, do not use directly. Will be made internal in v8. */
@@ -136,14 +136,12 @@ export class Subscriber<T> extends Subscription implements Observer<T> {
 }
 
 export class SafeSubscriber<T> extends Subscriber<T> {
-  constructor(
-    observerOrNext?: Partial<Observer<T>> | ((value: T) => void) | null,
-    error?: ((e?: any) => void) | null,
-    complete?: (() => void) | null
-  ) {
+  constructor(observerOrNext?: Partial<Observer<T>> | ((value: T) => void) | null) {
     super();
 
     let next: ((value: T) => void) | undefined;
+    let error: ((err: any) => void) | undefined;
+    let complete: (() => void) | undefined;
     if (isFunction(observerOrNext)) {
       // The first argument is a function, not an observer. The next
       // two arguments *could* be observers, or they could be empty.