useTabster is unsafe for strict mode/concurrent rendering in React 18

[ling1726]

Tabster is created in useMemo from #23811 which is unsafe for concurrent rendering.

In general singletons will be problematic in concurrent rendering because there is no built in way to ensure a singleton constructor is called once:

// all the below call the `createSingleton` function twice on mount
React.useState(() => createSingleton());
React.useMemo(() => createSingleton(), []);

React.useEffect(() => {
  createSingleton();
}, [])

React.useLayoutEffect(() => {
  createSingleton();
}, [])

The above can be observed https://codesandbox.io/s/damp-glade-r7xb2x?file=/src/index.js

[layershifter]

Update: useRef is also not safe as refs will be nulled between renders.

---

A solution between is to use React.useMemo() in production & with React versions older than 18, and use useEffect in React 18. Something like:

function useConstructor<T>(
  factory: () => T,
  dispose: (instance: T) => void,
  deps: any[]
): T | null {
  console.log("useConstructor");

  if (
    process.env.NODE_ENV === "production" ||
    !React.version.startsWith("18")
  ) {
    // eslint-disable-next-line react-hooks/rules-of-hooks
    const value = React.useMemo(factory, deps);

    React.useEffect(() => {
      return () => dispose(value);
    }, [value]);
  }

  // eslint-disable-next-line react-hooks/rules-of-hooks
  const [value, setValue] = React.useState<T | null>(null);

  // eslint-disable-next-line react-hooks/rules-of-hooks
  React.useEffect(() => {
    const value = factory();

    setValue(value);
    return () => dispose(value);
  }, deps);

  return value;
}

https://codesandbox.io/s/spring-currying-b1nt36?file=/src/App.tsx:253-999

[ling1726]

Interesting update here: reactwg/react-18#18 (comment)

React seems to promote this kind of lazy initialization pattern. This can be interesting to pass down singletons that need to be used in event handlers, such as vanillajs tabster or external state management. Here's an example: https://codesandbox.io/s/pedantic-brahmagupta-36tfo3?file=/src/App.tsx

cc @layershifter

// This ref holds the imperative thing.
// It should only be referenced by the current component.
const ref = useRef(null);

// This lazy init function ref can be shared with other components,
// although it should only be called from an effect or an event handler.
// It should not be called during render.
const getterRef = useRef(() => {
  if (ref.current === null) {
    ref.current = new SomeImperativeThing();
  }
  return ref.current;
});

useEffect(() => {
  // This component doesn't need to (re)create the imperative API.
  // Any code that needs it will do this automatically by calling the getter.

  return () => {
    // It's possible that nothing called the getter function,
    // in which case we don't have to cleanup the imperative code.
    if (ref.current !== null) {
      ref.current.destroy();
      ref.current = null;
    }
  };
}, []);

[ling1726]

Closing this for now, there are no reported issues with strict mode and I can't find any issues with strict mode currently. Tabster supports multiple creation since microsoft/tabster#136.

If there are problems discovered in the future, reopen this issue