Material-ui: setRef is possibly leaky

Created on 7 Nov 2018 · 4Comments · Source: mui-org/material-ui

Did some experimenting after reading reactjs/reactjs.org#834 and it turns out that changing a *ref prop can indeed cause a memory leak if it is handled by utils/reactHelpers#setRef. React will automatically clean up every ref attached to a node on update/unmount but every ref that is not attached to the node will still hold a reference to that node.

Live example: https://codesandbox.io/s/pkm11r8kjx

We would need to bookkeep every *ref prop that is passed to one of our components during its lifecycle and clean it up on cDU and cWU.

Not sure if this has any actual impact. Just documenting it in case someone actually encounters this issue.

discussion

Source

eps1lon

Most helpful comment

A fix naturally emerges by using a custom hook:

// useForkRef.js
export default function useForkRef(refA, refB) {
  /**
   * This will create a new function if the ref props change.
   * This means react will call the old forkRef with `null` and the new forkRef
   * with the ref. Cleanup naturally emerges from this behavior
   */
  return useCallback(
    refValue => {
      setRef(refA, refValue);
      setRef(refB, refValue);
    },
    [refA, refB]
  );
}
// usage.js
function Component({ buttonRef: buttonRefProp }) {
  const buttonRef = React.useRef();
  const handleRef = useForkRef(buttonRef, buttonRefProp);

  React.useEffect(() => buttonRef.current.focus(), []);

  return <button ref={handleRef} />
}

The codesandbox is updated.

Once every component that uses setRef is a function component we can use this hook instead and make setRef private.

eps1lon on 1 Apr 2019

👍3

All 4 comments

A fix naturally emerges by using a custom hook:

// useForkRef.js
export default function useForkRef(refA, refB) {
  /**
   * This will create a new function if the ref props change.
   * This means react will call the old forkRef with `null` and the new forkRef
   * with the ref. Cleanup naturally emerges from this behavior
   */
  return useCallback(
    refValue => {
      setRef(refA, refValue);
      setRef(refB, refValue);
    },
    [refA, refB]
  );
}
// usage.js
function Component({ buttonRef: buttonRefProp }) {
  const buttonRef = React.useRef();
  const handleRef = useForkRef(buttonRef, buttonRefProp);

  React.useEffect(() => buttonRef.current.focus(), []);

  return <button ref={handleRef} />
}

The codesandbox is updated.

Once every component that uses setRef is a function component we can use this hook instead and make setRef private.

eps1lon on 1 Apr 2019

👍3

@eps1lon I believe that we solve the problem correctly now. Do we need the issue to be open? Is there any work left to be done?