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

Running alert.show inside useEffect causes infinite loop #119

Closed
amillward opened this issue Jun 30, 2019 · 5 comments
Closed

Running alert.show inside useEffect causes infinite loop #119

amillward opened this issue Jun 30, 2019 · 5 comments

Comments

@amillward
Copy link

I have a functional component that loads data inside a useEffect. I'm required to provide alert as a dep, however doing so causes an infinite loop.

CodeSandBox here: https://codesandbox.io/s/mystifying-rubin-dhbov

@besLisbeth
Copy link
Collaborator

Hi, @amillward.
Can you please, explain, why did you pass an alert to useEffect?

What can I tell you - in your code useEffect is calling on every rerender, as it is written in the documentation. Because of the fact, that you calling an alert - it causes rerender, useEffect is calling - alert starts showing, useEffect is calling - infinite loop.
To prevent looping in such a situation as yours, you can pass the second argument into the useEffect.
Rerender will be caused only when the second argument will become truthy. You passing there an alert and showAlert - they will always be truthy, because alert is storing a link.

So, you need to pass to useEffect only [showAlert]. When it becomes true - an alert will be shown without any looping.
I'm adding a link to the official documentation.

@amillward
Copy link
Author

Omitting alert as a dependency shows the warning; React Hook useEffect has a missing dependency: 'alert'. Either include it or remove the dependency array. (react-hooks/exhaustive-deps). I'm sure I could remove the eslint warning here, but I'd rather have a solution that doesn't require me to ignore warnings. https://overreacted.io/a-complete-guide-to-useeffect/#dont-lie-to-react-about-dependencies

@besLisbeth
Copy link
Collaborator

besLisbeth commented Jul 1, 2019

I found a solution for you in one of the React issues. Please, have a look into this sandbox in issue facebook/react#15204
However, my personal preference would be to disable this rule (maybe).

@amillward
Copy link
Author

amillward commented Jul 1, 2019

ah yes - I actually tried wrapping with useCallback before posting but I wrapped alert instead of alert.show. It works here; https://codesandbox.io/s/nostalgic-bardeen-jnudf

But would it be appropriate to wrap these functions within the react-alert module itself? It should work the same I believe. Otherwise anyone who wishes to show an alert as a result of an effect - I assume fairly common for things like errors loading the content on a page - has to create this arbitrary wrapper every time.

@besLisbeth
Copy link
Collaborator

It won't work the same, because your component is a functional component. On every re-render, the new component with the new link to the showAlert function will be created.
useCallback returns memoized function, so I suppose that it stores the link somewhere inside the React and it is always the same.

From my point of view, I'm considering the useEffect hook as 'componentDidUpdate' method of React class component, where we are passing properties comparison for preventing an internal loop.

I assume, that useCallback prevents the creation of the new functions. But it seems really ridiculous to me to pass it into the useEffect argument if look at it from the perspective of 'componentDidUpdate' method.

Anyway, I think, that you should take a look at these React issues: facebook/react#14920, facebook/react#15204 where people discuss the same problem as you have.

Also, I'm also closing this issue, because it's not the drawback of this particular repo.

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

No branches or pull requests

2 participants