foxact icon indicating copy to clipboard operation
foxact copied to clipboard

`useSingleton` will execute multiple times on falsey values

Open jaens opened this issue 1 year ago • 5 comments

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.

jaens avatar Jul 20 '24 20:07 jaens

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
});

SukkaW avatar Jul 21 '24 07:07 SukkaW

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:

  • A function that only has a side effect (eg. initializing some other singleton) and does not return anything (ie. returns undefined). Note that useEffect can not be used due to different semantics - since useSingleton is a render-time effect, not a mount-time effect.
  • A singleton that only needs to be conditionally created (ie. it must be and stay undefined when the condition is false). The conditional use case with re-execute you proposed above is in 99% of cases best handled using regular useMemo which has clear semantics and is more readable.

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

jaens avatar Jul 21 '24 10:07 jaens

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;
};

nekocode avatar Sep 04 '24 08:09 nekocode

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

jaens avatar Sep 06 '24 12:09 jaens

Curious if I'm missing something important. Isn't useSingleton just useMemo without dependencies?

import { useMemo } from "react";

export function useSingleton<T>(fn: () => T): T {
    return useMemo<T>(fn, []);
}

Or a state without a setter?

import { useState } from "react";

export function useSingleton<T>(fn: () => T): T {
    const [singleton] = useState<T>(fn);
    return singleton;
}

Edit: I was. useSingleton returns a ref, not the value of the ref.

kevlened avatar Dec 09 '24 14:12 kevlened