feat: rewrite useSubscribe

Author: mbret

src/lib/binding/useSubscribe.ts

@@ -1,8 +1,8 @@
 import { act, cleanup, renderHook } from "@testing-library/react"
-import { useEffect, useState } from "react"
+import { useCallback, useEffect, useState } from "react"
 import { type Observable, of, tap } from "rxjs"
 import { afterEach, describe, expect, it } from "vitest"
-import { waitForTimeout } from "../../tests/utils"
+import { waitForTimeout } from "../../../tests/utils"
 import { useSubscribe } from "./useSubscribe"
 
 afterEach(() => {
@@ -14,7 +14,7 @@ describe("Given a function that returns undefined", () => {
     const maybeObservableFn = () => undefined as Observable<number> | undefined
 
     renderHook(() => {
-      useSubscribe(maybeObservableFn, [])
+      useSubscribe(maybeObservableFn)
     }, {})
 
     expect(true).toBe(true)
@@ -29,7 +29,7 @@ describe("Given a function that returns undefined", () => {
           undefined,
         )
 
-        useSubscribe(
+        const source$ = useCallback(
           () =>
             value?.pipe(
               tap((v) => {
@@ -39,6 +39,8 @@ describe("Given a function that returns undefined", () => {
           [value],
         )
 
+        useSubscribe(source$)
+
         useEffect(() => {
           setTimeout(() => {
             setValue(of(5))

src/lib/binding/types.ts src/lib/binding/useSubscribe/types.tssrc/lib/binding/types.ts renamed to src/lib/binding/useSubscribe/types.ts

@@ -0,0 +1,13 @@
+import { useEffect } from "react"
+import { makeObservable } from "../../utils/makeObservable"
+import type { SubscribeSource } from "./types"
+
+export function useSubscribe<T>(source: SubscribeSource<T>) {
+  useEffect(() => {
+    const sub = makeObservable(source)().subscribe()
+
+    return () => {
+      sub.unsubscribe()
+    }
+  }, [source])
+}

src/lib/binding/useSubscribe.test.tsx …nding/useSubscribe/useSubscribe.test.tsxsrc/lib/binding/useSubscribe.test.tsx renamed to src/lib/binding/useSubscribe/useSubscribe.test.tsx src/lib/binding/useSubscribe.test.tsx renamed to src/lib/binding/useSubscribe/useSubscribe.test.tsx

@@ -0,0 +1,66 @@
+import { act, cleanup, renderHook } from "@testing-library/react"
+import { config, defer, of, throwError } from "rxjs"
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
+import { waitForTimeout } from "../../tests/utils"
+import { useSubscribeEffect } from "./useSubscribeEffect"
+
+let previousOnUnhandledError = config.onUnhandledError
+
+beforeEach(() => {
+  previousOnUnhandledError = config.onUnhandledError
+  config.onUnhandledError = () => {}
+})
+
+afterEach(() => {
+  config.onUnhandledError = previousOnUnhandledError
+  cleanup()
+  vi.restoreAllMocks()
+})
+
+describe("useSubscribeEffect", () => {
+  it("should call onError when source errors", async () => {
+    const onError = vi.fn()
+    let subscribeCount = 0
+
+    const source$ = defer(() => {
+      subscribeCount += 1
+      return subscribeCount === 1 ? throwError(() => new Error("boom")) : of(1)
+    })
+
+    renderHook(() => {
+      useSubscribeEffect(source$, { onError, retryOnError: true })
+    })
+
+    await act(async () => {
+      await waitForTimeout(0)
+    })
+
+    expect(onError).toHaveBeenCalledTimes(1)
+    expect(subscribeCount).toBe(2)
+  })
+
+  it("should not retry when retryOnError is false", async () => {
+    const onError = vi.fn()
+    const onUnhandledError = vi.fn()
+    let subscribeCount = 0
+    config.onUnhandledError = onUnhandledError
+
+    const source$ = defer(() => {
+      subscribeCount += 1
+      return throwError(() => new Error("boom"))
+    })
+
+    renderHook(() => {
+      useSubscribeEffect(source$, { onError, retryOnError: false })
+    })
+
+    await act(async () => {
+      await waitForTimeout(0)
+    })
+
+    expect(onError).toHaveBeenCalledTimes(1)
+    expect(onUnhandledError).toHaveBeenCalledTimes(1)
+    expect(onUnhandledError.mock.calls[0]?.[0]).toBeInstanceOf(Error)
+    expect(subscribeCount).toBe(1)
+  })
+})

src/lib/binding/useSubscribe/useSubscribe.ts

@@ -1,64 +1,36 @@
-import { type DependencyList, useCallback } from "react"
-import { catchError, identity, retry } from "rxjs"
+import { useCallback } from "react"
+import { of, retry, tap, throwError } from "rxjs"
 import { makeObservable } from "../utils/makeObservable"
 import { useLiveRef } from "../utils/react/useLiveRef"
-import type { SubscribeSource } from "./types"
-import { useSubscribe } from "./useSubscribe"
+import type { SubscribeSource } from "./useSubscribe/types"
+import { useSubscribe } from "./useSubscribe/useSubscribe"
 
 interface Option {
-  retry?: boolean
+  retryOnError?: boolean
   onError?: (error: unknown) => void
 }
 
-export function useSubscribeEffect<T>(source: SubscribeSource<T>): void
 export function useSubscribeEffect<T>(
   source: SubscribeSource<T>,
-  options: Option,
-): void
-
-export function useSubscribeEffect<T>(
-  source: SubscribeSource<T>,
-  deps: DependencyList,
-): void
-
-export function useSubscribeEffect<T>(
-  source: SubscribeSource<T>,
-  options: Option,
-  deps: DependencyList,
-): void
-
-export function useSubscribeEffect<T>(
-  source: SubscribeSource<T>,
-  unsafeOptions?: Option | DependencyList,
-  deps: DependencyList = [],
+  options?: Option,
 ) {
-  const options =
-    unsafeOptions != null && !Array.isArray(unsafeOptions)
-      ? (unsafeOptions as Option)
-      : ({} satisfies Option)
-  const retryOption = options.retry ?? true
-  const onErrorRef = useLiveRef(
-    options.onError ??
-      ((error: unknown) => {
-        console.error(error)
-      }),
-  )
+  const optionsRef = useLiveRef(options)
 
-  // biome-ignore lint/correctness/useExhaustiveDependencies: TODO
-  const sourceAsObservable = useCallback(() => makeObservable(source)(), deps)
+  const enhancerMakeObservable = useCallback(() => {
+    const source$ = makeObservable(source)()
 
-  const enhancerMakeObservable = useCallback(
-    () =>
-      sourceAsObservable().pipe(
-        catchError((error) => {
-          onErrorRef.current(error)
-
-          throw error
-        }),
-        retryOption ? retry() : identity,
-      ),
-    [sourceAsObservable, retryOption, onErrorRef],
-  )
+    return source$.pipe(
+      tap({
+        error: (error) => {
+          optionsRef.current?.onError?.(error)
+        },
+      }),
+      retry({
+        delay: (error) =>
+          optionsRef.current?.retryOnError ? of(0) : throwError(() => error),
+      }),
+    )
+  }, [source, optionsRef])
 
-  useSubscribe(enhancerMakeObservable, deps)
+  useSubscribe(enhancerMakeObservable)
 }

src/lib/binding/useSubscribeEffect.test.tsx

@@ -21,7 +21,9 @@ export default defineConfig({
         devDeps: true,
       }),
     },
-    dts(),
+    dts({
+      staticImport: true,
+    }),
   ],
   build: {
     minify: "terser",