fix: fixed useobserve

Author: mbret

src/lib/binding/useObserve/store.ts

@@ -1,81 +1,53 @@
 import {
-  catchError,
+  BehaviorSubject,
   distinctUntilChanged,
-  EMPTY,
-  map,
   NEVER,
   type Observable,
+  type Subscription,
   share,
-  shareReplay,
   tap,
 } from "rxjs"
-import { filterObjectByKey } from "../../utils/filterObjectByKey"
-import { makeObservable } from "../../utils/makeObservable"
 
-export class ObservableStore<T> {
-  state: {
-    data: T | undefined
-    status: "pending" | "success" | "error"
-    observableState: "complete" | "error" | "live"
-    error: Error | undefined
-  } = {
-    data: undefined,
-    status: "pending",
-    observableState: "live",
-    error: undefined,
-  }
+export type State<T, DefaultValue, Error = unknown> = {
+  data: T | DefaultValue
+  status: "pending" | "success" | "error"
+  observableState: "complete" | "error" | "live"
+  error: Error | undefined
+}
+
+export interface ObservableStoreOptions<T, DefaultValue> {
+  defaultValue: DefaultValue
+  compareFn: ((a: T, b: T) => boolean) | undefined
+}
 
+export class ObservableStore<T, DefaultValue, Error = unknown> {
+  state: State<T, DefaultValue, Error>
   source$: Observable<T | undefined>
+  sub: Subscription
 
   constructor({
     source$: miscSource$,
     defaultValue,
-    selectorKeys,
     compareFn,
   }: {
     source$: Observable<T> | (() => Observable<T> | undefined)
-    defaultValue: T | undefined
-    selectorKeys: (keyof T)[] | undefined
-    compareFn: ((a: T, b: T) => boolean) | undefined
-  }) {
-    this.state.data = defaultValue
-
+  } & ObservableStoreOptions<T, DefaultValue>) {
     const source$ =
       typeof miscSource$ === "function" ? miscSource$() : miscSource$
 
-    console.log("source$", source$)
+    const hasNoDefinedSource = source$ === undefined
 
-    if (source$ === undefined) {
-      this.state = {
-        ...this.state,
-        status: "success",
-        observableState: "complete",
-      }
+    this.state = {
+      data: source$ instanceof BehaviorSubject ? source$.value : defaultValue,
+      status: hasNoDefinedSource ? "success" : "pending",
+      observableState: hasNoDefinedSource ? "complete" : "live",
+      error: undefined,
     }
 
     this.source$ = (source$ ?? NEVER).pipe(
-      // Maybe selector
-      map((v) => {
-        if (selectorKeys && typeof v === "object" && v !== null) {
-          return filterObjectByKey(v, selectorKeys) as T | undefined
-        }
-
-        return v
-      }),
-      // Maybe compareFn
-      distinctUntilChanged((a, b) => {
-        if (a === undefined || b === undefined) return false
-
-        if (compareFn) {
-          return compareFn(a as T, b as T)
-        }
-
-        return a === b
-      }),
+      distinctUntilChanged(compareFn),
       tap({
         complete: () => {
-          console.log("complete")
-
           this.state = {
             ...this.state,
             status: "success",
@@ -91,19 +63,31 @@ export class ObservableStore<T> {
           }
         },
         next: (data) => {
-          console.log("next", data)
-
           this.state = { ...this.state, data }
         },
       }),
-      shareReplay({ refCount: true, bufferSize: 1 }),
+      share(),
     )
 
-    // to clean up
-    this.source$.subscribe()
+    /**
+     * @important This eager subscription will optimistically update the state.
+     * Any observable that is non async (behavior, of(x), etc) will have in fact their state completed
+     * by the first render cycle.
+     * Although we only correctly type sync state for `BehaviorSubject` we can in fact get the value in sync
+     * for more than them.
+     * Technically the whole pipe chain runs "synchronously".
+     *
+     * This is not a guarantee, just that in best case scenario there will be only one render.
+     */
+    this.sub = this.source$.subscribe()
   }
 
   subscribe = (next: () => void) => {
+    // in some case the observable is already complete by the time we subscribe to it.
+    if (this.state.observableState === "complete") {
+      return () => {}
+    }
+
     const sub = this.source$.subscribe({
       complete: next,
       error: next,
@@ -115,11 +99,7 @@ export class ObservableStore<T> {
     }
   }
 
-  getSnapshot = (): typeof this.state => {
-    // console.log("getSnapshot", this.state)
-
+  getSnapshot = () => {
     return this.state
   }
 }
-
-export const storeMap = new Map<string, ObservableStore<unknown>>()

src/lib/binding/useObserve/types.ts

@@ -1,11 +1,7 @@
-export interface UseObserveResult<Value, DefaultValue, Error = unknown> {
-    data: Value | DefaultValue
-    error: Error | undefined
-    /**
-     * - pending: if there's no cached data and no query attempt was finished yet.
-     * - success: if the query has received a response with no errors and is ready to display its data
-     * - error: if the query attempt resulted in an error. The corresponding error property has the error received from the attempted fetch
-     */
-    status: "pending" | "success" | "error"
-    observableState: "complete" | "live"
-  }
+import type { ObservableStoreOptions, State } from "./store"
+
+export interface UseObserveResult<Value, DefaultValue, Error = unknown>
+  extends State<Value, DefaultValue, Error> {}
+
+export interface UseObserveOptions<T, DefaultValue>
+  extends Partial<ObservableStoreOptions<T, DefaultValue>> {}

src/lib/binding/useObserve/useObserve.compare.test.tsx

@@ -1,23 +1,29 @@
 import { renderHook } from "@testing-library/react"
-import { act, useEffect } from "react"
+import { act, useEffect, useState } from "react"
 import { BehaviorSubject } from "rxjs"
 import { describe, expect, it } from "vitest"
 import { isShallowEqual } from "../../utils/shallowEqual"
+import type { UseObserveResult } from "./types"
 import { useObserve } from "./useObserve"
 
 it("should not re-render when the value is same reference", async () => {
-  const values: Array<undefined> = []
+  const values: Array<UseObserveResult<undefined, undefined>> = []
   const source$ = new BehaviorSubject(undefined)
 
   renderHook(() => {
     const value = useObserve(source$)
 
     useEffect(() => {
-      values.push(value.data)
+      values.push(value)
     }, [value])
   }, {})
 
-  expect(values[0]).toEqual(undefined)
+  expect(values[0]).toEqual({
+    data: undefined,
+    error: undefined,
+    status: "pending",
+    observableState: "live",
+  })
 
   act(() => {
     source$.next(undefined)
@@ -118,3 +124,61 @@ describe("given a custom compareFn", () => {
     expect(values.length).toBe(1)
   })
 })
+
+describe("Given a forever true compareFn", () => {
+  describe("and a new value is pushed", () => {
+    it("should not return the new value", async () => {
+      const values: Array<UseObserveResult<number, number>> = []
+      const source$ = new BehaviorSubject(1)
+
+      renderHook(() => {
+        const value = useObserve(source$, { compareFn: () => true })
+
+        useEffect(() => {
+          values.push(value)
+        }, [value])
+      }, {})
+
+      act(() => {
+        source$.next(2)
+      })
+
+      expect(values.length).toBe(1)
+      expect(values.at(-1)?.data).toBe(1)
+    })
+  })
+
+  describe("when a new compareFn is provided with forever false", () => {
+    describe("and a new value is pushed", () => {
+      it("should return the new value", async () => {
+        const values: Array<UseObserveResult<number, number>> = []
+        const source$ = new BehaviorSubject(1)
+
+        const { result } = renderHook(() => {
+          const [compareFn, setCompareFn] = useState(() => () => true)
+          const value = useObserve(source$, { compareFn })
+
+          useEffect(() => {
+            values.push(value)
+          }, [value])
+
+          return { setCompareFn }
+        }, {})
+
+        expect(values.length).toBe(1)
+        expect(values.at(-1)?.data).toBe(1)
+
+        act(() => {
+          result.current.setCompareFn(() => () => false)
+        })
+
+        act(() => {
+          source$.next(2)
+        })
+
+        expect(values.length).toBe(2)
+        expect(values.at(-1)?.data).toBe(2)
+      })
+    })
+  })
+})

src/lib/binding/useObserve/useObserve.selector.test.tsx

@@ -1,16 +1,21 @@
 import { renderHook } from "@testing-library/react"
 import { act, useEffect } from "react"
-import { BehaviorSubject, of } from "rxjs"
+import { BehaviorSubject, distinctUntilChanged, map, of } from "rxjs"
 import { expect, it } from "vitest"
+import { filterObjectByKey } from "../../utils/filterObjectByKey"
+import { isShallowEqual } from "../../utils/shallowEqual"
 import type { UseObserveResult } from "./types"
 import { useObserve } from "./useObserve"
 
 it("should return the selected keys", async () => {
   const values: Array<{ foo: string } | undefined> = []
-  const source$ = of({ foo: "foo" })
+  const source$ = of({ foo: "foo" }).pipe(
+    map((data) => filterObjectByKey(data, ["foo"])),
+    distinctUntilChanged(isShallowEqual),
+  )
 
   renderHook(() => {
-    const value = useObserve(source$, ["foo"])
+    const value = useObserve(source$)
 
     values.push(value.data)
   }, {})
@@ -19,12 +24,20 @@ it("should return the selected keys", async () => {
   expect(values.length).toBe(1)
 })
 
-it("should not re-render when the selected keys don't change foobar", async () => {
+it("should not re-render when the selected keys don't change", async () => {
   const values: Array<UseObserveResult<{ a: string }, { a: string }>> = []
   const source$ = new BehaviorSubject({ a: "a", b: "b" })
 
   renderHook(() => {
-    const value = useObserve(source$, ["a"])
+    const value = useObserve(
+      () =>
+        source$.pipe(
+          map((data) => filterObjectByKey(data, ["a"])),
+          distinctUntilChanged(isShallowEqual),
+        ),
+      { defaultValue: source$.value },
+      [],
+    )
 
     useEffect(() => {
       values.push(value)
@@ -42,13 +55,6 @@ it("should not re-render when the selected keys don't change foobar", async () =
     source$.next({ a: "a", b: "c" })
   })
 
-  expect(values[1]).toEqual({
-    data: { a: "a" },
-    error: undefined,
-    status: "pending",
-    observableState: "live",
-  })
-
   expect(values.length).toBe(1)
 })
 
@@ -57,7 +63,15 @@ it("should re-render when the selected keys change", async () => {
   const source$ = new BehaviorSubject({ a: "a", b: "b" })
 
   renderHook(() => {
-    const value = useObserve(source$, ["a"])
+    const value = useObserve(
+      () =>
+        source$.pipe(
+          map((data) => filterObjectByKey(data, ["a"])),
+          distinctUntilChanged(isShallowEqual),
+        ),
+      { defaultValue: source$.value },
+      [],
+    )
 
     useEffect(() => {
       values.push(value.data)

src/lib/binding/useObserve/useObserve.test.tsx

@@ -4,6 +4,7 @@ import { BehaviorSubject, map, type Observable, of, Subject, timer } from "rxjs"
 import { afterEach, describe, expect, expectTypeOf, it } from "vitest"
 import { waitForTimeout } from "../../../tests/utils"
 import { useConstant } from "../../utils/react/useConstant"
+import type { UseObserveResult } from "./types"
 import { useObserve } from "./useObserve"
 
 afterEach(() => {
@@ -13,14 +14,17 @@ afterEach(() => {
 describe("useObserve", () => {
   describe("Given a non BehaviorSubject observable", () => {
     it("should return `foo` as first render cycle due to internal optimization and should only have one render cycle", async () => {
-      const values: Array<string | undefined> = []
+      const values: Array<UseObserveResult<string, undefined>> = []
       const source$ = of("foo")
 
       renderHook(() => {
-        values.push(useObserve(source$).data)
+        values.push(useObserve(source$))
       }, {})
 
-      expect(values[0]).toBe("foo")
+      expect(values[0]?.data).toBe("foo")
+
+      console.log("values", values)
+
       expect(values.length).toBe(1)
     })
   })
@@ -275,7 +279,6 @@ describe("useObserve", () => {
           undefined,
         )
 
-        // console.log("RENDER")
         values.push(useObserve(() => res, [res]))
 
         useEffect(() => {