React: useEffect(effect, [ref.current]) is prematurely re-running

Created on 4 Dec 2018  路  16Comments  路  Source: facebook/react

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

What is the current behavior?
It seems useEffect(effect, [ref.current]) is re-running under certain circumstances without the current ref identity changing.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React:
https://codesandbox.io/s/qkzl9xjj44

What is the expected behavior?
In this example, the effect should only be run once, not re-run (and thus cleaned up) upon clicking the button for the first time. The isActive state should be returned to false on mouseup. CLEAN UP! should not be logged to the console since the ref hasn't been reassigned.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
16.7.0-alpha.2 (affects all environments)

Hooks Discussion

Most helpful comment

@anion155 This is not a correct solution. If you want to use state, just use state directly. Refs are for mutable values that are _intended_ to be mutated outside the render data flow. If you want them to cause a re-render, you shouldn't be using a ref.

I'm going to close this thread. As I mentioned earlier, if you put [ref.current] in dependencies, you're likely making a mistake. Refs are for values whose changes don't need to trigger a re-render.

If you want to re-run effect when a ref changes, you probably want a callback ref instead. This is described here, for example.

All 16 comments

I don't believe this is actually a bug. The problem is, ref is not actually defined when you pass it in, but since you're passing a reference that gets assigned, it actually gets assigned when the effect kicks off.

Where you have [ref.current] in your useEffect call (line 28), try replacing it with this and and you'll see what I mean:

[
  (() => {
    console.log("ref to compare with", ref.current);
    return ref.current;
  })()
]

The ref is undefined when you first pass it because the component hasn't actually mounted, and thus it has nothing to reference.

This simple solution is to this is to use useLayoutEffect, since that will ensure that the ref is in fact referencing the DOM node.

The signature [useLayoutEffect] is identical to useEffect, but it fires synchronously after all DOM mutations.

Ah, right. React checks the effect dependencies during render, but defers running the effect until after commit/paint. So it doesn't recheck the deps until the next render, which happens after setIsActive(true).

So while I guess this isn't a bug per se, it still might be something that could be addressed. Maybe after commit, React could re-run the render function to update the dependencies (but not actually re-render) if it's not too expensive and doesn't cause any other unwanted side effects. I did notice useLayoutEffect happened to skirt around the issue in this particular example, but I'm more concerned with fixing this unintuitive behavior if possible than fixing the implementation of that hook.

Since this is only a problem with mutable values used as deps, and mutable values should be captured inside of a ref, perhaps React can special case the dependency diffing for refs to check the current value. So you would call useEffect(effect, [ref]) and React would know to compare the previous value to whatever ref.current is.

We'll need to add this to FAQ, it's an unfortunate confusing aspect. [ref.current] (or any mutable field) is going to be problematic when included in dependencies because there's no guarantee it would actually be changed before rendering (or that changing it would cause a rendering). In case of React refs, they get modified after rendering which is why it always lags behind.

We're working on a lint rule for this but let's keep this open so we don't forget to update the docs too.

So I also ran into this issue on my first attempt at a custom hook.

One solution is to use the old ref callback method instead. This allows you to store the ref node in a useState, so you can add it to the useEffect update dependencies.

import React, {useState} from 'react'

function useHookWithRefCallback() {
  const [node, setRef] = useState(null)

  useEffect(
    () => {
      if (node) {
        // Your Hook now has a reference to the ref element.
      }
    },
    [node],
  )

  return [setRef]
}

function Component() {
  // In your component you'll still recieve a `ref`, but it 
  // will be a callback function instead of a Ref Object
  const [ref] = useHookWithRefCallback()

  return <div ref={ref}>Ref element</div>
}

I ended up making a Medium post about the problem: https://medium.com/@teh_builder/ref-objects-inside-useeffect-hooks-eb7c15198780

Just to be clear here, useRef _does_ invalidate the component and cause a re-render when the ref is bound by react, correct?

(If not, I suppose it could be used in conjunction with useState and a Proxy, similarly to @thebuilder's callback-based workaround above.)

EDIT: Nope, it appears that it does not, and the actual createRef definition is far less magical than I was expecting 馃え... This seems quite easy to solve on my side, but it really makes me question what the use of useRef really is.

I'm now really struggling to understand why useRef is a thing when (as far as I can tell) this would have the exact same effect:

const [ ref ] = useState({current: null})

I had assumed useRef worked more like this:

function useCustomRef<T>(d: T): { current: T } {
  const [value, setValue] = useState<T>(d);
  const [ref] = useState({
    set current(value) {
      setValue(value);
    },

    get current(): T {
      return value;
    }
  });

  return ref;
}

No, useRef works more like this:

function useRef(initialValue) {
  const [ref, ignored] = useState({ current: initialValue })
  return ref
}

So setting the current field would never trigger a re-render.

When you try to put ref.current in dependencies, you usually want a callback ref instead: https://reactjs.org/docs/hooks-faq.html#how-can-i-measure-a-dom-node

edit: I totally responded to the wrong thread. My bad, apologies for wasting time.

Worked for me:

function useCorrectRef(initial) {
  const [value, setValue] = useState(initial);
  const [ref, setRef] = useState({ current: initial });
  useEffect(() => {
    setRef({
      get current() {
        return value;
      },
      set current(next) {
        setValue(next);
      }
    });
  }, [value]);
  return ref;
}

@anion155 This is not a correct solution. If you want to use state, just use state directly. Refs are for mutable values that are _intended_ to be mutated outside the render data flow. If you want them to cause a re-render, you shouldn't be using a ref.

I'm going to close this thread. As I mentioned earlier, if you put [ref.current] in dependencies, you're likely making a mistake. Refs are for values whose changes don't need to trigger a re-render.

If you want to re-run effect when a ref changes, you probably want a callback ref instead. This is described here, for example.

@gaearon Okay. Than if I would need to pass canvas to some couple of hand made hooks, to create some objects in imperative style, will this code be correct? (I know it's not SO, but it was a question I made this kind of solution):

function useViewFabric(transport: Socket | null, canvas: HTMLCanvasElement | null) {
  const [view, set] = useState(null);
  useEffect(() => {
    if (transport && canvas) {
      const created = new View(transport, canvas);
      set(created);
      return () => created.dispose();
    }
  }, [transport, canvas]);
  return view;
}

function CanvasView() {
  const [canvas, setCanvas] = useState(null);
  const transport = useContext(TransportContext);
  const view = useViewFabric(transport, canvas);
  return <canvas ref={setCanvas} />;
}

function App() {
  return <><CanvasView /><CanvasView /><>;
}

Or is there a better approach?

As I mentioned earlier, if you put [ref.current] in dependencies, you're likely making a mistake. Refs are for values whose changes don't need to trigger a re-render.

This is great insight. Using the callback ref pattern seems to solve this issue. I made a const [node, ref] = useNodeRef() hook to give me a proper reference to the node. I've updated the example here:
https://codesandbox.io/s/sxkq6

Thanks @gaearon!

EDIT: useNodeRef is functionally equivalent to useState ...!
You could just as well do const [node, ref] = useState(). Maybe this is bad?

As I mentioned earlier, if you put [ref.current] in dependencies, you're likely making a mistake. Refs are for values whose changes don't need to trigger a re-render.

@gaearon Not sure if this is a dumb question, but should you include a ref in the dependencies if it's a forwardRef argument? I searched around but couldn't find an answer.

I figured the answer was no, so I excluded it. However, the react hooks linter is saying that I should be including the ref argument from forwardRef as a dependency.

I understand the plan is to put ref back into props (which would be great). But even in that case, is the linter right?

See Sandbox link. Line 19 shows the linter error.

What I'm intending to do is have a chat library where users can pass in a ref and then do multiple methods on it (scroll to end of ScrollView, focus the TextInput, etc.) I'm doing this by setting the forwarded ref.current in a useEffect with no dependencies ([]).

Should I not use an effect at all and simply edit ref.current in the main component scope?

Thank you!

@anion155 This is not a correct solution. If you want to use state, just use state directly. Refs are for mutable values that are _intended_ to be mutated outside the render data flow. If you want them to cause a re-render, you shouldn't be using a ref.

I'm going to close this thread. As I mentioned earlier, if you put [ref.current] in dependencies, you're likely making a mistake. Refs are for values whose changes don't need to trigger a re-render.

If you want to re-run effect when a ref changes, you probably want a callback ref instead. This is described here, for example.

@gaearon I think this is not always true. useRef represents two different concepts: instance-like variable for functional component and reference to mounted component itself.

For example, I have this custom hook:

// file1.ts
export function useImmerRef(initialValue: any, deps?: DependencyList) {
  const valueRef = useRef<any>(null);
  const updateFunction: any = useMemo(
    () => {
      valueRef.current = produce(initialValue, (_draft: any) => {});
      return (updater: any) => {
        const newState = produce(initialValue, updater);
        valueRef.current = newState;
        return newState;
      };
    },

    // eslint-disable-next-line react-hooks/exhaustive-deps
    deps
  );
  return [valueRef, updateFunction];
}

// file2.tsx
const [tagsRef, updateTags] = useImmerRef(tags, []);

Here I'm using useRef as instance variable and it's different from reference to mounted component.

  1. It's never get assigned null value
  2. We need to use useRef to be able to substitute .current to new reference, so changes would be observable (this is the problem with implementation using useState)

If I use original useImmer implementation which internally uses useState - reference can be updated only on next rendering on this line

// tags is Map instance
const [tagsState, updateTagsState] = useImmer(tags)

otherwise it's not possible to update tagsState reference. That leads to bug that changes are not observable:

updateTagsState(draft => {
  const tag = draft.get(data.id);
  if (tag !== undefined) tag.isSelected = data.isSelected;
});
// Here tagsState has reference to the old structure, 
// it's not possible to observe changes until next render
const tag = tagsState.get(data.id);

I think it's totally valid to use ref.current as dependency in this case. But it turns out it's not working as I expected.

Was this page helpful?
0 / 5 - 0 ratings