React: Bug: button onFocus not called when focus() called from useEffect cleanup

[fyodore82]

React version: 16.12.0

Steps To Reproduce

1. Have a <button/> HTML element with onFocus event handler defined

<button ref={btnRef} onFocus={() => console.log("I'm in focus")}>Focus Target</button>

1. Have another component with useEffect cleanup code which will call focus()

React.useEffect(() => {
  const ref = btnRef.current;  // btnRef is a ref to the button component above
  return () => {
    console.log("effect cleanup ", new Date().toISOString());
    ref.focus();
  };
}, []);

When the cleanup code from useEffect is called, onFocus event handler of the button is not called. The button is rendered in a component which is mounted when useEffect cleanup executes. For example it can be in the parent component.

Link to code example: https://codesandbox.io/s/onfocus-not-called-from-useeffect-cleanup-npw7k

1. Open example in browser (tested in Chrome 79.0.3945.88 and in Firefox 72.0.1).
2. Open dev console
3. Click on Focus target button. See another component below it with two buttons.
4. Remove focus from Focus target button by setting it in adjacent input field
5. Click on Set focus button. Observe that focus is set to Focus target button and in console see message 'I'm in focus...'. This message is logged from onFocus event handler
6. Remove focus from Focus target button.
7. Click Close me button. Observe that focus is set to Focus target button but no message in console from onFocus event handler

The current behavior

onFocus is not called

The expected behavior

onFocus is called

[vkurchatkin]

It also seems that this is only broken when the effect is unmounting, but works fine when it is updating:

// Helper component to trigger a function when either k changes, or it is unmounted
function OnUnmount({ fn, k }) {
    React.useEffect(() => {
      return () => {
        fn();
      };
    }, [k])

  return null;
}

export default function App() {
  const inputRef = React.useRef();
  const [state, setState] = React.useState(0);

  function run() {
    setState(s => s + 1);
  }

  return (
    <div>
      <button onClick={run}>Run</button>
      <OnUnmount fn={() => {
        inputRef.current.focus();
      }} key={state}/> //
      <input ref={inputRef} onFocus={() => {
        console.log('onFocus');
      }}/>
    </div>
  );
}

In this example the input isn't even focused at all, but if you change key={state} to k={state} it works fine, although there is seemingly no reason for such a discrepancy.

It goes without saying that adding setTimeout(0) fixes things.

[ruslan-shuster]

The problem is a bit wider than that: any dom events triggered using ref will not work, and the reason is the following:

1)When the DOM modification happens, such as on unmount of the buttons from the example, eventually commitRootImpl is being called. This is where React applies DOM modifications.

2)As part of the prepareForCommit, React disables browser event emitter.

3)The cleanup function returned from the callback of useEffect hook triggers browser focus event using ref, and if you attach any non-React event listeners to the element it will be triggered, but React event is not being dispatched because event emitter is not enabled. Thus, React suppresses focus event.

4)Then, React enables event emitter back in resetAfterCommit, but its too late, because the focus event is already lost.

As a result, you see that the element is in focus in your browser, but proper React callback was not called. And, it explains why setTimeout(0) works.

I hope my investigation will be helpful to React core team, and I'm ready to help with the solution implementation, once there is an agreement about what to do about it.

[stale[bot]]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[fyodore82]

bump

[gaearon]

It seems like the original intent was to suppress events caused by DOM mutations but because cleanup fires synchronously, this suppresses the focus too.

However, we have already changed the behavior in master to fire useEffect cleanup asynchronously, which accidentally fixes this issue (to be included in React 17). I think it would still be relevant for synchronous cleanup (such as useLayoutEffect) but I'm not sure if we're going to fix that case. I'll keep it open for more discussion.

[benoitgrelard]

Hey all,

I think I have stumbled on the same bug but also discovered another weird behaviour in relation to this.
Both issues I'm seeing here (the event one already mentioned, + the new one) appear to have been fixed in v17.
That said, I thought I'd report it anyway because of the second behavior I saw, as I don't know if the react team is aware.

I have setup a sandbox to demonstrate the issue.

• The black box has an onFocusCapture listener.
• I have also setup a native focus listener (capture phase too) on the document.
• Pressing the "Open blue box" button opens a blue box component below.
• When pressing the "Close blue box" button, the blue box component gets unmounted, and we move focus to the input (in an effect on unmount).

latest stable react 16.13.1

https://codesandbox.io/s/react-unmount-focus-bug-0qn8q

Here's what we can observe:

• First of all, at first it looks like focus wasn't moved to the input but instead remained on the button.
• If we take a closer look at the events we can see:
  
  
  
  • When unmounting, the react onFocusCapture event was never called (same issue as discussed in this thread I think), we only see the native one.
  
  
  • But we also see that the focus WAS indeed moved to the input as we wanted, BUT react moved it back to the input for some reason (this is the other issue I have been mentioning).
  
  

react 17.0.0-rc.3

Here is the exact same sandbox, but with react updated to v17, you can see both issues seem to be resolved:

• The input ended up focused.
• The series of events is correct.

https://codesandbox.io/s/react-unmount-focus-bug-fixed-in-v17-t7zcw

---

Let me know if that weird re-focusing behavior was known or not. Happy to see if fixed in 17 but wasn't sure if it was accidental like @gaearon mentioned.

✌️