React: useEffect: separate refreshing dependencies from running effect

[gnordhielm]

Do you want to request a feature or report a bug?

Looking for a clarification.

---

useEffect assumes a connection between a change in dependencies and the effect running.

Following the exhaustive dependencies rule, I'm running into quite a few cases where I'm stuck in a confusing middle space.

For example:

useEffect(() => {
  if (thisCommentId === focusedCommentId && isCommentCollapsed) 
    setIsCommentCollapsed(false)

}, [focusedCommentId, thisCommentId, isCommentCollapsed, setIsCommentCollapsed])

The idea here: a comment is specified as "focused" in some higher-level state, when that happens, I want to make sure that comment, when rendered, is not "collapsed".

Now, I can't collapse this comment while it's focused, because the effect will fire and set collapsed to "false" whenever a new collapsed value comes through. I can't avoid passing it through, because it's a legitimate dependency of the effect. But it's also not a reason to run the effect.

I suspect that my whole approach to this is not quite aligned with how hooks are meant to be used. Does the question "how to I separate dependencies from re-run conditions?" reveal a problem in this approach? Otherwise - what's the right way to do this?

[audiolion]

putting this in a small, focused, codesandbox (https://codesandbox.io) will make it much easier to debug your issue

[brunolemos]

I want to make sure that comment, when rendered, is not "collapsed"

Sounds like you want it to run only on the first render. If that helps, you can think of it being similar to componentDidMount, not componentDidUpdate.

Try this instead:

useEffect(() => {
  if (thisCommentId === focusedCommentId && isCommentCollapsed) setIsCommentCollapsed(false)
}, [])

You can also try some alternatives based on the behavior you want, for example:

useEffect(() => {
  if (thisCommentId === focusedCommentId && isCommentCollapsed) setIsCommentCollapsed(false)
}, [thisCommentId === focusedCommentId])

[gnordhielm]

@audiolion I'm less looking to debug an issue, more looking to get better at "thinking in hooks". The behavior I'm seeing is totally expected and understandable, but I'm also missing an alternative hooks pattern.

[gnordhielm]

@brunolemos I'm hoping to move out of a lifecycle mindset and into a synchronization one (https://overreacted.io/a-complete-guide-to-useeffect/#synchronization-not-lifecycle).

I do feel like your second example is a lot better though, I can be more explicit about what the effect is trying to do that way:

const hasFocus = thisCommentId === focusedCommentId
useEffect(() => {
  if (hasFocus && isCommentCollapsed) setIsCommentCollapsed(false)
}, [hasFocus, isCommentCollapsed, setIsCommentCollapsed])

The working solution I have now looks like this:

const hasFocus = thisCommentId === focusedCommentId
useEffect(() => {
  if (hasFocus && isCommentCollapsed) {
    setFocusedCommentId(null)
    setIsCommentCollapsed(false)
  }
}, [hasFocus, isCommentCollapsed, setIsCommentCollapsed, setFocusedCommentId])

It's declarative, but feels like an iffy representation of the desired behavior (and a little risky).

[brunolemos]

You don't need to specify setIsCommentCollapsed and setFocusedCommentId as deps if they are coming from useState.

[malerba118]

I've been fiddling around with some delta utilities to diff values across renders. I'm not quite sure what the accepted opinion is on accessing values from previous renders in a current render. It potentially feels a little dirty, but I'm not sure whether it's an anti-pattern or not.

Anyhow, I've realized useEffect can be a little dangerous if you don't specify all dependencies in the dependency array, so i've been experimenting with providing no dependency array at all (letting the effect run after each render). Then inside of the callback I can do some conditionals to determine whether or not to run my business logic.

  const [essay, setEssay] = useState("");
  const essayDelta = useDelta(essay);

  // If essay length changed significantly across a single render,
  // student may have copied and pasted someone else's work.
  useEffect(() => {
    if (
      essayDelta &&
      String(essayDelta.curr).length - String(essayDelta.prev).length > 15
    ) {
      console.warn("Student may have copied and pasted part of essay");
      // ... Post warning to server ...
    }
  });

https://codesandbox.io/s/ovlr14l9l6

[brunolemos]

One thing I do (more often than I should) is use useRef so I can use useEffect with [] and still have access to the latest value.

[robertknight]

The idea here: a comment is specified as "focused" in some higher-level state, when that happens, I want to make sure that comment, when rendered, is not "collapsed".

Is the comment collapsed status a piece of local state? Does collapsing a comment involve any side effects other than changing a piece of state?

Assuming the collapsed status is a piece of local state and collapsing a comment doesn't involve any external effects, then you can use the focused status from the parent to initialize the collapsed state (const collapsed = useState(!isFocused)).

As I understand it, effects are mainly intended for when you need to interact with the world outside of the component in some way - for example: Subscribing to a data source, fetching some data from somewhere, scheduling an animation, calling methods on a DOM node. If you want to derive initial state from props, then the argument to useState is for that.

[gnordhielm]

The collapsed status is totally local to the comment, yes.

For an deriving the initial state, what you're suggesting would work, the trouble is I want to expand the comment when it becomes focused - which means that I want to run that derivation any time the focused comment changes.

[robertknight]

Ah, so you want the hooks equivalent of getDerivedStateFromProps? - The docs have a recommendation on that: https://reactjs.org/docs/hooks-faq.html#how-do-i-implement-getderivedstatefromprops

[gnordhielm]

Agh, yeah that example totally covers this use case. I've been trying to block the getDerivedStateFromProps pattern from my mind as it's usually a symptom of poorly designed state - that certainly could be the case here. Thank you!

I'm going to leave this open for a sec on the off chance someone on the react team can offer a high-level explanation of the connection between a variable which is a dependency of an effect, and the assumption that its changing should fire the effect.

[malerba118]

https://github.com/malerba118/react-delta

I just released V1 of a library aimed to give more control over when effects run. It's an expansion of the useDelta idea I mentioned above in this thread. I think it's worth checking out, even just as a thought experiment. The overall idea is to give the developer the previous, current, and latest value of a variable in relation to a render, and let them run effects by diffing the values.

[mthurlin]

Even though the use-case in this issue could be solved differently, I still feel the gist of the question is not really answered.

Is it OK to use effect dependencies to control when the effect trigger, rather than for dependency management?

Example:

// Scroll to top when route changes
useEffect(() => {
   ref.current.scroll(0, 0);
}, [currentRoute]);

Is it a semantic guarantee that effects are triggering only when dependencies change? Or "just" a performance hint that React might chose to ignore in the future, like useMemo etc?

Should we prefer to use something like useDelta mentioned above? Or usePrevious:

// Scroll to top when route changes
const routeChanged = usePrevious(currentRoute) !== currentRoute;
useEffect(() => {
   if (routeChanged) {
       ref.current.scroll(0, 0);
   }
});

[robertknight]

Is it a semantic guarantee that effects are triggering only when dependencies change? Or "just" a performance hint that React might chose to ignore in the future, like useMemo etc?

My understanding is that the answer to the first question is _yes_, since useEffect's callback can invoke arbitrary user-defined effects, which may not be un-doable or idempotent or may have user-visible results each time they run. Therefore unlike the useMemo callback, it may not be safe to, for example, randomly run it on render sometimes even if dependencies have not changed.

[mthurlin]

While the current implementation seems to guarantee it, the docs are hinting of a different vision:

In the future, a sufficiently advanced compiler could create this array automatically.

Also, it is explicitly talked about as an optimization, never as a semantic guarantee:

However, this may be overkill in some cases, like the subscription example from the previous section. We don’t need to create a new subscription on every update, only if the source props has changed.

If you use this optimization, make sure the array includes all values from the component scope (such as props and state) that change over time and that are used by the effect.

[malerba118]

My conclusions

1. As mentioned, useEffect callbacks run as a semantic guarantee in contrast to useMemo and useCallback.

2. Caveat to number 1, if you pass a memoized value into the useEffect dependency array, by extension you cannot know exactly when the effect will run because you cannot know exactly when the memoized value will update. For this reason, I tend to prefer useRef to useCallback and useMemo.

function Foo ({someFunc}) {
  const memoizedFunc = useCallback(someFunc, [])

  useEffect(() => {
    // This runs whenever memoizedFunc changes, 
    // which is not a semantic guarantee
  }, [memoizedFunc])
}

1. Using conditionals inside of effects is dangerous if you're returning cleanup functions. Consider the following example. At first glance, the two implementations appear equivalent, but in fact the one that uses a conditional inside of useEffect runs the cleanup logic at the wrong time.

function CounterOne() {
  const [count, setCount] = useState(0)

  useEffect(() => {
    const id = setInterval(() => {
      setCount(prev => prev + 1)
    }, 1000)
    return () => clearInterval(id)
  }, [])

  return (
    <div>
      CounterOne: {count}
    </div>
  );
}

function CounterTwo() {
  const firstRenderRef = useRef(true)
  const [count, setCount] = useState(0)

  useEffect(() => {
    // run this only on the first render
    if (firstRenderRef.current) {
      firstRenderRef.current = false
      const id = setInterval(() => {
        setCount(prev => prev + 1)
      }, 1000)
      return () => clearInterval(id)
    }
  })

  return (
    <div>
      CounterTwo: {count}
    </div>
  );
}

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[stale[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!