@uppy/react is not Compatible with React.StrictMode

[rart]

Initial checklist

• I understand this is a bug report and questions should be posted in the Community Forum
• I searched issues and couldn’t find anything (or linked relevant results below)

Link to runnable example

https://codesandbox.io/s/nifty-fire-7luyeb

Steps to reproduce

When using React.StrictMode the Dashboard doesn't render. To reproduce, wrap your app or component containing the Dashboard in <React.StrictMode>. See codesandbox above.

Expected behavior

Dashboard — and other components — works regardless of StrictMode

Actual behavior

The Dashboard renders an empty square without most functionality.

Most of this may be due to the usage of class components in @uppy/react.

[mifi]

This might be because in dev-mode, React with StrictMode will mount every component twice to reveal any bugs related to shared state and race conditions not handled by frequent mount/unmount. We should definitely fix this, because async rendering needs this to work.

[aduh95]

FWIW the bug is also reproducible on the 3.x branch: https://codesandbox.io/s/musing-blackwell-rpqhxr

[d-pollard]

Are there any workarounds for this?

[mifi]

I think a workaround is to selectively enable strict mode in other places in your app, except for Uppy. See facebook/react#16362

[Stouffi]

I rewrote a hook which follows a recommended pattern from this discussion reactwg/react-18#18
I've successfully tested it with React.StrictMode but the drawback is that it would be a breaking change as the uppy instance is no longer returned directly by the hook (and it was bad according to the new react docs, https://beta.reactjs.org/learn/referencing-values-with-refs#best-practices-for-refs, "don't read or write ref.current during rendering").

I made a codesandbox to compare the current useUppy which make the app fails at some point to retrieve the correct uppy instance and a new implementation compliant with StrictMode.

https://codesandbox.io/s/useuppy-c6688u?file=/src/Unsafe.tsx

Start by clicking Render button which will log an error that the TestPlugin is no longer defined (that is the instance has been closed)

The new hook code (written in TypeScript) would be

import Uppy from "@uppy/core";
import { useEffect, useRef } from "react";

export const useSafeUppy = (factory: () => Uppy) => {
  const ref = useRef<Uppy>();
  const getterRef = useRef(() => {
    if (ref.current === undefined) {
      ref.current = factory();
    }
    return ref.current;
  });
  useEffect(() => {
    return () => {
      if (ref.current !== undefined) {
        ref.current.close();
        ref.current = undefined;
      }
    };
  }, []);
  return getterRef;
};

[Murderlon]

Thanks for investigating this! I'm inclined to introduce a breaking change if we fix strict mode and at the same time get rid of a bad practice.