Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support double invocation of useMemo useEffect and functional components #47

Closed
penx opened this issue May 20, 2022 · 2 comments · Fixed by #46
Closed

Support double invocation of useMemo useEffect and functional components #47

penx opened this issue May 20, 2022 · 2 comments · Fixed by #46

Comments

@penx
Copy link
Owner

penx commented May 20, 2022

useMemo should not be used for side effects.

In strict mode during development on React 17, function component bodies and functions passed to useState, useMemo, or useReducer are invoked twice.

Invocations of a functional component happen before any useEffect within it is resolved. Note that in the following code in React 18, in strict mode, "render" will be logged twice first and then "effect" will be logged twice:

import { useEffect } from "react";

export default function App() {
  console.log("render");
  useEffect(() => console.log("effect"));
  return null;
}

https://codesandbox.io/s/use-effect-order-strict-mode-6v00ys?file=/src/App.js:0-152

(note that in React 17, the console is muted on one of the invocations, which caused me a lot of pain during debugging - this isn't the case in React 18).

In React 18, functions passed to useEffect are also called twice. I'm not sure if this is dev mode only.

These are all intentional in order to detect issues e.g. with async react and upcoming features such as preserving state between unmounts.

We need to set loading=true within the hook depending on whether fetch is going to be called, and then return this value from the hook.

Perhaps we need to call fetch with useSyncExternalStore (via the separate module for React 17 compatibility) although as we're not caching responses, I don't think we would have an external store, so perhaps this isn't relevant.

...or the upcoming useEvent if/when it makes it in to React.

@penx
Copy link
Owner Author

penx commented May 20, 2022

I have a PR open to replace useMemo with useEffect but this doesn't completely solve the issue

#46

@penx
Copy link
Owner Author

penx commented May 20, 2022

I was considering detecting whether fetch should be called with some code such as this

const useHasChanged= (value) => {
    const prevValue = usePrevious(value);

    return prevValue !== value;
}

const usePrevious = (value) => {
    const ref = useRef();
    useEffect(() => {
      ref.current = value;
    });
    return ref.current;
}

https://stackoverflow.com/a/68174829/1582783

However this doesn't work if the the component renders twice before the useEffect is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant