react: [BUG] ref function gets called twice on update (but not on first mount), first call with null value.
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.
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Comments: 16
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
twothree explanations:refwithnullwhen unmountingrefwithnullbefore the update whenreffunction changesrefwithnullbefore the update whenrefis an arrow functionWhy we call
refwithnullwhen unmountingConsider 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
refabout 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:
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
refcleanup 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: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.or
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
refwithnullbefore the update whenreffunction changesSay
ref={previousRef}is replaced byref={nextRef}wherepreviousRefandnextRefare two different functions. For the reasons explained above, it is important to at least notify thepreviousRefthat the node is being detached. We can’t just “hope” it’ll be the same ref, it could be something likeref={this.props.isInSidebar ? this.props.sidebarInputRef : this.props.timelineInputRef}coming from different owner components.If we accept that
previousRefhas to be cleaned up, then it makes sense to call it withnullbefore callingnextRefwith the instance (or the DOM node).Just like if
onChange={cond ? prevListener : nextListener}would detach theprevListener, and attach thenextListener,ref={cond ? prevRef : nextRef}detaches theprevRefand attaches thenextRef. And we already talked about why detaching refs is important.Which brings us to…
Why we call
refwithnullbefore the update whenrefis an arrow functionBecause arrow functions have different referential identity inside each
render()call. So we can’t know for sure if it’s the samerefor 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
nullwhen that node is unmounting).I hope this helps.
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.
I believe I have explained why it is intentional in the above comment:
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
componentDidMountandcomponentDidUpdatehooks 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 withnullwhen unmounting. Which React also does. At this point you can see that if the code is able to handlenullon 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).Regarding your question:
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
nullas 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
Refsuffix. 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.I’ve never seen an API like this, that explicitly passes a
nullvalue every other call for no reason from an outer-API end-user perspective. It’s not like thenullvalue 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 againstnullbecause it will always call it first withnullthen with the answer.That would be strange, just like this ref function behavior is.
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 whateverthisis, thenthis.elwill also be cleaned up. The only way that isn’t possible is if the end-user is storingelsomewhere else that persists outside of React, or if React is keeping references internally, but in simple cases likeref={el => this.el = el}, the elements will be collected just fine.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
nullis 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:
Notice that passing
nullinto 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
nullis likely to set the end-user’s component’sthis.elproperty tonull, 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
refsobject), 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
nullto ref functions.