SukkaW / foxact

React Hooks/Utils done right. For Browser, SSR, and React Server Components.
https://foxact.skk.moe
MIT License
304 stars 10 forks source link

`useSingleton` will execute multiple times on falsey values #22

Open jaens opened 1 month ago

jaens commented 1 month ago

The check in the source code is !r.current which will trigger the function every call when the value happens to be 0, "", undefined, null etc.

Here's a more universal implementation:

(technically, EMPTY can just be {} as const if Symbol support is unavailable)

import { useRef } from "react";

const EMPTY = Symbol.for("useOnce/EMPTY");

export function useSingleton<T>(fn: () => T): T {
    const hasRun = useRef<T | typeof EMPTY>(EMPTY);
    const { current } = hasRun;
    if (current !== EMPTY) {
        return current;
    }

    return (hasRun.current = fn());
}

An alternative is to add a <T extends object> type constraint, although that seems slightly unintuitive.

SukkaW commented 1 month ago

const EMPTY = Symbol.for("useOnce/EMPTY");

I like the idea of the Symbol. But IMHO would you mind explaining your use case of useSingleton? The main idea behind useSingleton is to create a singleton instance, it doesn't expect the return value of the creation function to be undefined or null:

const playerRef = useSingleton(() => new APlayer())

Also, with the current behavior, it is possible to conditionally create the instance:

const [shouldLoad, setShouldLoad] = useState(false);

const instance1Ref = useSingleton(() => {
  shouldLoad ? new APlayer() : null
});
const instance2Ref = useSingleton(() => {
  props.foo ? new Foo() : null
});
jaens commented 1 month ago

It's not really about the use case (of course there are use cases), it's about the API behaving in surprising and undocumented ways.

Example use cases:

(In my own code, this function is actually called useOnce for clarity, since it's not really about just singletons)

nekocode commented 2 days ago

Why not add a new ref to determine if it has been initialized?

export const useSingleton = <T>(initializor: () => T): SingletonRefObject<T> => {
  const initialized = useRef(false);
  const r = useRef<T>();
  if (!initialized.current) {
    r.current = initializor();
    initialized.current = true;
  }

  return r;
};
jaens commented 16 hours ago

That works, but is less efficient and doubles the clutter in the React debug panel.