React: `ref` callback attribute not behaving as expected

[remko]

It seems the ref callback isn't behaving as I expected it to, based on the docs.

The docs say that ref is called when the component is mounted (and all the other times it's called, it's called with null as a parameter). However, when I put a console.log in componentDidMount, and one in the ref callback, the one in the ref callback is called on every render(), whereas the componentDidMount is only called once. The component i'm reffing isn't even changing props.

Am I misunderstanding something? I'm using React 0.14.

var Value = React.createClass({
    componentDidMount() {
        console.log("Mounted");
    },
    render() {
        return <div>Dummy</div>;
    }
});

var Hello = React.createClass({
  getInitialState() {
    return { value: 0 };
  },

  componentDidMount () {
     setInterval(() => { this.setState({value: this.state.value + 1})}, 1000);
  },

  render: function() {
    return (
        <div>
        <span>{this.state.value}</span>
        <Value ref={(e) => { if (e) { console.log("ref", e); }}} />
      </div>
    );
  }
});

ReactDOM.render(<Hello/>, document.getElementById('container'));

[syranide]

This is intended (discussed elsewhere) but rather unintuitive behavior. Every time you render:

<Value ref={(e) => { if (e) { console.log("ref", e); }}} />

You are generating a new function and supplying it as the ref-callback. React has no way of knowing that it's (for all intents and purposes) identical to the previous one so React treats the new ref callback as different from the previous one and initializes it with the current reference.

PS. Blame JavaScript :P

[remko]

Oh, I see. So that's simple to work around, then, thanks!

Maybe the example in the docs should be updated, because it's now also using an inline callback to focus an element, which would have some weird effect on every render. I'm closing the issue though, since my issue is resolved.

[vitalybe]

I am confused, what's an easy work-around for this issue? Even if I do something like that ref={this.func.bind(this)} I get the same problem. Even worse, if I have a setState in func that I get an endless loop.

Since the functions have to be bound to access this the only solution I see to get the actual expected behavior is something like:

  func2 = function() { 
    console.log("got ref 2", this); 
  }.bind(this)

and then doing ref={this.func2}

This kind of mess makes ref callback rather annoying/unreliable to use.

I've made a jsbin of the issue here: https://jsbin.com/poboqo/1/edit?html,js,output

[remko]

@vitalybe I'm guessing this is why many React examples that use ES6 classes use trailing/leading underscores for method names, and bind them in the constructor to this (without the underscore). I'm also guessing this isn't a problem with React.createClass, which strangely enough would make createClass cleaner to use than ES6, but more knowledgeable people can tell me I'm wrong.

[goto-bus-stop]

I am confused, what's an easy work-around for this issue?

With ES6 class properties and arrow functions, you can do this:

class MyComponent extends React.Component {
  callback = () => {
    console.log(this)
  }

  render () {
    return <div ref={this.callback} />
  }
}

Here the callback function is added as a class property, which makes sure that the this binding is always the current instance. That's because doing callback = () => is roughly the same as:

constructor () {
  this.callback = () => {
    console.log(this)
  }
}

[gaearon]

@vitalybe What is your use case for refs? If you just set an instance field it should make absolutely no difference whether it is reattached on every render or not.

[vitalybe]

@goto-bus-stop Thanks for the snippet, that indeed looks better.

@gaearon The problem was that I was causing, by proxy, a setState() to be called by the ref callback function, causing a never-ending loop. If I was merely saving the element, it's true that it wouldn't matter much.

We've mentioned several solutions to this issue on the thread, I think that the main point, however is that its current behaviour is rather unexpected.

[gaearon]

We should probably throw early if you setState from a ref callback. I don't think this is meant to be a supported pattern.

[ffxsam]

@syranide Whoa. :beers: to you! So I should probably refactor all my code that looks like:

<div ref={node => node && this.node = node}>

[gaearon]

@ffxsam On the contrary, we give you null so that you don’t accidentally keep references to dead DOM nodes and components, potentially preventing them from getting garbage collected.

[gaearon]

Oh wait, I misunderstood your comment. Yes, you need to refactor from that pattern to a simple:

<div ref={node => this.node = node}>

[ffxsam]

<div ref={node => this.node = node}>

This doesn't work well for me though. When I use this pattern, as the OP said, the ref callback fires off every time the component renders. I should mention, that div is the top-level BTW:

render() {
  return <div ref={node => this.node = node}>

I don't know if that's the reason I was getting repeated ref callback invocations?

[gaearon]

the ref callback fires off every time the component render

Why is this a problem for you? This is the expected behavior.
It is expected because https://github.com/facebook/react/issues/6249#issuecomment-195412139.

[gaearon]

In case you’re worried about bottlenecks, it is extremely unlikely that setting a field is a performance bottleneck in your app. In the extreme case when it is, you can hoist it to be a bound class method (just like you do with event handlers), and you’ll avoid extra calls.

[ffxsam]

I understand why it's getting called upon re-renders. Sort of.. I'll get back to that in a second.

What you suggest (<div ref={node => this.node = node}>) is of course, perfectly fine. Except in my scenario where I'm relying upon the ref callback to trigger another callback to push information about the DOM node into an array. In this case, I would get several duplicates of the node because the ref callback is fired on re-render, _not_ just on DOM node mount. The obvious solution for my case is to use <div ref={this.somethingMounted}>.

What I don't understand is why the ref callback is called upon re-render. I get that it's a new function every time, but shouldn't the determining factor be whether the DOM node has already mounted or not? In other words, let's say I have this:

render() {
  return <div>
    <img ref={() => blahblah('abc')} src="..." />
  </div>
}

Rather than the internal React logic saying (upon re-render), "Hey, this is a new function, I should call it" - shouldn't it instead say "I've already invoked a ref callback for img, so I won't do it again"?

[gaearon]

Except in my scenario where I'm relying upon the ref callback to trigger another callback to push information about the DOM node into an array

Could you just do it in componentDidMount and componentDidUpdate instead?

What I don't understand is why the ref callback is called upon re-render.

shouldn't the determining factor be whether the DOM node has already mounted or not

Imagine this case:

<img ref={this.props.isAvatar ? this.handleAvatarRef : this.handleNormalRef} src="..." />

It’s a bit contrived but it illustrates the API makes it possible to pass different callbacks every time.
If it is possible then somebody will do it. (Maybe, with a layer of indirection, but still.)

If we didn’t clean up the old ref, it would be a terrible footgun the user actually intends to pass a different ref because we wouldn't respect it. So they would keep the old reference but not set a new one. This would make the initial mount behave inconsistently from update, which is against how other React APIs (e.g. props) work.

I hope this makes sense.

[ffxsam]

<img ref={this.props.isAvatar ? this.handleAvatarRef : this.handleNormalRef} src="..." />

Maybe I'm thinking about this wrong, but I would still expect this to fire only once. So if the component mounts and props.isAvatar is false, it would call this.handleNormalRef and that's it. I always pictured ref callbacks as a sort of componentDidMount but for individual DOM nodes, in that they only fire once the node in question is rendered for the first time. But again, maybe I'm thinking about it wrong the way.

Could you just do it in componentDidMount and componentDidUpdate instead?

I thought about that, but I need the actual DOM node (so I can run .getBoundingClientRect() on it), which would require me to do this:

componentDidMount() {
  const node = this.refs.rootNode;
  this.props.onMount({ rect: node.getBoundingClientRect() });
}

render() {
  return <div ref="rootNode"> ... </div>
}

To provide some context: I'm building a component that renders file icons and folders (each as a React component), and each <Folder> component reports to its parent upon mounting, so it knows the rect coordinates for each folder icon. That's used when dragging files to know whether the user is hovering over a folder or not (and which one).

[gaearon]

So if the component mounts and props.isAvatar is false, it would call this.handleNormalRef and that's it.

That's not how React works generally. There is no other case where "initial props" are in any way special to "updated props". Usually React can update any prop but this would be a case where the first value "gets stuck".

I thought about that, but I need the actual DOM node (so I can run .getBoundingClientRect() on it), which would require me to do this:

Why use string refs there?

componentDidMount() {
  this.props.onMount({ rect: this.rootNode.getBoundingClientRect() });
}

render() {
  return <div ref={node => this.rootNode = node}> ... </div>
}

[ffxsam]

Oh. Ok, yeah, that makes a lot of sense. 😁 So componentDidMount is fired off after all the ref callbacks have been executed? That would totally work.
(dumb code removed)

Thanks for clarifying all this, Dan!

[gaearon]

So componentDidMount is fired off after all the ref callbacks have been executed?

Yes.

Except when the component is unmounting, where ref gets called again with a null value.

No, this is not necessary.
How could you get componentDidMount when a component is unmounting?

[ffxsam]

Er, no - I know this. I know React! 😆 Sorry, I replied in haste.

[Intregrisist]

I am seeing something strange about this. This is what I am understanding:

If a component updates it will simply call ref with a reference to the DOM element. The only time ref is called with null is when it unmounts. If I am following correctly, then why is this occurring:

https://jsfiddle.net/dflores009/rwpfrge0/

Every time the Controlled input get's updated with a new value, it get's called with null and instantly calls it again with a reference to the DOM element. Shouldn't ref always be called with a reference to the DOM since it did not unmount. You can see that componentWillUnmount was never called as you type and it updates the state.

[jedmao]

Straight from the docs:

React will call the ref callback with the DOM element when the component mounts, and call it with null when it unmounts.

There are only two references to null in that page of documentation and the other is a code example, so @Intregrisist is absolutely correct. In his example, null is passed on every key press of the text box, even though the component is never unmounted.

Either the documentation needs to be changed to clarify this is expected behavior or something needs to be fixed here. I hope we can all agree the latter is the case, because this would be very weird expected behavior!

[unimonkiez]

Oh wait, I misunderstood your comment. Yes, you need to refactor from that pattern to a simple:

<div ref={node => this.node = node}>

Or just

<div ref={node => { this.node = node}}>

to avoid add an unnecessary return to your function 😆

Also thanks for clearing that up, been looking in the documentation about that, also thought ref acts like componentDidMount.

[gaearon]

The docs were updated, just not published to website yet. See last section.

https://github.com/facebook/react/pull/8707

[gaearon]

@jedmao @Intregrisist

Have you had a chance to read this thread? Yes, the docs were incomplete, but I believe it has been explained why this is happening.

The explanation is both in https://github.com/facebook/react/issues/6249#issuecomment-195412139 and our last conversation with @ffxsam.

I hope this helps.

[zzxoto]

You can supply the "bound" callback method instead so that the javascript does not inefficiently recreate the callback method and call it on every render.