Redux-form: v6 (alpha 9): Some input fields lose focus after typing

[jervi]

Fixing issue #936 solved a lot of problems I had with validation across multiple fields (for instance when one field is required based on the value of another field).
However, some of my custom input fields now lose focus after typing. This did not happen in alpha < 9.

        <Field name='lumpSum' component={lumpSum =>
            <InputBox id='lumpSum' label='Lump sum' type='number' white={true} size='small' {...lumpSum}>
                {lumpSum.touched && lumpSum.error && <ValidationTooltip>{lumpSum.error}</ValidationTooltip>}
            </InputBox>
        }/>

InputBox looks like this:

const InputBox = ({ id, label, help, value, size, white, children, ...rest }) =>
    <div className={ classNames('form-group', `input-${size}`)}>
        <label className={classNames({'hide' : !label})} htmlFor={id}>{label}</label>
        <input
            {...rest}
            className={classNames({'form-control' : !white, 'form-control-alternate' : white})}
            id={id}
            value={value}
            />
        {children}
        {help && <div className="help-text-under short-text">{help}</div>}
    </div>;

[erikras]

I have seen this. It happens on the sync validation example on v6.0.0-alpha.10, but not on v6.0.0-alpha.8. It must be related to #940 somehow. @leesiongchan?

[jervi]

Yeah, that is the same behavior. It only happens after I write the first character, and not on all fields.
Seems like it only happens when the state is populated for the first time or something. In the sync validation example, it happens on all fields, but _only_ if the two other fields are blank.

[erikras]

It's related to the valid and dirty flags getting flipped, I think. Very, very strange.

[leesiongchan]

Ok, I will look into it ASAP.

tgif

[fredrikolovsson]

For what it's worth, I encountered the same problem when having a higher order component that consumed the dirty prop from reduxForm() and caused a render when it flipped. The setup was like this:

export default reduxForm({
  form: 'formName'
})(higherOrderComponent(FormComponent));

My particular case was solved by returning false in shouldComponentUpdate() in the HOC. Thought I'd share just to show the problem _might_ be caused by something outside of redux-form.

[Iuriy-Budnikov]

+1

[mauvew]

+1, it happens to the MUI example in alpha 10

[leesiongchan]

It seems create your component beforehand will solve this issue, only lazy component will trigger this issue. Example:

const HardWorkingComponent = props => (
  <div>
    <input {...props} />
    {props.touched && props.error && <span>{props.error}</span>}
  </div>
);

...
<Field name="hardworkingComponent" component={HardWorkingComponent} />
<Field name="lazyComponent" component={username =>
  <div>
    <input type="text" {...username} placeholder="Username"/>
    {username.touched && username.error && <span>{username.error}</span>}
  </div>
} />

This will only happen when reduxForm re-rerenders. @erikras any idea why?

[erikras]

Ah yes, I think I came to that conclusion before, too. Which is why the Simple example doesn't exhibit this behavior.

Because the stateless function is being created in render(), its component is essentially re-constructed on every render, which is then replacing the DOM node, and the result is that the DOM node loses its state (focus). If you can mock up an example in JSBin or JSFiddle without redux or redux-form, and demonstrate that a rerender of the container kills the focus of an input inside a stateless function, we can take it to the React experts for help.

[fredrikolovsson]

tl;dr adding dirty, pristine, and anyTouched to propsToNotUpdateFor in reduxForm.js solves this bug (but might be problematic in other ways?)

In addition to the first-typing bug, I discovered that the field also loses focus when trying to tab from one field to the other, even after the first-typing bug has happened. This behavior is reproducible in (at least) the sync validation example and the submit validation example.

The previous discussion here indicated the bug(s) might have to do with props flipping values and my debug logs showed that the entire form re-renders when these bugs happen, so I went into shouldComponentUpdate() in reduxForm.js and enabled the changed-props-logging in the out-commented code and added a log for the value of !~propsToNotUpdateFor.indexOf(prop). This showed that changes in values of dirty, pristine and anyTouched will return true in shouldComponentUpdate().

To avoid re-rendering the entire form component when these props change, I added them to propsToNotUpdateFor in reduxForm.js like so:

const propsToNotUpdateFor = [
  ...Object.keys(importedActions),
  'anyTouched', // <-- added
  'array',
  'asyncErrors',
  'dirty', // <-- added
  'initialized',
  'initialValues',
  'pristine', // <-- added
  'syncErrors',
  'values'
]

This fixed both the first-typing and first-tabbing bugs.

Below are screenshots of the debug logs for both cases in my LoginForm with two fields, email and password. The scenario is that I, regardless of any bugs trying to stop me, want to:

1. Type "123" in the email field
2. Tab from the email to the password field.

Release 6.0.0-alpha.10

_Note: Only the first prop change that triggers a component update is logged._

With dirty, pristine and anyTouched added to propsToNotUpdateFor

_Note: No form render, but the fields re-render fine._

While the bugs are annoying, I am not sure if there are other features depending on changes to dirty, pristine and anyTouched actually triggering a component update, so it would be good to learn what the expected behavior is for redux-form?

[erikras]

That's some great investigation and reporting, @fredrikolovsson! However, those props really _do_ need to cause a form rerender. And it works fine on the Simple example, so I highly suspect that it's related to the stateless components, as I mentioned above. But your testing definitely confirmed that it was the changing of those props that is causing the issue.

[brotzky]

Upgrading to 6.0.0-alpha.11 doesn't resolve the losing focus bug for me (it seems you guys are aware). I am trying to figure it out on my end, but don't have a solid understanding of the codebase yet.

[erikras]

That is correct. alpha.11 does not fix this.

[erikras]

I've written the JSBin test that I suggested above is the next step.

@gaearon Can you offer some assistance? Look at this and tell me if there is any way to pass component as a stateless function and have it not destroy all the DOM state on every render.

[fredrikolovsson]

I was using stateless functional components for the testing above and in the naive prop-ignoring case when the form was not re-rendering, the SFC given to Field under the component prop was still re-rendering, without causing any bug. Thought it might indicate the problem lies in the Field itself and, perhaps this is obvious, but reverting the introduction of shallowCompare in shouldComponentUpdate also fixes the issues. (This 👇 in https://github.com/erikras/redux-form/pull/940)

[erikras]

Perhaps this is obvious, but reverting the introduction of shallowCompare in shouldComponentUpdate also fixes the issues.

My first instinct was that, "Yes, that's obvious", but what if we didn't rerender Field every time the component prop changes, because the component prop is going to change on every render. I might investigate this....

[erikras]

THAT'S IT! If we ignore changes to component, it is fixed.

Thanks to @fredrikolovsson for your "Perhaps this is obvious, but..." suggestions. That is how problems are solved.

[brotzky]

Updated to 6.0.0-alpha.13 and the focus issue is gone.
The last piece to my migration to v6 is complete 💯
Thanks everyone for the help.

[gaearon]

THAT'S IT! If we ignore changes to component, it is fixed.

This impacts correctness though. It means you no longer support changing component on the fly. Which is fine by itself, but it means something like

render() {
  return this.state.stuff ? <Field component='input' /> : <Field component={MyInput} />
}

won’t update at first, _and might later update suddenly_ when the field value changes, causing shouldComponentUpdate() to return true.

If you’re fine with it never updating, it’s better to read the component from the props in the constructor and always use this value for consistency.

Still, this somewhat contradicts the React contract that you can change props on the fly. For example, it can break in more subtle ways:

render() {
  let { number } = this.props
  return <Field component={(stuff) => <div>{number} {stuff}</div>} />
}

Do you see a bug here? number is destructured outside the closure, so component references a specific value. When number prop changes, the next value of component fat arrow would reference the new value. However, after the merged change, this will no longer happen: since component never updates, number always references the initial number value from the first render.

This is why it doesn’t look like a correct change to me. Maybe I don’t understand the API well enough, but passing fat arrow components to component seems wrong anyway. You’re generally not supposed to create components _inside_ render(). What’s wrong with defining them separately?

[erikras]

@gaearon Yes, this was already reported on #985.

The "fat arrow component" thing was meant to both provide convenience and also as a stepping stone for v5-to-v6 migration.

[gaearon]

I see.. Still, never updating is better than updating-sometimes-maybe-who-knows-when 😄 .
So it needs to be read from props in constructor.

[ooflorent]

I think Dan raised valid concerns. Blacklisting component from shouldComponentUpdate is a bad idea. Arrow functions will cause massive re-renders but it's OK since it is not the recommended way of specifying component.

However, @gaearon, if we don't ignore component there is this nasty React behavior shown by @erikras in this JSBin. It looks like a React bug. Any thoughts about it?

[ooflorent]

Well, after thinking about, it is not a bug but the expecting behavior. Since we generate a new arrow function, we generate a new component. When render is called, the field element's type is not same as in the previous render so React asserts that this is no longer the same instance.

[gaearon]

Well, after thinking about, it is not a bug but the expecting behavior. Since we generate a new arrow function, we generate a new component. When render is called, the field element's type is not same as in the previous render so React asserts that this is no longer the same instance.

Exactly, this was the point I was trying to convey 😄 :

You’re generally not supposed to create components inside render(). What’s wrong with defining them separately?

[erikras]

Our options are:

1. Forbid stateless function components passed to component.
2. Allow them, but document the hell out of the _NO CLOSURES!!!_ rule.

I think I prefer the latter. It's too bad there's no way to inspect a function to determine if it has bound closures. Seems like something an eslint rule could be made for, or even a babel plugin to pass any closures as props.

[ooflorent]

Option 2 is definitely the way to go. Providing an ESLint rule would be awesome!

[clayne11]

So are we going to remove component from shouldComponentUpdate? @gaearon brings up some good points. It makes more sense to define the components outside of the render and use those and allow for component to update in the expected React way rather than defining component inline and then hacking around it.

[ooflorent]

I think we should revert b1e258bdf11b04e633f0852b15ab6005c818a456. shallowEquals on the instance is right choice.

[clayne11]

The docs should also be updated to show that you shouldn't be defining stateless functional components inside the render.

[erikras]

@ooflorent, so @clayne11 and @gaearon turned you to Option 1?

I'm still on the fence. I'd like to see the Sync Validation Example rewritten to avoid "fat arrow components" (my new favorite React term). I almost wrote in the sync validation example description that any decent developer would realize that all the fat arrow components were identical and should be abstracted out to a const.

It annoys me that the code elegance of <Field name="myField" howToRenderItHereInline={...}/> might be a dead end. It would be so nice if it worked.

[clayne11]

Sort of Option 1. The issue isn't with using stateless functional components, it's with declaring the component inline inside the render. As long as the component is declared outside of render it isn't an issue.

I suggest simply updating the docs to show that you pass a React component into component rather than an inline stateless functional component and reverting b1e258bdf11b04e633f0852b15ab6005c818a456.

Example:

<Field {...{
    name: 'amountDollars',
    component: FormTextField,
}}/>

This is the idiomatic React way. Putting in special rules around this specific use case doesn't seem like the right solution to me.

[ooflorent]

What I want:

• component should accept everything that is renderable
• We should not block rendering when component changes

Basically I want to revert what I've been done. If someone wants to use an arrow component, fine, but it will re-render excessively and inputs will not behave correctly (but custom ones may work properly).

[erikras]

Okay, sounds reasonable. This one is _definitely_ going on the FAQ. 😄

[clayne11]

I agree, but I also think that all of the docs and examples need to be updated to show this as well.

Using stateless functional components in the docs is definitely going to confuse people even if you put it in the FAQ.

[erikras]

I agree, but I also think that all of the docs and examples need to be updated to show this as well.

Yes, I was assuming that. 👍

[sebinsua]

How would we write stuff with FieldArray if we're no longer allowed to use arrow functions? We presumably won't have access to push('members', {}) and other actions like this within our stateless function components if they're declared outside of render. Will you pass these into the props of the component we pass into component={...}?

[erikras]

How would we write stuff with FieldArray if we're no longer allowed to use arrow functions?

Sure, you will.

const renderMembers = array =>
  (
    <div>
      {array.map(name => <Field name={name} component="input"/>)}
      <button type="button" onClick={() => array.push()}>Add Member</button>
    </div>
  )

@reduxForm({ form: 'myForm' })
class MyForm extends Component {
  render() {
    const { handleSubmit } = this.props
    return (
      <form onSubmit={handleSubmit}>
        <FieldArray name="members" component={renderMembers}/>
      </form>
    )
  }
}

That should work. (untested)

[jervi]

A big thanks to all of you for the quick response and the interesting discussion. Refactoring away all the fat arrow components solved the problem, and resulted in cleaner code as well.

[erikras]

Okay, guys, I've reworked all the examples and released v6.0.0-alpha.14, detailing how form components must be written now.

[jfrolich]

Hard to debug :) But no arrow functions solved it for me as well.

[Aaronius]

The v6 migration guide still shows arrow functions inside render. Is this a mistake or just on the backlog?

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.