Bug: Form reset lost checkbox onChange event

[imagine10255]

Hi, I use checkbox uncontrolled mode, onChange in form reset after, lose onChange.

<input type="checkbox" onChange={onChange} />

but use add ref.addEventListener('change', onChange) is ok

const checkRef = useRef<HTMLInputElement>();
useEffect(() => {
        if (checkboxRef) {
            checkboxRef.current.addEventListener('change', onChange);
        }
    }, []);

<input type="checkbox" ref="checkboxRef" onChange={onChange} />

React version: 16.13 and old

Steps To Reproduce

1. checkbox => checked
2. form reset
3. checked => checked

Link to code example:
not react is ok
reset is lose target onChange

The current behavior

1. checkbox => checked (target onChange)
2. form reset
3. checked => checked (lose target onChange)

The expected behavior

1. checkbox => checked (target onChange)
2. form reset
3. checked => checked (target onChange)

[Jacco]

Did some research:

I used even simpler code:

function onChange() {
  console.log('onChange');
}

function App() {
  return (
        <form>
          <input type="checkbox" onChange={onChange} />
          <button type="reset">Reset</button>
        </form>
  );
}

• It only happens when then checkbox is checked when you reset the form then check it again.
• It does NOT happen when the checkbox is unchecked when you reset the form then check it.

In the first case it happens when checking the second time because in updateValueIfChanged lastValue and nextValue are both true. So the function returns false and the onChange will be skipped.

The tracker on the node did not record the reset. I see that a reset event is fired for the target form but nothing seems to happen with that event. I do not see any code in react-dom that collect all inputs of a form to do something, so I am a bit reluctant to start adding something that big.

I would be interested in trying to make a fix for this.

[imagine10255]

@Jacco Hi, thank you for your feedback. Is there a plan to fix it? Let him keep the same with other inputs

[vinodsai-a]

Hi @bvaughn, Shall I work on this? (I'm new to opensource contribution)

[bvaughn]

If you'd like to, sure thing

[vinodsai-a]

Thanks @bvaughn . I'll try.

[imagine10255]

vinodsai-a <notifications@github.com> 於 2020年6月22日 週一 上午12:57寫道：

…

Thanks @bvaughn <https://github.com/bvaughn> . I'll try. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#19078 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABMBAWLX7VZBOFR2JIAGDHLRXY3YTANCNFSM4NTLCGGQ> .

[kassiomaia]

Did some research:

I used even simpler code:

function onChange() {
  console.log('onChange');
}

function App() {
  return (
        <form>
          <input type="checkbox" onChange={onChange} />
          <button type="reset">Reset</button>
        </form>
  );
}

• It only happens when then checkbox is checked when you reset the form then check it again.
• It does NOT happen when the checkbox is unchecked when you reset the form then check it.

In the first case it happens when checking the second time because in updateValueIfChanged lastValue and nextValue are both true. So the function returns false and the onChange will be skipped.

The tracker on the node did not record the reset. I see that a reset event is fired for the target form but nothing seems to happen with that event. I do not see any code in react-dom that collect all inputs of a form to do something, so I am a bit reluctant to start adding something that big.

I would be interested in trying to make a fix for this.

I wrote a pice of code but I'm not sure if it's on the right place, did this directly on react-dom.development.js.

@bvaughn, Is dispatchEvent the right place for this kind of actions?

function attemptToResetTrackers(nativeEvent) {
  var target = nativeEvent.target

  if (target && target.tagName === 'FORM') {
    for (var i = 0; i <= nativeEvent.target.length; i++) {
      var node = nativeEvent.target[i]

      if (node) {
        var tracker = getTracker(node)

        if (tracker) {
          tracker.setValue(node.defaultValue)
        }
      }

    }
  }
}

function dispatchEvent(topLevelType, eventSystemFlags, container, nativeEvent) {
  if (!_enabled) {
    return;
  }

  // Reset trackers when reset is called
  if (topLevelType === 'reset') {
    attemptToResetTrackers(nativeEvent)
  }

  if (hasQueuedDiscreteEvents() && isReplayableDiscreteEvent(topLevelType)) {
    // If we already have a queue of discrete events, and this is another discrete
    // event, then we can't dispatch it regardless of its target, since they
    // need to dispatch in order.
    queueDiscreteEvent(null, // Flags that we're not actually blocked on anything as far as we know.
    topLevelType, eventSystemFlags, container, nativeEvent);
    return;
  }

  var blockedOn = attemptToDispatchEvent(topLevelType, eventSystemFlags, container, nativeEvent);

  if (blockedOn === null) {
    // We successfully dispatched this event.
    clearIfContinuousEvent(topLevelType, nativeEvent);
    return;
  }

  if (isReplayableDiscreteEvent(topLevelType)) {
    // This this to be replayed later once the target is available.
    queueDiscreteEvent(blockedOn, topLevelType, eventSystemFlags, container, nativeEvent);
    return;
  }

  if (queueIfContinuousEvent(blockedOn, topLevelType, eventSystemFlags, container, nativeEvent)) {
    return;
  } // We need to clear only if we didn't queue because
  // queueing is accummulative.


  clearIfContinuousEvent(topLevelType, nativeEvent); // This is not replayable so we'll invoke it but without a target,
  // in case the event system needs to trace it.

  {
    dispatchEventForLegacyPluginEventSystem(topLevelType, eventSystemFlags, nativeEvent, null);
  }
} // Attempt dispatching an event. Returns a SuspenseInstance or Container if it's blocked.

[bvaughn]

I'm probably not the right person to review event stuff. Can you make the changes to the source files, then submit a PR?

[mvattuone]

There is a workaround that doesn't require a DOM event - basically, have a key for the checkbox that changes when onReset fires

Example below:
https://codesandbox.io/p/sandbox/crimson-monad-39s3fw?file=%2Fsrc%2FApp.js%3A1%2C1-25%2C1

[Andarist]

To be fair, I don't see the behavior being different between React and vanilla event handlers here. Even when using the provided repro. I'm testing this with Chrome Version 131.0.6778.205 (Official Build) (arm64)

[abhishekgautam95]

Bug: Form reset lost checkbox onChange event

Description:
Hello, I noticed that when a form is reset, the onChange event handler for checkboxes is lost. This issue occurs because the event handler is not properly reattached after the form reset.

Proposed Solution:
To ensure that the onChange event handler remains attached even after the form is reset, we can use the useRef hook to keep a reference to the checkbox and add an event listener for the change event in the useEffect hook. This approach ensures that the event handler is properly reattached after the form reset.

Code Example:

import React, { useRef, useEffect } from "react";

const CheckboxForm = ({ onChange }) => {
  const checkboxRef = useRef(null);

  useEffect(() => {
    const checkbox = checkboxRef.current;
    if (checkbox) {
      // Attach the event listener
      checkbox.addEventListener("change", onChange);

      // Cleanup function to remove the event listener
      return () => {
        checkbox.removeEventListener("change", onChange);
      };
    }
  }, [onChange]);

  const handleReset = () => {
    // Perform form reset logic here
    const form = checkboxRef.current.form;
    if (form) {
      form.reset();
    }
  };

  return (
    <form>
      <input type="checkbox" ref={checkboxRef} />
      <button type="button" onClick={handleReset}>Reset</button>
    </form>
  );
};

export default CheckboxForm;