imbhargav5 / rooks

Essential React custom hooks ⚓ to super charge your components!
https://rooks.vercel.app
MIT License
3.19k stars 216 forks source link

Missing memoization #500

Closed AdrienLemaire closed 3 years ago

AdrienLemaire commented 3 years ago

Describe the bug Several hooks are missing useMemo or useCallback and returning new objects/function at every render. This cause issues in effects that have dependencies on these, triggering them when they shouldn't.

To Reproduce Example using useTimeout

  const myRef = useRef(null);
  const callback = useCallback(() => {
      myRef.current.x = x;
      myRef.current.y = y
    }, [])
  const { start: addInitialPositions, clear } = useTimeout(callback, 0);

  useEffect(() => {
    addInitialPositions();
    return clear;
  }, [addInitialPositions, clear]);

I want my effect to be executed only once, and using a timeout of 0 to change the priority of the callback in the execution stack. But useTimeout returns new functions for start & clear at every render, which retriggers the useEffect every time.

Expected behavior

in https://github.com/imbhargav5/rooks/blob/535f788bbd1196fed4ae88cfd2afea36f85a0c53/src/hooks/useTimeout.ts#L27-L37

-  function start() {
-    setIsTimeoutActive(true);
-  }
+ const start = useCallback(() => setIsTimeoutActive(true), [])

Alternative solution Force an empty dependency list

  useEffect(() => {
    addInitialPositions();
    return clear;
  }, []); // eslint-disable-line react-hooks/exhaustive-deps

Unfortunately, this hides the problem that functions are unnecessarily being recreated. Since our component is rerendered at a 60FPS rate, we have 60 new start & clear functions created at every second, increasing the number of garbage collector calls, which can create visual lags.

Please consider reviewing all your hooks to insure that objects & functions are only recreated when needed (when a dependency is changing), thanks.

imbhargav5 commented 3 years ago

@AdrienLemaire Excellent point. I will address them this weekend.

imbhargav5 commented 3 years ago

Also, apologies for living under a cave and not looking at PRs.

imbhargav5 commented 3 years ago

Fix is WIP. It will be out in a couple of days. Thank you

AdrienLemaire commented 3 years ago

Thank you!

github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version 5.0.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket: