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

Why useEffect's default behavior is to run on every render? #17454

Closed
elomid opened this issue Nov 25, 2019 · 3 comments
Closed

Why useEffect's default behavior is to run on every render? #17454

elomid opened this issue Nov 25, 2019 · 3 comments

Comments

@elomid
Copy link

@elomid elomid commented Nov 25, 2019

Do you want to request a feature or report a bug?
API design question about useEffect

What is the current behavior?
Currently useEffect runs on every render. This default behavior can be dangerous in situations like dealing with HTTP requests when you forget to pass the second argument. This seems to be a common mistake especially for newcomers like myself. I can't think of many (any) patterns where you want to run useEffect on every render. What was the reasoning behind not defaulting to run once?

@mwskwong
Copy link

@mwskwong mwskwong commented Nov 26, 2019

By the API design:

useEffect(
    () => {
        // do something
    },
    [/* dependency list */]
);

useEffect will run only when the dependency list changes, and empty array [] means no dependency (i.e. will only run once). It feels logical to me that undefined dependency list means run the function wrapped by useEffect every render.

@embeddedt
Copy link

@embeddedt embeddedt commented Nov 27, 2019

I can't think of many (any) patterns where you want to run useEffect on every render. What was the reasoning behind not defaulting to run once?

Suppose you are very new to React, and you want to do something whenever a useState variable changes (e.g. POST the new value to a server).

  • With the current behavior, you would POST the same value multiple times, but you would never miss posting a changed value.
  • With the behavior you're suggesting, only the initial value/first change would get POSTed, and further changes would not trigger the useEffect callback.

I think the dependency list is intended to be more of a performance optimization/aid to code simplification.

@bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Dec 2, 2019

The current API allows for you to decide between the following behaviors:

  • Run the effect every time. This mimics the old class componentDidMount + componentDidUpdate behavior.
  • Run the effect only once. This mimics the old componentDidMount behavior.
  • Run the effect only when the dependencies change. This is kind of new, although you could (and often did) implement the same thing by component this.props and prevProps in the class component API.

The "default" behavior you describe (when you don't explicitly specify the dependencies) is often the safest since it prevents stale values from being used in closures.

In the future, hopefully we will provide some type of compiler to help automate a lot of this away. In the meantime, we provide an official ESLint plug-in to help make it a little easier. 😄

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

Successfully merging a pull request may close this issue.

None yet
4 participants