refactor(rx-effects-react): Removed a result Subscription from `use…

…Observer()` hook.
mnasyrov · Jan 29, 2023 · 04c8f8e · 04c8f8e
1 parent a1522a3
commit 04c8f8e
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 58 deletions.
diff --git a/packages/rx-effects-react/src/useObserver.test.ts b/packages/rx-effects-react/src/useObserver.test.ts
@@ -118,53 +118,17 @@ describe('useObserver()', () => {
     delete global.window;
   });
 
-  it('should return a subscription', () => {
-    const source$ = new BehaviorSubject(1);
-    const listener = jest.fn();
-
-    const { result } = renderHook(() => useObserver(source$, listener));
-
-    result.current.unsubscribe();
-    source$.next(2);
-
-    expect(listener).toHaveBeenCalledTimes(1);
-    expect(listener).toHaveBeenCalledWith(1);
-  });
-
-  it('should do nothing if is unsubscribe', () => {
-    const source1$ = new BehaviorSubject(1);
-    const source2$ = new BehaviorSubject(1);
-    const listener = jest.fn();
-
-    const { result, rerender } = renderHook(
-      ({ source$ }) => useObserver(source$, listener),
-      { initialProps: { source$: source1$ } },
-    );
-
-    result.current.unsubscribe();
-
-    rerender({ source$: source2$ });
-
-    source2$.next(2);
-
-    expect(listener).toHaveBeenCalledTimes(1);
-    expect(listener).toHaveBeenCalledWith(1);
-  });
-
   it('should unsubscribe on unmount', () => {
     const source$ = new BehaviorSubject(1);
     const listener = jest.fn();
 
-    const { result, unmount } = renderHook(() =>
-      useObserver(source$, listener),
-    );
+    const { unmount } = renderHook(() => useObserver(source$, listener));
 
     unmount();
     source$.next(2);
 
     expect(listener).toHaveBeenCalledTimes(1);
     expect(listener).toHaveBeenCalledWith(1);
-    expect(result.current.closed).toBe(true);
   });
 
   it('should unsubscribe old Observable and subscribe to new one when it changes', () => {
@@ -192,24 +156,29 @@ describe('useObserver()', () => {
     expect(listener).toHaveBeenLastCalledWith(2);
   });
 
-  it("should not subscribe a new observer in case hook's subscription was unsubscribed", () => {
+  it('should not subscribe a new observer in case a listener is changed', () => {
     const source$ = new BehaviorSubject(1);
     const listener1 = jest.fn();
     const listener2 = jest.fn();
 
-    const { result, rerender } = renderHook(
+    const { rerender } = renderHook(
       ({ listener }) => useObserver(source$, listener),
       { initialProps: { listener: listener1 } },
     );
 
-    result.current.unsubscribe();
+    const observer = source$.observers[0];
+    expect(observer).toBeDefined();
+
     rerender({ listener: listener2 });
     source$.next(2);
 
+    expect(source$.observers.length).toBe(1);
+    expect(source$.observers[0]).toBe(observer);
+
     expect(listener1).toHaveBeenCalledTimes(1);
     expect(listener1).toHaveBeenCalledWith(1);
 
-    expect(listener2).toHaveBeenCalledTimes(0);
+    expect(listener2).toHaveBeenCalledTimes(1);
   });
 });
 

diff --git a/packages/rx-effects-react/src/useObserver.ts b/packages/rx-effects-react/src/useObserver.ts
@@ -1,6 +1,5 @@
 import { useEffect, useLayoutEffect, useRef } from 'react';
-import { Observable, Observer, Subscription } from 'rxjs';
-import { useConst } from './useConst';
+import { Observable, Observer } from 'rxjs';
 
 /**
  * Subscribes the provided observer or `next` handler on `source$` observable.
@@ -20,9 +19,7 @@ import { useConst } from './useConst';
 export function useObserver<T>(
   source$: Observable<T>,
   observerOrNext: Partial<Observer<T>> | ((value: T) => void),
-): Subscription {
-  const hookSubscriptions = useConst(() => new Subscription());
-
+): void {
   const argsRef = useRef<Partial<Observer<T>>>();
 
   // Update the latest observable and callbacks
@@ -35,24 +32,14 @@ export function useObserver<T>(
   });
 
   useEffect(() => {
-    if (hookSubscriptions.closed) {
-      return;
-    }
-
     const subscription = source$.subscribe({
       next: (value) => argsRef.current?.next?.(value),
       error: (error) => argsRef.current?.error?.(error),
       complete: () => argsRef.current?.complete?.(),
     });
-    hookSubscriptions.add(subscription);
-    return () => subscription.unsubscribe();
-  }, [hookSubscriptions, source$]);
 
-  useEffect(() => {
-    return () => hookSubscriptions.unsubscribe();
-  }, [hookSubscriptions]);
-
-  return hookSubscriptions;
+    return () => subscription.unsubscribe();
+  }, [source$]);
 }
 
 export function isBrowser() {