Bug: onChange handler is lost between re-renders

This issue has been tracked since 2022-09-16.

React version: 18
In our application, we have a checkbox and a button on a page. The button opens a popup which in turn attaches a onClick event of type capture at body level.
Now, when we click on the checkbox we observed the onChange handler never gets a chance to execute.

Steps To Reproduce

  1. Click on the button to open a popup.
  2. popup attaches a click handler at body level. This event is of type 'capture' to close the popup whenever some random click happens anywhere on the page.
  3. Now, if we click on the checkbox the body level click event is executed before the checkbox's onChange event.
  4. This click event in turn updates some state, which causes re-render of the checkbox component.
  5. popup is closed.
  6. The onChange event attached with the checkbox never got a chance to be executed. It is lost somewhere.

Link to code example:

The current behavior

The checkbox remains either checked or unchecked.

The expected behavior

The checkbox should get checked and unchecked with every other click.

umerprogramm wrote this answer on 2022-09-16

Hey @subha84 I'm interested to work on it
So right now, you are working on it or not?

subha84 wrote this answer on 2022-09-16

Yes, right now working on it. @umerprogramm

subha84 wrote this answer on 2022-09-17

@eps1lon @gaearon Any ideas ?

LuBustos wrote this answer on 2022-09-17

Hey @subha84! I saw you error in the codesanbox if you remove the third option in the event listener method, it seems to work.
The useEffect should look like this:

  useEffect(() => {
    let onclick = function (event) {
      setRandom(Math.floor(Math.random() * 10));
    };
    document.addEventListener("click", onclick);
    return function () {
      document.removeEventListener("click", onclick);
    };
  }, []);
riamahajan08 wrote this answer on 2022-09-19

Hey @subha84! I saw you error in the codesanbox if you remove the third option in the event listener method, it seems to work. The useEffect should look like this:

  useEffect(() => {
    let onclick = function (event) {
      setRandom(Math.floor(Math.random() * 10));
    };
    document.addEventListener("click", onclick);
    return function () {
      document.removeEventListener("click", onclick);
    };
  }, []);

@LuBustos What would be an alternative if the event needs to be processed in the capture phase?

ujjawalD07 wrote this answer on 2022-09-22

Is anyone working on this issue? I am new to open source contribution and I want to give this a go.

gaearon wrote this answer on 2022-09-22

I don't see any button in the reproducing case. How do I follow these instructions?

gaearon wrote this answer on 2022-09-22

OK, so the button in the description seems irrelevant. The issue I'm seeing is that commenting out setRandom call makes the checkbox work. So the setRandom call somehow breaks the checkbox.

I think this looks like a bug, though I'm not sure if it's fixable. I assume what happens here is:

  • Suppose controlled checked is false in the beginning.
  • When you click, the document capture level event fires.
  • The document capture level event handler updates the random state, re-rendering the component with checked={false}. This updates the controlled checkbox to have node.checked = false, even though it was true just before.
  • By the time React handles the checkbox change event, node.checked is false. So setChecked(false) keeps it unchecked.

Intuitively I'd expect that the issue is with the bold step — I think a re-render shouldn't have forced the controlled value to change if we haven't had a chance to run React handlers yet. But I'm not sure if this is something we can fix. Someone would need to dig deeper into this.

Is anyone working on this issue? I am new to open source contribution and I want to give this a go.

You're always welcome to try — note this might be a difficult one since you need to figure out how controlled inputs are implemented, and which assumption is being broken here.

subha84 wrote this answer on 2022-09-24

OK, so the button in the description seems irrelevant. The issue I'm seeing is that commenting out setRandom call makes the checkbox work. So the setRandom call somehow breaks the checkbox.

I think this looks like a bug, though I'm not sure if it's fixable. I assume what happens here is:

  • Suppose controlled checked is false in the beginning.
  • When you click, the document capture level event fires.
  • The document capture level event handler updates the random state, re-rendering the component with checked={false}. This updates the controlled checkbox to have node.checked = false, even though it was true just before.
  • By the time React handles the checkbox change event, node.checked is false. So setChecked(false) keeps it unchecked.

Intuitively I'd expect that the issue is with the bold step — I think a re-render shouldn't have forced the controlled value to change if we haven't had a chance to run React handlers yet. But I'm not sure if this is something we can fix. Someone would need to dig deeper into this.

Is anyone working on this issue? I am new to open source contribution and I want to give this a go.

You're always welcome to try — note this might be a difficult one since you need to figure out how controlled inputs are implemented, and which assumption is being broken here.

@gaearon exactly, setRandom is just simulate the use case to re-render the checkbox before the 'onChange' handler gets a chance to execute. We debugged into React code and we found the same reason you stated above, If we stop the re-render thinks work. We still haven't figured out the way to fix it considering the way the whole event system is designed. @riamahajan08

smchinna wrote this answer on 2022-09-27

@gaearon and @subha84
I have a few thoughts on this

  • Add Event listener function executed on capturing phase (document → html → body → parent → child) because of the third params passed as true on the event listener function - it won't wait to execute state updates.
  • It can resolve if executed in the bubbling phase. like
    document.addEventListener("click", onclick, false);
More Details About Repo
Owner Name facebook
Repo Name react
Full Name facebook/react
Language JavaScript
Created Date 2013-05-24
Updated Date 2022-10-06
Star Count 195649
Watcher Count 6648
Fork Count 40524
Issue Count 1110

YOU MAY BE INTERESTED

Issue Title Created Date Updated Date