Open andrewdoro opened 3 months ago
TIL! Thanks for pointing this out! Would you be able to create a PR for this?
Thinking about this a bit more, what do we do about more complex cases or dependent intialization, for example
const geocodingLib = useMapsLibrary('geocoding');
const geocoder = useMemo(() => geocodingLib && new geocodingLib.Geocoder(), [geocodingLib]);
If the rule-of-thumb is "only use useMemo
if the code would also work without it" - well, technically it does work, but not memoizing in these cases would trigger a dependency chain that could have a dramatic impact.
And this is true for all uses of useMemo
I have reviewed so far (even including those you pointed out). The code will still work, but if react forgets about the memo it could incur some significant cost.
So how should we decide which pattern to use?
There are at least these options to choose from:
useMemo
– should only be used if it would be ok for react to occasionally forget about it, but it also looks best and I personally think the name fits best and it does return the value and not an array to be destructured.
const thing = useMemo(() => createThing());
// with dependency
const thing = useMemo(() => dependency && dependency.createThing(), [dependency]);
useState
with lazy init / with useEffect
for dependencies – this is also fine I guess, and everyone should be able to understand.
const [thing] = useState(() => createThing());
// with dependency
const [thing, setThing] = useState();
useEffect(() => { dependency && setThing(dependency.createThing()); }, [dependency]);
for the second case, we could even go one step further and introduce our own hook that would behave exactly like most people think useMemo
does.
So what do we do with this? Any opinions?
Target Use Case
We shouldn't have
useMemo
for one time initializations. While the current behaviour is the same as auseState
,useMemo
should only be used as an optimization. Future React releases might break this functionality.https://tkdodo.eu/blog/use-state-for-one-time-initializations React Forget
Proposal
Replace
useMemo
withuseState
for one time initializations. There might be more places where we need to change this:https://github.com/visgl/react-google-maps/blob/8140b4ae6df22a74f7be253430554b07267bb944/examples/deckgl-overlay/src/deckgl-overlay.ts#L16 https://github.com/visgl/react-google-maps/blob/8140b4ae6df22a74f7be253430554b07267bb944/src/components/map-control.tsx#L50 https://github.com/visgl/react-google-maps/blob/8140b4ae6df22a74f7be253430554b07267bb944/src/components/pin.tsx#L22