react-google-maps icon indicating copy to clipboard operation
react-google-maps copied to clipboard

[Feat] Replace useMemo with useState for one time initializations

Open andrewdoro opened this issue 1 year ago • 2 comments

Target Use Case

We shouldn't have useMemo for one time initializations. While the current behaviour is the same as a useState, 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

You may rely on useMemo as a performance optimization, not as a semantic guarantee. In the future, React may choose to “forget” some previously memoized values and recalculate them on next render, e.g. to free memory for offscreen components. Write your code so that it still works without useMemo — and then add it to optimize performance.

Proposal

Replace useMemo with useState 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

andrewdoro avatar Apr 30 '24 06:04 andrewdoro

TIL! Thanks for pointing this out! Would you be able to create a PR for this?

usefulthink avatar Apr 30 '24 09:04 usefulthink

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?

usefulthink avatar Apr 30 '24 13:04 usefulthink