From dd06f9ee1cec0319643eeeb433fe946e4bc32660 Mon Sep 17 00:00:00 2001 From: crimx Date: Wed, 21 Aug 2019 22:45:24 +0800 Subject: [PATCH] refactor: let use-subscription support closure accessing deprecated use-observable-props-callback in favor or the new use-subscription BREAKING CHANGE: useSubscription supports only one way to pass subscribe arguments for simplicity. |useSubscription used to ignore changes of the subscribe functions, now it will always call the latest one. --- src/use-observable-props-callback.ts | 41 +-------- src/use-subscription.ts | 93 ++++++++++++++++----- test/use-observable-props-callback.spec.ts | 95 --------------------- test/use-subscription.spec.ts | 97 +++++++++++++++++++++- 4 files changed, 169 insertions(+), 157 deletions(-) delete mode 100644 test/use-observable-props-callback.spec.ts diff --git a/src/use-observable-props-callback.ts b/src/use-observable-props-callback.ts index f4bead91..742e866d 100644 --- a/src/use-observable-props-callback.ts +++ b/src/use-observable-props-callback.ts @@ -1,45 +1,8 @@ import { Observable, Subscription } from 'rxjs' import { withLatestFrom } from 'rxjs/operators' import { useSubscription } from './use-subscription' -import { useObservable } from './use-observable' /** - * Whenever the Observable emits a value, callback - * is called with that value. - * - * Note that changes of callback will not trigger - * an emission. If you need that just create another - * Observable with `useObservable`. - * - * Examples: - * - * ```typescript - * const events$ = useObservable(() => interval(1000)) - * - * useObservablePropsCallback(events$, props.onChange) - * ``` - * - * So why not use [[useSubscription]]? - * - * ```typescript - * useSubscription(events$, props.onChange) - * ``` - * - * [[useSubscription]] works the same if `props.onChange` never changes. - * `useObservablePropsCallback` ensures the latest `props.onChange` is called. + * @deprecated use [[useSubscription]] instead. */ -export function useObservablePropsCallback( - events$: Observable, - callback: (e: Event) => any -): Subscription { - const enhanced$ = useObservable( - callbacks$ => events$.pipe(withLatestFrom(callbacks$)), - [callback] as [typeof callback] - ) - return useSubscription(enhanced$, subscribe) -} - -/** @ignore */ -function subscribe([e, [callback]]: [E, [T]]) { - callback(e) -} +export const useObservablePropsCallback = useSubscription diff --git a/src/use-subscription.ts b/src/use-subscription.ts index c6145682..6f0591d0 100644 --- a/src/use-subscription.ts +++ b/src/use-subscription.ts @@ -1,19 +1,21 @@ -import { PartialObserver, Observable, Subscription } from 'rxjs' -import { useRefFn } from './helpers' -import { useEffect } from 'react' +import { Observable, Subscription } from 'rxjs' +import { useRefFn, emptyTuple } from './helpers' +import { useEffect, useRef } from 'react' /** - * Accepts an Observable and RxJS subscribe parameters. - * Deprecated subscribe parameter types are not included - * but you can use it anyway if not writing TypeScript. + * Accepts an Observable and optional `next`, `error`, `complete` functions. + * These functions must be in correct order. + * Use `undefined` or `null` for placeholder. * * Subscription will unsubscribe when unmount, you can also * unsubscribe manually. * - * Note that `useSubscription` will only subscribe once. - * Subsequent changes of the callback functions will be ignored. - * If you need that, take a look at [[useObservablePropsCallback]] - * or create an stream of callbacks yourself. + * Note that changes of callbacks will not trigger + * an emission. If you need that just create another + * Observable of the callback with [[useObservable]]. + * + * You can also access closure in the callback like in `useEffect`. + * `useSubscription` will ensure the latest callback is called. * * Examples: * @@ -21,32 +23,81 @@ import { useEffect } from 'react' * const subscription = useSubscription(events$, e => console.log(e.type)) * ``` * - * Or: + * On complete + * + * ```typescript + * const subscription = useSubscription(events$, null, null, () => console.log('complete')) + * ``` + * + * Access closure: * * ```typescript - * const subscription = useSubscription(events$, { - * next: console.log, - * error: console.error, - * complete: () => console.log('complete') + * const [debug, setDebug] = useState(false) + * const subscription = useSubscription(events$, null, error => { + * if (debug) { + * console.log(error) + * } * }) * ``` */ +export function useSubscription(stream$: Observable): Subscription export function useSubscription( stream$: Observable, - observer?: PartialObserver + next: (value: T) => void | null | undefined ): Subscription export function useSubscription( stream$: Observable, - next?: (value: T) => void, - error?: (error: any) => void, - complete?: () => void + next: (value: T) => void | null | undefined, + error: (error: any) => void | null | undefined ): Subscription export function useSubscription( stream$: Observable, - ...args: any[] + next: (value: T) => void | null | undefined, + error: (error: any) => void | null | undefined, + complete: () => void | null | undefined +): Subscription +export function useSubscription( + stream$: Observable, + ...args: + | [] + | [(value: T) => void | null | undefined] + | [ + (value: T) => void | null | undefined, + (error: any) => void | null | undefined + ] + | [ + (value: T) => void | null | undefined, + (error: any) => void | null | undefined, + () => void | null | undefined + ] ): Subscription { - const subscriptionRef = useRefFn(() => stream$.subscribe(...args)) + const argsRef = useRef< + Readonly< + | [] + | [(value: T) => void | null | undefined] + | [ + (value: T) => void | null | undefined, + (error: any) => void | null | undefined + ] + | [ + (value: T) => void | null | undefined, + (error: any) => void | null | undefined, + () => void | null | undefined + ] + > + >(emptyTuple) + argsRef.current = args + + const subscriptionRef = useRefFn(() => + stream$.subscribe({ + next: value => argsRef.current[0] && argsRef.current[0](value), + error: error => argsRef.current[1] && argsRef.current[1](error), + complete: () => argsRef.current[2] && argsRef.current[2]() + }) + ) + // unsubscribe when unmount useEffect(() => () => subscriptionRef.current.unsubscribe(), []) + return subscriptionRef.current } diff --git a/test/use-observable-props-callback.spec.ts b/test/use-observable-props-callback.spec.ts deleted file mode 100644 index 9b69e0ab..00000000 --- a/test/use-observable-props-callback.spec.ts +++ /dev/null @@ -1,95 +0,0 @@ -import { useObservablePropsCallback } from '../src' -import { renderHook } from '@testing-library/react-hooks' -import { Subject } from 'rxjs' - -describe('useObservablePropsCallback', () => { - it('should invoke the callback when Observable emits values', () => { - const num$$ = new Subject() - const spy = jest.fn() - renderHook(() => { - useObservablePropsCallback(num$$, spy) - }) - expect(spy).toBeCalledTimes(0) - num$$.next(1) - expect(spy).toBeCalledTimes(1) - expect(spy).lastCalledWith(1) - num$$.next(2) - expect(spy).toBeCalledTimes(2) - expect(spy).lastCalledWith(2) - }) - - it('should invoke the latest callback', () => { - const num$$ = new Subject() - const spy1 = jest.fn() - const spy2 = jest.fn() - const { rerender } = renderHook( - props => { - useObservablePropsCallback(num$$, props.cb) - }, - { - initialProps: { - cb: spy1 - } - } - ) - expect(spy1).toBeCalledTimes(0) - expect(spy2).toBeCalledTimes(0) - num$$.next(1) - expect(spy1).toBeCalledTimes(1) - expect(spy1).lastCalledWith(1) - expect(spy2).toBeCalledTimes(0) - spy1.mockClear() - spy2.mockClear() - rerender({ cb: spy2 }) - num$$.next(2) - expect(spy1).toBeCalledTimes(0) - expect(spy2).toBeCalledTimes(1) - expect(spy2).lastCalledWith(2) - }) - - it('should not emit value for callback changing', () => { - const num$$ = new Subject() - const spy1 = jest.fn() - const spy2 = jest.fn() - const { rerender } = renderHook( - props => { - useObservablePropsCallback(num$$, props.cb) - }, - { - initialProps: { - cb: spy1 - } - } - ) - expect(spy1).toBeCalledTimes(0) - expect(spy2).toBeCalledTimes(0) - num$$.next(1) - expect(spy1).toBeCalledTimes(1) - expect(spy1).lastCalledWith(1) - expect(spy2).toBeCalledTimes(0) - spy1.mockClear() - spy2.mockClear() - rerender({ cb: spy2 }) - expect(spy1).toBeCalledTimes(0) - expect(spy2).toBeCalledTimes(0) - }) - - it('should not invoke the callback when unmount', () => { - const num$$ = new Subject() - const spy = jest.fn() - const { unmount } = renderHook(() => { - useObservablePropsCallback(num$$, spy) - }) - expect(spy).toBeCalledTimes(0) - num$$.next(1) - expect(spy).toBeCalledTimes(1) - expect(spy).lastCalledWith(1) - num$$.next(2) - expect(spy).toBeCalledTimes(2) - expect(spy).lastCalledWith(2) - spy.mockClear() - unmount() - num$$.next(3) - expect(spy).toBeCalledTimes(0) - }) -}) diff --git a/test/use-subscription.spec.ts b/test/use-subscription.spec.ts index 297ae14c..cc2244eb 100644 --- a/test/use-subscription.spec.ts +++ b/test/use-subscription.spec.ts @@ -1,6 +1,7 @@ import { useSubscription } from '../src' -import { renderHook } from '@testing-library/react-hooks' -import { of, BehaviorSubject } from 'rxjs' +import { renderHook, act } from '@testing-library/react-hooks' +import { of, BehaviorSubject, Subject } from 'rxjs' +import { useState } from 'react' describe('useSubscription', () => { it('should always return the same Subscription', () => { @@ -26,6 +27,98 @@ describe('useSubscription', () => { expect(numSpy).lastCalledWith(3) }) + it('should invoke the latest callback', () => { + const num$$ = new Subject() + const spy1 = jest.fn() + const spy2 = jest.fn() + const { rerender } = renderHook( + props => { + useSubscription(num$$, props.cb) + }, + { + initialProps: { + cb: spy1 + } + } + ) + expect(spy1).toBeCalledTimes(0) + expect(spy2).toBeCalledTimes(0) + num$$.next(1) + expect(spy1).toBeCalledTimes(1) + expect(spy1).lastCalledWith(1) + expect(spy2).toBeCalledTimes(0) + spy1.mockClear() + spy2.mockClear() + rerender({ cb: spy2 }) + num$$.next(2) + expect(spy1).toBeCalledTimes(0) + expect(spy2).toBeCalledTimes(1) + expect(spy2).lastCalledWith(2) + }) + + it('should be able to access closure', () => { + const num$$ = new Subject() + const numSpy = jest.fn() + const { rerender, result } = renderHook( + props => { + const [stateVal, setState] = useState('s1') + useSubscription(num$$, num => { + numSpy(num, stateVal, props.propVal) + }) + return { setState } + }, + { + initialProps: { + propVal: 'p1' + } + } + ) + expect(numSpy).toBeCalledTimes(0) + num$$.next(1) + expect(numSpy).lastCalledWith(1, 's1', 'p1') + + numSpy.mockClear() + act(() => { + result.current.setState('s2') + }) + expect(numSpy).toBeCalledTimes(0) + num$$.next(2) + expect(numSpy).lastCalledWith(2, 's2', 'p1') + + numSpy.mockClear() + rerender({ propVal: 'p2' }) + expect(numSpy).toBeCalledTimes(0) + num$$.next(2) + expect(numSpy).lastCalledWith(2, 's2', 'p2') + }) + + it('should not emit value for callback changing', () => { + const num$$ = new Subject() + const spy1 = jest.fn() + const spy2 = jest.fn() + const { rerender } = renderHook( + props => { + useSubscription(num$$, props.cb) + }, + { + initialProps: { + cb: spy1 + } + } + ) + expect(spy1).toBeCalledTimes(0) + expect(spy2).toBeCalledTimes(0) + num$$.next(1) + expect(spy1).toBeCalledTimes(1) + expect(spy1).lastCalledWith(1) + expect(spy2).toBeCalledTimes(0) + spy1.mockClear() + spy2.mockClear() + rerender({ cb: spy2 }) + expect(spy1).toBeCalledTimes(0) + expect(spy2).toBeCalledTimes(0) + }) + it('should unsubscribe when unmount', () => { const num$$ = new BehaviorSubject(1) const numSpy = jest.fn()