facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.61k stars 46.43k forks source link

Bug: manually created 'change' events via new Event() don't trigger React event handlers #19846

Closed jesseko closed 2 months ago

jesseko commented 4 years ago

Description:

Manually created events created via new Event and emitted from a hidden input work great for 'input' events, they bubble as expected and can be caught via onInput handlers, but using 'change' events this way doesn't work -- onChange handlers are never called.

The vanilla JS 'change' events do bubble normally and can be caught by parents with vanilla JS listeners ( using addEventListener), but the React onChange listeners don't register anything.

I've created a codepen to demonstrate a minimal case for this via console logging. See repro steps below and code comments for additional details.

React version: 16.13.1

Steps To Reproduce

  1. Open https://codepen.io/jesseko/pen/dyMqGKG
  2. Observe that logging shows a single entry, the change event being emitted from a child component after render. We expect a second log from a parent's onChange but it never comes.
  3. Change EVENT_TYPE to 'input'
  4. Observe that logging shows two entries, one for the event being emitted and a second for it being detected via an onInput handler in the parent component.
  5. optional: there's some commented out code at the bottom to test a vanilla JS listener. Change EVENT_TYPE back to 'change' and uncomment that code and you'll see that that listener does work
gaearon commented 4 years ago

React onChange events do not map to the native change listener. Rather, they are a combination of input and a few other events. Can you reframe the issue around your actual goal — what do you want to do and why?

jesseko commented 4 years ago

Sure yes

First just to clarify: We do want React's notion of onChange -- events for every character typed.

My expectation is just that a manually created change event would cause a react change event and that an onChange listener on some parent element should fire for those events just like they do for the change events that fire when you interact with native inputs.


I think it's worth being thorough here so my answer doesn't just raise more questions.

We have around 40 custom form input components -- many contain native inputs, some don't. We used to use onChange for everything, but ~4 years ago we started using onInput for text inputs and some others for a few reasons -- I think there was a change in React that made onChange less reliable for text inputs, and I think at the time autocomplete events fired input but not change. Also we've had this trouble firing custom change events. We use these manually created events for things like "Select all" "Deselect all" buttons on a CheckboxGroup component. More on that below.

In our setup we use a custom form component that catches bubbled events and fires actions for those events. We've been under the impression that for our text inputs and any controls that need manually created events, we need to listen to input events while for others we can continue to use change. We've coded our form to respond change events by default, but inputs can include a data attribute to signal to the form to respond to input events instead for that particular component (form looks for the data attribute on e.target on the bubbled event).

Note: I just did a test and I'm seeing both events firing for text inputs and for autocomplete events, as well as a few others. Our assumptions may be wrong or out of date... Maybe we could move towards standardizing to use input events everywhere at this point?

Custom values on events

In the example of our CheckboxGroup component, the we want the value to be an object indicating what's checked. Taking that object value in as a prop is easy. Emitting events with that value is a little harder. We have to catch the event just after it emits using a handler in the component, then attach a custom e.value property on it (and also call e.persist() until we get to React 17). Our form component looks for e.value first, then falls back to e.target.value if that's not present.
We can do this with

So I guess my questions are


My codepen above was the minimal repro. What I'm actually working on is making a nice encapsulated way of dealing with all the details required to emit events with complex values. I've got a draft component that uses useImperativeHandle to expose an emit method, so the API to consumers would be.

const emitterRef = useRef();

//in render
<SyntheticEventEmitter name="myInput" type="input" ref={emitterRef} />

// to fire an event with a custom value
emitterRef.current.emit(customValueObject);

Complete codepen demo.

It works for input but not change, which seems strange to me and I've struggled to find explanation of the differences.

gaearon commented 4 years ago

My expectation is just that a manually created change event would cause a react change event

This is incorrect, since React onChange is not triggered by native change events.

What I'm actually working on is making a nice encapsulated way of dealing with all the details required to emit events with complex values.

I haven't read in depth but my initial take is that you need to reconsider the approach and move away from emitting custom events. This is whack-a-mole where you're trying to guess implementation details that keep changing.

Can we refocus on the initial product problem? E.g. the autocomplete issues — can we get a minimal repro of that? And if they're not relevant, what's stopping you from simply always using React onChange and never emitting events yourself?

jesseko commented 4 years ago

This is incorrect, since React onChange is not triggered by native change events.

OK, that does make sense.

We can mostly do what we want with input events. We wanted the flexibility that if a given component is normally expected to produce change events that our custom events should conform to that standard, but sounds like we should abandon attempts to make custom change events. That's kind of the conclusion we came to earlier, but I always felt unsettled about it. I can see how it makes sense though, give that React's onChange is actually something different. Thanks for the clarity on that.

Fine to close this ticket then. Thanks!

I'll still answer this though:

what's stopping you from simply always using React onChange and never emitting events yourself?

Custom events have been useful to us for two reasons:

1) Some of our form components contain multiple inputs or controls, but we want to treat them as a single field in the store with a single value (though the value for some components may be an array or an object). We need to handle clicks and keyboard events, and when those cause a change to the value of the component, we'd like to fire change events to communicate that. Examples: --Clicking Select all on a checkbox group changes the value of what's checked from ['a'] to ['a', 'b', 'c']. we'd like a change event to fire to describe that. --If you interact with a custom Select or an autosuggest via keyboard, arrow down a few times and hit enter to select an option, we get a keydown event but we'd like to have a change event fire to report the new value.

2) Throttling: For components that include sliders: the slider events come very quickly. We generally stop all those, then use _.throttle to emit our own events at a slower pace. Picture a mortgage calculator with a down payment slider where the store needs to recalculate multiple fields and run validation when changes happen.

As a bit of context on (1): I'm at Redfin and we may have an unusually strong interest in encapsulating complexity in components vs. just writing more store code. We have a form builder tool where non-devs can build forms and the structure is stored in a DB, then sent to the client for display and we need to handle all forms build that way with some generic client store code that loads that structure, shows the form, and handles client validation and submit. We want all our components to work with that generic store code with no special cases for particular field types. But even when devs construct forms manually, we want to help them be efficient by giving them powerful components. An address component makes it easy to add 5 inputs to your form, set up validation on the whole thing, and save the resulting address structure.

jesseko commented 4 years ago

FWIW others struggle with this as well https://stackoverflow.com/questions/23892547/what-is-the-best-way-to-trigger-onchange-event-in-react-js The winning answer there just settles for input events.

and React issue https://github.com/facebook/react/issues/11488

Is there any way to manually create a React change event? We don't actually need the native change event at all. Just wish we could write custom stuff that integrates with the React event system to act more consistent to what native inputs do.

gaearon commented 4 years ago

I understand the desire for encapsulated components, the part I don't understand is why you want to fire native browser events instead of exposing your own component's events via props. <MyInput onChange={...} /> and then do props.onChange() from MyInput whenever you need. What is the problem with that approach?

jesseko commented 4 years ago

We do use onChange props as well, but we still find that event bubbling is useful for a couple reasons:

1) Most common usage is for instrumentation: you can listen to bubbled events from anywhere in the tree, which makes it easy to do things like track whether users interact with a certain section of the page (can fire an analytics event only for the first interaction in that section).

An advanced use of this: modify events intended for logging as they travel up the tree to gather context so you can log `(page, section, component, action)` without needing any knowledge of section from the input where an event originates.  i.e. the section component could catch the event and add a property `e.logging.section = "mySectionName"`.

2) Centralizing handling: Our custom Form component is the thing that knows how to fire actions to the store and route them to the right form there. Input components emit, then the form catches the bubbled events and fires actions using props the form has. Again, saves us from having to pass that info down.

If we were starting from scratch we could use Context to accomplish a lot of this, but I'm not convinced it's a better path.
-If you want to keep the ability to modify an event more locally before it reaches someplace higher up, or just generally want the ability to get callbacks further up the tree, you end up needing to implement something that looks a whole lot like bubbling, and in that case why not just use the bubbling that's built in. -If you don't need that, Context could solve it well - just use it to pass down some bound functions or all the raw info needed for an input component to directly fire analytics events or actions to stores.

In our case, switching implementations would be expensive and risky, so just trying to continue to make it cleaner and easier to use without a drastic change.

jesseko commented 4 years ago

Here's an illustration of that scenario of logging (page, section, component, action)

Say you're logging ("myPage", "articlesSection", "favoriteButton", "click")

Using event bubbling you can make a global setup where you handle sections like

<TrackingSection sectionName="articlesSection">…content…

and deep down in there, some component could do

onClick(e) {
  e.trackingComponent = 'favoriteButton';
  e.trackingAction = 'click';
}

then TrackingSection could have it’s own handlers where it does

onClick(e) {
  if (e.trackingComponent && e.trackingAction) {
    e.trackingSection = this.props.sectionName;
  }
}

and then we log the event when it hits the top of the page if the event has all the expected parts.

If the section needs to discern between events, only log some and not others, or attach extra info for some, that can all happen right there and everything further down the tree can be agnostic about where they're used - don't have to thread extra props through or read from Context or import some section-specific tracking util.

It's a really lightweight way of letting a button or input component say, "this happened, if you care" and letting a section say "I do care, thanks". And if you need to modify the tracking at more places in the tree that's easy too.

AprilArcus commented 4 years ago

I would also like to be able to do this. I am building an AngularJS-in-React bridge, and I'm mapping Angular's concept of two-way binding to React change events. This allows React to see each bridged Angular component as something akin to a <textarea /> and the calling React component can then capture its state and treat it as a controlled component.

I recognize that the very idea of this is unpleasant, but it is helping my team refactor our legacy AngularJS components into React from the top down. I'm also using an onChange prop to make this happen, and I don't strictly need event bubbling, but it would make the abstraction less leaky to have a way of emitting change events from React components.

stale[bot] commented 3 years ago

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!

segevofer commented 3 years ago

This is also useful for using native web components which can fire a change event when their value changes.

import { MyCustomThing } from "some-web-components-library"; // Native components, not react!

// doSomething() will not be triggered
<MyCustomThing onChange={() => this.doSomething()} />

It is surprising that React is not listening to change events inside an onChange handler.

This unfortunate implementation detail of react enforces developers to use aRef and manual code/bind to keep web components updated with react's life cycle.

krutoo commented 1 year ago

I understand the desire for encapsulated components, the part I don't understand is why you want to fire native browser events instead of exposing your own component's events via props. <MyInput onChange={...} /> and then do props.onChange() from MyInput whenever you need. What is the problem with that approach?

@gaearon Are there any instructions on how to properly create synthetic event based on change event of input element?

github-actions[bot] commented 5 months ago

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!

AprilArcus commented 5 months ago

bump

GCastilho commented 5 months ago

So, 4 years have passed. Is there any canonical way to trigger react's onChange handler?

github-actions[bot] commented 2 months ago

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!

github-actions[bot] commented 2 months ago

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!