[BUG] ref function gets called twice on update (but not on first mount), first call with null value.

[trusktr]

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

bug

What is the current behavior?

ref functions get called twice on update (but not on first mount).

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/84v837e9/).

I have some code like this:

render() {
  const frames = []
  frames.length = this.props.totalFrames

  for (let i=0, l=frames.length; i<l; i+=1)
      frames[i] = i

  this.frames = []

  return (
      <div>
          {frames.map(frame =>
              <div className="frame" key={frame} ref={el => {this.frames.push(el)}}>
              </div>
          )}
      </div>
  )
}

In componentDidMount, I can verify that this.frames contains this.props.totalFrames.

However, on componentDidUpdate, this.frames has a length of double this.props.totalFrame, where the first half of the items are all null. This means that subsequent render calls after the first are first passing null into the ref={el => {this.frames.push(el)} function for each element followed by the correct non-null values.

For example, if this.props.totalFrames is 3, then we observe this:

componentDidMount() {
  console.log(this.frames)
  // output:
  // [div, div, div]
}

componentDidUpdate() {
  console.log(this.frames)
  // output:
  // [null, null, null, div, div, div]
}

where this.frames is always twice as expected.

What is the expected behavior?

The array should not contain null values, only the divs.

NOTE, render() is not being called more than once, and I am setting this.frames to a new array inside render, then componentDidUpdate shows that it has double the length, with the first half of the values all null.

It seems like React is calling the ref functions one time too many for each frame element, the first call with a null value, the second call with the expected value.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

React 15.4.2

Workaround

As a workaround, I can simply and consistently filter() the null values out before interacting with the array.

[gaearon]

Please see https://facebook.github.io/react/docs/refs-and-the-dom.html#caveats:

[trusktr]

@gaearon So instead of fixing it the solution is to document the bad behavior?

[gaearon]

What do you mean by “fixing”? The behavior is intentional. If you pass an arrow function, referentially it is different every time. React has no way to know if it’s the same function in the code or not, so it has to “clean up” the old ref and “set up” the new ref just in case they turn out to be truly different functions. And cleaning up is necessary to avoid stale references (and memory leaks).

In most code this shouldn’t matter. Again, you can avoid it if you make the ref callback into a class property or something that doesn’t change on every render. If you’re running into a case where this does matter please describe it. Generally we only recommend setting a field in the ref callback, and if you’re doing something more sophisticated, it might be better to move this logic to a lifecycle hook.

[trusktr]

What do you mean by “fixing”? The behavior is intentional. If you pass an arrow function, referentially it is different every time. React has no way to know if it’s the same function in the code or not, so it has to “clean up” the old ref and “set up” the new ref just in case they turn out to be truly different functions. And cleaning up is necessary to avoid stale references (and memory leaks).

I don't know how it is implemented, and as an end user I probably don't need to know. I'm just saying that this is a bug. The function doesn't need to be called with null and then with the actual value. However you implement the fix doesn't matter from the outside perspective. I'm sure there's a way for the implementation to call it only once, with the proper value.

If you’re running into a case where this does matter please describe it.

I'm passing props directly to props, I wasn't desiring to have to write an extra class method each time I do that:

class SomeComponent extend React.Component {
  // ...
  render() {
    return <div>{this.items.map(i => <h1 ref={this.props.someFunction}></h1>)}</div>
  }
  // ...
}

I don't want to have to write an extra class method just to pass along a prop.

Also, the outer component that is using my component doesn't necessarily know how my component will use the function, and that outer component author shouldn't have to worry about the passed function being possibly and awkwardly called twice with null then expected values each render.

[trusktr]

It is documented, but it is still a bug.

[trusktr]

The behavior is intentional.

Why do you desire such behavior (having ref functions called twice each update, once with null, once with proper values). What benefit does that bring?

[gaearon]

I believe I have explained why it is intentional in the above comment:

If you pass an arrow function, referentially it is different every time. React has no way to know if it’s the same function in the code or not, so it has to “clean up” the old ref and “set up” the new ref just in case they turn out to be truly different functions. And cleaning up is necessary to avoid stale references (and memory leaks).

Just repeating it’s a bug is not a very productive way to frame this discussion. It is not a bug even if the behavior is a little confusing. As I mentioned it happens because you are technically passing a new function every time and so React has to clean up to avoid memory leaks and stale refs in case it really was a different function in the code. Unfortunately JavaScript does not give us a way to tell this with certainty so we have to play it safe.

If you want to call some function every time the ref update you can do this from componentDidMount and componentDidUpdate hooks of your component. Then you won’t expose this detail of API to the parent. But if it keeps a reference to the node then it might be a memory leak unless you call it with null when unmounting. Which React also does. At this point you can see that if the code is able to handle null on unmounting, it might as well do this before updates. I understand why this is surprising but I believe I explained above why this happens (referentially functions are different).

[trusktr]

If you pass an arrow function, referentially it is different every time. React has no way to know if it’s the same function in the code or not, so it has to “clean up” the old ref and “set up” the new ref just in case they turn out to be truly different functions. And cleaning up is necessary to avoid stale references (and memory leaks).

As I mentioned it happens because you are technically passing a new function every time and so React has to clean up to avoid memory leaks and stale refs in case it really was a different function in the code. Unfortunately JavaScript does not give us a way to tell this with certainty so we have to play it safe.

If I understand correctly, you're describing an implementation detail which is supposedly the reason why ref functions are called twice.

However, I'm saying that from an outer-API end-user perspective, I don't care about the implementation reason, the behavior is a bug from an outer-API design perspective.

If I read correctly, you're saying that the reason why it passes null is because the implementation behind the API is designed to try to prevent end-users of the React API from creating memory leaks.

This implementation detail that you are describing doesn't necessarily stop leaks. For example, take the following ref function:

<div ref={el => {if (!el) return; this.el = el}}></div>

Notice that passing null into this user-defined ref function does not prevent any memory leaks, and on top of that it caused the end-user to have to write an extra conditional check.

I believe that you're saying that passing null is likely to set the end-user's component's this.el property to null, forcing the old value to be collected, but this is not an assumption that is always correct, as I just showed in the last example.

In general, you can't really guarantee that some API makes the end-user's code 100% memory-leak free. At least, not with the current implementation and API surface. As with the vast majority of JavaScript libraries and framework in existence, preventing memory leaks is the job of the app developer when writing the app, and the library author on the library internals of the library used by the app developer, but the lines don't blend. Library authors can't really prevent app developers from writing memory leaks if the implementation only prevents memory leaks in a fraction of all use cases. If library implementation should prevent outer-API-side app-developer leaks, then it needs to prevent 100% of the leaks in all possible use cases of the outer-API, but it cannot guarantee 100% memory-leak prevention is areas outside of the API's control.

Currently, React ref function API doesn't control what goes inside a ref function, and therefore it isn't necessary to call ref functions with null; it only reduces possible end-user memory-leaks by some unknown percentage.

If you want to prevent memory leaks with refs in a more certain way, then go back to the deprecated string method for refs, make it read-only (f.e. freeze the refs object), and clean it up internally.

If you provide an API that accepts a user-defined function, you can't possibly assume you know how to prevent the end-user's memory leaks, and you can't guarantee that a user won't create a leak in it by doing something different than you expect.

Honestly, a novice programmer is going to make leaks, and it isn't going to be because of React.

You simply don't need to pass null to ref functions.

[trusktr]

On top of that, I'm willing to bet that Garbage Collection is really good these days, so if a ref function is only ref={el => this.el = el}, then if React loses a reference to whatever this is, then this.el will also be cleaned up. The only way that isn't possible is if the end-user is storing el somewhere else that persists outside of React, or if React is keeping references internally, but in simple cases like ref={el => this.el = el}, the elements will be collected just fine.

[trusktr]

I've never seen an API like this, that explicitly passes a null value every other call for no reason from an outer-API end-user perspective. It's not like the null value every needs to be used in a meaningful way by the end user.

It's like if I give you a function called doMath(n, callback) that accepts a number and calls back with an async result, except you have to guard your callback against null because it will always call it first with null then with the answer.

doMath(5, function(answer) {
  if (!answer) return // first call is null
  console.log(answer) // second call gives answer
})

That would be strange, just like this ref function behavior is.

[trusktr]

@gaearon I made the new issue because you closed this one, which hides the issue.

[gaearon]

I saw (and closed it). Sorry, but this is not how open source projects on GitHub generally work. I am happy to continue the discussion here, and allocate some time to answering to your comments.

Please consider that your issue is in direct competition with 565 open issues. I generally try to answer when I find the time, and you can just ping me on this thread again once in a while if you feel like the communication is not enough.

[gaearon]

I believe that you're saying that passing null is likely to set the end-user's component's this.el property to null, forcing the old value to be collected, but this is not an assumption that is always correct, as I just showed in the last example.

Yes, but at least it is possible to clean up. It “just works” in most simple cases (like you mentioned), and it is possible in more complex cases.

I will split this answer into two three explanations:

• Why we call ref with null when unmounting
• Why we call ref with null before the update when ref function changes
• Why we call ref with null before the update when ref is an arrow function

Why we call ref with null when unmounting

Consider an always-mounted grandparent component that passes a ref callback several levels deep to a list item (that can both mount and unmount). If we never notify the ref about that node unmounting, such grandparent has no way of cleaning up the ref at all. It would just hold onto that DOM node forever (since the grandparent never unmounts), creating a memory leak. And it’s not the grandparent component’s fault: if we don’t notify the ref about unmounting, we just don’t give it the tools to stop holding onto that node when it’s the time.

Since refs are first-class values, and are composable (they are functions after all!), we should expect some refs are coming from components that are not direct parents. In fact that was one of the motivations for making them functions.

A similar issue pops comes up when something “clones” a ref along the way. For example:

var child = this.props.children;
return cloneElement(child, {
  ref(node) {
    if (child.ref) {
      child.ref(node);
    }
    // Do something else with the node here
  }
});

In cases like this, we might want to run a side effect, set a field, or add a subscription—but none of these things are reversible unless we are also notified when the ref is detached. And since a ref can correspond to any component (or DOM node) arbitrarily deep in the tree, your own component lifecycle isn’t helpful to protect against such leaks.

Even in a simple case where you conditionally render something, relying on ref cleanup helps you free the DOM reference as soon as the subtree unmounts (e.g. on a state change), rather than when the component itself unmounts:

  <div>
    {this.state.showExpensiveSubtree &&
      <div ref={n => this.myRef = n}>
        <ExpensiveSubtree />
      </div>
    }
  </div>

This can be important in resource-constrained environments such as React Native.

Calling ref(null) is just one possible disposable mechanism. We could’ve come up with a more convoluted way to signal that the ref is empty, e.g.

<div attachRef={onAttach} detachRef={onDetach} />

or

<div ref={{attach() { }, detach(){ }}} />

In practice, though, using a single value for the attribute is simpler, and also makes it easier to spot places where there is a side effect, or a subscription, that gets added, but not removed, because that code would be colocated in one function. It isn’t too verbose for the common case (setting a field). And as a nice bonus it happens to nicely detach the field reference when you use it in a simple way with an assignment, no matter how deeply the ref owner and the child are separated by the component hierarchy. Ergonomics and good defaults are important.

Why we call ref with null before the update when ref function changes

Say ref={previousRef} is replaced by ref={nextRef} where previousRef and nextRef are two different functions. For the reasons explained above, it is important to at least notify the previousRef that the node is being detached. We can’t just “hope” it’ll be the same ref, it could be something like ref={this.props.isInSidebar ? this.props.sidebarInputRef : this.props.timelineInputRef} coming from different owner components.

If we accept that previousRef has to be cleaned up, then it makes sense to call it with null before calling nextRef with the instance (or the DOM node).

Just like if onChange={cond ? prevListener : nextListener} would detach the prevListener, and attach the nextListener, ref={cond ? prevRef : nextRef} detaches the prevRef and attaches the nextRef. And we already talked about why detaching refs is important.

Which brings us to...

Why we call ref with null before the update when ref is an arrow function

Because arrow functions have different referential identity inside each render() call. So we can’t know for sure if it’s the same ref or a different one.

Since there is no harm in detaching up the old one during the update, before attaching the new one, and since it is vitally important for some corner cases, we’re okay with calling them for arrow functions the way you described.

If it is annoying you, you can work around this by declaring them as a bound method on the class. In this case they would be referentially identical on every call, and React would not do the tiny bit of extra work detaching and attaching them because it would know the ref has not changed (but it would still call it with null when that node is unmounting).

I hope this helps.

[gaearon]

Regarding your question:

Also, the outer component that is using my component doesn't necessarily know how my component will use the function, and that outer component author shouldn't have to worry about the passed function being possibly and awkwardly called twice with null then expected values each render.

Consider this part of the ref callback contract. There are many different function contracts: a function that returns a Promise, a Node-style errback function, a reducer, etc. React ref callback is also such a contract, and we use null as a signal for disposal.

If it doesn’t make sense to expose this contract to the outer component, you can always wrap the function provided by the user, or the other way around, to modify the contract at the seam where it’s happening. But you could also make it clear that you’re exposing that contract by naming a prop with a Ref suffix. In fact, as I linked earlier, intentionally exposing ref callbacks as props is a fairly popular pattern—precisely because it guarantees the same contract, which people find useful.

[langpavel]

Future way: RFC #17 — New createRef() API for ref objects

[gaearon]

Object refs landed in 16.3 and solve this inconvenience for most cases:
https://reactjs.org/blog/2018/03/29/react-v-16-3.html#createref-api