3

We're using a higher-order component to provide validation in our forms and it is responsible for acting on all change events. The problem I'm having stems from not being able to add a onChange handler to the wrapped component itself so the HoC wraps it in a div. We have a flex-based grid-system that really does not appreciate that extra div.

The HoC basically looks like this:

function withValidation(TargetComponent, options = {}) {

    class ValidatingComponent extends Component {

        ...

        render() {
            return (
                <div
                    ref={(_target) => { this._target = _target; }}
                    onChange={this.onChange}
                >
                    <TargetComponent
                        {...this.props}
                        {...this.state}
                        onSubmit={this.onSubmit}
                    />
                </div>
            );
        }
    }

    return ValidatingComponent;
}

The first thing I tried was to simply remove the wrapper

render() {
    return (
        <TargetComponent
            {...this.props}
            {...this.state}
            ref={(_target) => { this._target = _target; }}
            onChange={this.onChange}          
            onSubmit={this.onSubmit}
        />
    )
}

But that just passes the onChange as a prop to the wrapped component and I kind of don't want to have to call it manually. I tried to attach a regular vanilla event-listener by doing

componentDidMount() {
    findDOMNode(this._target).addEventListener('change', this.onChange);
}

But that didn't work. The onChange was never triggered, I'm guessing it's not catching whatever synthetic event it is that React dispatches.

Is there any way I can convince a React component to listen for change events on itself? I really don't want to have to pass that callback all the way down the component tree untill I reach the relevant <input> and call it from there.

I realize this question seems similar to this one but they're dealing with custom DOM events. I'm not.

EDIT

In case I wasn't being clear about what I'm trying to do I made a fiddle as well, it's here. When <TargetComponent /> is wrapped by a div and the onChange handler is attached to that div the form works but when onChange is just passed as a prop to the TargetComponent itself it doesn't. I need some way to tell my HoC to actually listen to events and not just pass a function. I'm not wrapping input's here, I'm wrapping entire forms.

ivarni
  • 17,658
  • 17
  • 76
  • 92
  • are you asking how to attach an event handler to a component? whats wrong with doing `onClick` or `onChange` on the `input`? – Sagiv b.g May 25 '17 at 14:13
  • @Sag1v Yes, that's what I'm asking. The higher order component wrapper is used extensively throughout the project to wrap other form components, if I was to attach `onChange` directly to the `` I would need to go around the entire codebase doing `onChange={this.props.onChange}` on every `` in every form. I'd rather not have to do that and rely on event bubbling instead. – ivarni May 25 '17 at 16:06
  • ok and what's wrong with passing it through the HOC? (like you did in your example) – Sagiv b.g May 25 '17 at 21:23
  • 1
    @Sag1v I might not have been clear enough about what I'm trying to do here, https://jsfiddle.net/ivarni/69z2wepo/79558/ might illustrate it. You can see the working and the non-working version of the HoC (Wrapped vs UnWrapped). If I passed the `onChange` as a prop to my form I would have to manually pass it to both inputs for the form to work. That's what I'm trying to avoid while at the same time getting rid of that wrapping div. I basically need to tell the HoC to listen for events on its own root and not pass `onChange` as a prop. I hope that was more clear. I've not had my coffee yet :) – ivarni May 26 '17 at 04:44

2 Answers2

1

You can use bubbling, of course. Just set onInput or onChange listener on HTML element that your <TargetComponent /> use as a container. Check event target (using regular DOM API), if it passes your conditions (maybe it should have some class or something like that) call this.props.onChange() that you passed to <TargetComponent />. But don't forget to bind this to the callback if you do not use ES6 arrow finction as a callback:

<TargetComponent
    onChange={this.onChange.bind(this)}
/>
GProst
  • 9,229
  • 3
  • 25
  • 47
  • 1
    binding a function in the `render` function is bad practice. `bind` will create an instance of this `onChange` function and this will happen on each `render` call. this could lead to a performance hit. you should bind it in the class's `constructor`. – Sagiv b.g May 25 '17 at 21:21
  • That's exactly what I mentioned trying to do and that doesn't work. It will pass `onChange` as a prop but the component receiving it does not know what to do with it. See https://jsfiddle.net/ivarni/69z2wepo/79558/ for a running example of that exact approach not working in this particular usecase. – ivarni May 26 '17 at 04:41
  • Sorry, maybe I expressed myself unclearly. I meant you should set `onChange` handler on your `` **inner** container, which is a `form` element in your jsfiddle example. Yes, you will have to set it on each inner container, but at least not on each `input`. However, I don't think that setting handler on each input element will be a bad ReactJS practice. Rather vice versa, I think it will be totally OK. – GProst May 26 '17 at 06:39
  • Ah right. Yes, setting it on the top level inner container will work and save me from having to repeat it too many places but I'm really looking for a solution here where I don't have to change the wrapped component at all. I already have 3-4 workarounds for this problem that I can use but I'm looking for a solution to the problem and not the symptom so to speak. – ivarni May 26 '17 at 07:12
  • 1
    I am afraid this is a `jsx` limitation, you can't set event listener directly on custom (non-html) element – GProst May 26 '17 at 08:16
1

I think it is safe to attach the onChange event handler on the form it self, that way you don't have to wrap it. a modified version of your jsfiddle.

    // ...
render() {
        const {
            formData: {
                age,
                name,
            },
            onChange
        } = this.props;

        return (
            <form onSubmit={this.onSubmit} noValidate={true} onChange={onChange}>
                <div>
                    <label htmlFor="age">Age</label>
                    <input name="age" value={age} />
                </div>
                <div>
                    <label htmlFor="name">Name</label>
                    <input name="name" value={name} />
                </div>
            </form>
        );
    }
    // ...

This will work as you expect though it still throws the same warning you are getting now:

Warning: Failed form propType: You provided a value prop to a form field without an onChange handler. This will render a read-only field. If the field should be mutable use defaultValue. Otherwise, set either onChange or readOnly. Check the render method of MyForm.

Sagiv b.g
  • 30,379
  • 9
  • 68
  • 99
  • Thanks. While that would work it's not exactly what I was hoping for. I'm guessing what I'm trying to do here isn't possible with React (and it would kind of breach the abstraction as well) so I'll be going with a workaround where I simply don't wrap grid-elements. It's just convenient to not have to pass that handler down the daisy-chain. I'm aware of the warning and I have ways of dealing with it :) – ivarni May 27 '17 at 09:27