`@uppy/react` is not Compatible with `React.StrictMode`
Initial checklist
- [X] I understand this is a bug report and questions should be posted in the Community Forum
- [X] 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.
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.
FWIW the bug is also reproducible on the 3.x branch: https://codesandbox.io/s/musing-blackwell-rpqhxr
Are there any workarounds for this?
I think a workaround is to selectively enable strict mode in other places in your app, except for Uppy. See https://github.com/facebook/react/issues/16362
I rewrote a hook which follows a recommended pattern from this discussion https://github.com/reactwg/react-18/discussions/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;
};
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.