redux-form / redux-form

A Higher Order Component using react-redux to keep form state in a Redux store
https://redux-form.com
MIT License
12.57k stars 1.63k forks source link

Form values not initialized from initialValues until 2nd render cycle #621

Closed erikthedeveloper closed 8 years ago

erikthedeveloper commented 8 years ago

We have been struggling w/ this for a while now and following some of the related issues. I'm not sure if our problem is directly related to these others or not (using release 4.1.6):

To summarize, we are finding that values are not being initialized until the second render cycle for our reduxForm-ed components. Example being that none of the following properties would be initialized until the first call of componentWillReceiveProps (i.e. the 2nd render):

Although, the following properties are initialized (even for the 1st render cycle) according to initialValues

My question would be, is what I've described above the expected behavior from redux-form? This causes issues for us where children component who depend on valid props being passed down to them receive undefined on the 1st render cycle and we can not rely on the "values" being reliably initialized.

We have resorted to something similar to

render() {
  if (typeof values.someField === 'undefined') {
    return null;
  }

  // ...
}

but then we can not rely on componentDidMount, etc...

I will make an effort to circle back and try and clarify this issue. I just want to get this out there for possible discussion. Any feedback/advice is much appreciated!


Here is some mostly-valid code below to illustrate the issue we have been experiencing

const SomeForm = React.createClass({

  componentWillMount() {
    const {values} = this.props;
    if (typeof values.someString === 'undefined') {
      // All values = undefined
      // props.fields.someString.defaultValue is set as expected
      // props.fields.someString.initialValue is set as expected
      debugger;
    } else {
      debugger;
    }
  },

  componentDidMount() {
    const {values} = this.props;
    if (typeof values.someString === 'undefined') {
      // All values = undefined
      // props.fields.someString.defaultValue is set as expected
      // props.fields.someString.initialValue is set as expected
      debugger;
    } else {
      debugger;
    }
  },

  componentWillReceiveProps(newProps) {
    const {values} = newProps;
    if (typeof values.someString === 'undefined') {
      debugger;
    } else {
      // Values initialized on 1st call (2nd render)
      debugger;
    }
  },

  render() {
    const {
      fields: {someString, someArray, someObject},
    } = this.props;

    return (
      <div>
        <ComponentUsesSomeString
          value={someString.value}
          onChange={someString.onChange}
        />
        <ComponentUsesSomeArray
          value={someArray.value}
          onChange={someArray.onChange}
        />
        <ComponentUsesSomeObject
          value={someObject.value}
          onChange={someObject.onChange}
        />
      </div>
    )
  }

});

const ReduxFormedSomeForm = reduxForm(
  {name: 'SomeForm', fields: ['someString', 'someArray', 'someObject']},
  (state) => {
    return {
      initialValues: {
        someString: 'Some String',
        someArray: ['some', 'array'],
        someObject: {a: 'some', b: 'object'},
      },
    };
  }
)(SomeForm);

const App = React.createClass({
  render() {
    return (
      <div>
        <ReduxFormedSomeForm />
      </div>
    )
  }
})
shilpan commented 8 years ago

485 and #514 do point at the same thing I think. It would be great to have some direction on this matter since it seriously hinders server side rendering.

erikras commented 8 years ago

Excellent description. :+1: I understood what you meant before I even got to the code.

Yes, this is a bizarre inconsistency with the library that should be remedied. The problem is that it is using the values from the redux state, which are not yet set until the the INITIALIZE action gets dispatched on componentWillMount(), so it doesn't get them until the second render. What needs to happen is that redux-form needs to be smart enough to go ahead and merge in any initialValues as though they were the values from the redux state on the first render.

I think I know how to fix this, but I can't get to it today.

andrewmclagan commented 8 years ago
Guibod commented 8 years ago

Is there any practical solution to this ?

I went for this in my form Component:

  componentDidMount() {
    // handle server side rendering, that bring a form that is NOT initialized
    if (! this.props.initialized) {
      this.props.dispatch(reset('project'));
    }
  }
sars commented 8 years ago

@erikras so... if I understand correctly, the problem is caused by following reason:

  1. we define fields here: https://github.com/erikras/redux-form/blob/master/src/createHigherOrderComponent.js#L36 without initial values.
  2. invoke initialize on componentWillMount https://github.com/erikras/redux-form/blob/master/src/createHigherOrderComponent.js#L44 here is initialize action https://github.com/erikras/redux-form/blob/master/src/actions.js#L19 that is handling by reducer: https://github.com/erikras/redux-form/blob/master/src/reducer.js#L71
  3. component renders with fields without initial values
  4. reducer changes form in redux state. Our component is connected to this state branch, so it receive new props and componentWillReceiveProps is invoking: https://github.com/erikras/redux-form/blob/master/src/createHigherOrderComponent.js#L48 The form was changed, so readFields is invoking: https://github.com/erikras/redux-form/blob/master/src/createHigherOrderComponent.js#L50 Now our fields have values
  5. render component with new fields with initial values
sars commented 8 years ago

As i see, we need move initialize from reducer to component, so we will have all fields and form with initial values in component from the very beginning. Then we can define shouldComponentUpdate that will prevent second render if there was nothing changed in fields or form

sars commented 8 years ago

guys, nobody cares? why?

erikras commented 8 years ago

I care, but I am currently focusing my efforts on the next version, which will take this and all the other unresolved bugs of v4 into account. It's maybe a month or two away from release. I have not taken the time to examine your analysis in depth, but it sounds like you have analyzed the problem well. If you would like to submit a PR that makes the test I wrote (linked above) pass and doesn't break any of the other tests, I'd be happy to accept it into v4.

pgilad commented 8 years ago

I am currently seeing this double render as well, which is also affecting the validate function (getting called the 1st time without any values from initialValues, even though they are available).

Anyway - I think @sars has properly identified the issue, now it's time to conceive a solution.

genyded commented 8 years ago

+1

2easy commented 8 years ago

+1

ankerek commented 8 years ago

+1

andrewmclagan commented 8 years ago

Also causes some tests to fail / be run twice.

erikthedeveloper commented 8 years ago

Just to let anybody know that stumbles across this, it looks like this issue was specifically solved in the 5.0.0 release

The value prop of your fields will never be undefined [...]

I have yet to fully confirm this (to update our usages to 5.x), but if that is the case this issue might be appropriately closed (unless a 4.x patch is really needed/in demand).

silvenon commented 8 years ago

In my case v5 only sets the value to "", not to initialValue like I expected.

erikras commented 8 years ago

Hey guys. Just wanted to let you know v6.0.0-alpha has been released!

erikthedeveloper commented 8 years ago

@silvenon it seems that the change from "uninitialized being `undefined" to "uninitialized being an empty string" might make things worse (for my use cases at least). If you see my examples above (issue description), that is how we are guarding against attempting to render w/ uninitialized values. Hmmm....

FYI I am diving into this now to address throughout our project(s) at work (we ended up having to guard against this on every usage of redux-form throughout :sweat_smile:...) I'll be sure to update if I land on any sensible workaround or come up with a possible patch to submit. I do have a few in mind that I'll start playing with.

It seems that I may be better off sticking w/ v4.x for now considering the "empty string" issue @silvenon described here

silvenon commented 8 years ago

@erikthedeveloper I downgraded as well.

erikthedeveloper commented 8 years ago

Hey all! I just submitted https://github.com/erikras/redux-form/pull/843 as a fix for this. As far as I understand things, it should pretty well cover this issue. Thanks @sars for laying out the flow of things, that helped me dive in a lot easier πŸ˜„

cc @silvenon @Guibod @shilpan

ignatevdev commented 8 years ago

@erikthedeveloper Waiting for a merge from @erikras

erikthedeveloper commented 8 years ago

I case anybody is looking for a temporary fix before https://github.com/erikras/redux-form/pull/843 or others are accepted/merged, this is loosely what I've landed on for the moment so that I can sweep through and remove all my temporary hacks/guards throughout my components. This at least opens up a reliable data flow so that values[field] and fields[field].value are reliably set (for use within that 1st pesky render cycle and components' lifecycle methods).

// SomeFormComponent.js
import { makeFormSafe } from '../../makeFormSafe';

export const MyForm = React.createClass({/* */});

// ...

export const MyFormConnected = reduxForm(
  /*...*/
)(makeFormSafe(MyForm));
// makeFormSafe.js
import { createElement } from 'react';
import _ from 'lodash';

const fieldToValue = ({value, initialValue}) => typeof value !== 'undefined'
  ? value
  : initialValue;

const makeFieldsSafe = (fields) => _.mapValues(fields,
  (field) => typeof field.value !== 'undefined'
    ? field
    : {...field, value: fieldToValue(field)}
);

const makeValuesSafe = (values, safeFields) => _.mapValues(values,
  (value, fieldName) => typeof value !== 'undefined'
    ? value
    : safeFields[fieldName].value
);

/**
 * "Safely" wrap up a FormComponent before passing it to reduxForm
 *  This basically ensures that field.value and values[field] are reliably available...
 *  Pending redux-form patch. See {@link https://github.com/erikras/redux-form/pull/843}
 * @param {Component} component
 * @return {Component}
 */
export function makeFormSafe(component) {
  const ReduxFormSafetyWrapper = ({fields, values, ...otherProps}) => {
    const safeFields = makeFieldsSafe(fields);
    return createElement(component, {
      ...otherProps,
      fields: safeFields,
      values: makeValuesSafe(values, safeFields),
    });
  };

  return ReduxFormSafetyWrapper;
}
ignatevdev commented 8 years ago

@erikras Can you please accept the merge? The issue is still relevant and needs an urgent fix

erikras commented 8 years ago

I apologize, @NSLS and @erikthedeveloper, I totally missed seeing that PR.

erikthedeveloper commented 8 years ago

Thanks a ton @erikras!

erikras commented 8 years ago

Published as v5.1.4. πŸ‘ Cheers, @sars and @erikthedeveloper!

erikras commented 8 years ago

@erikthedeveloper Can you please look at this test that I wrote to demonstrate the bug for this issue? It's still not passing with your PR. Either the test is wrong or the bug is not really fixed.

erikthedeveloper commented 8 years ago

@erikras I've been looking into this. I'm not quite familiar with all of the inner workings here. If I follow this all correctly, it seems that the point of entry for the problem is somewhere underneath ReduxForm.constructor assigning this.fields via readFields(/* ... */), likely within or under readField about in this hunk

Either that or initializeState.

Also, I have been unable to locate where field.defaultValue is being set... only where it is being tested for.

image

I'm still digging in so I'll be sure to post what I come up with.

erikthedeveloper commented 8 years ago

@erikras After digging around for what amounted to my full morning, I have yet to come up with a sensible solution for this. I'm going to have to call it quits for the moment... I'll see if I can circle back sometime this week.

The key components I found myself digging around in were

It may be related to the change in behavior from 4.x to 5.x described [https://github.com/erikras/redux-form/issues/621#issuecomment-213152226] there seems to be some conflict/duplication surrounding where we "fallback/default" certain properties.

erikras commented 8 years ago

@erikthedeveloper Okay, no worries. No doubt the library is better with your PR than without it, but I don't think the dragon is quite dead yet.

yesmeck commented 8 years ago

After a few hours digging. I think the key point is here and here. In readField.js#L119 field.value is assigned by the initial value that is what we expected, but in updateField.js#L9, because at very first, form state is not created yet, so formFieldValue will always be '' and override the initial value at updateField.js#L13.

cheesemacfly commented 8 years ago

I have an issue with this and nested fields. If I try to 'clear' the form by changing the state of my store, not all fields are set to the right value. Let's say I have the following object in the store:

state.myObject = {
  "name": "test",
  "properties": [
    {
      "name": "first"
    },
    {
      "name": "second"
    }
}

and the following form initialization:

MyForm = reduxForm({
  form: 'myForm',
  fields: [
    'name', 
    'fields[].name'
  ]
},
state => ({
  initialValues: state.myObject
}))(MyForm);

Now if somewhere I dispatch an action that reinitialize state.Object to {} or undefined, the form state will be in a weird state where fields contains 2 empty object. I would expect it to fallback to the initial state instead (where fields is empty).

UPDATE

I now use

import { destroy } from 'redux-form';
...
dispatch(destroy('myForm'));

inside my action that reinitialize state.Object and it seems to do the trick. Not quite sure if this is the way to go because now my reducer has a dependency on the redux-form's one and needs to know the name of the form.

erikras commented 8 years ago

Fix published as v5.2.5.

rpominov commented 8 years ago

I'm still getting uninitialized values in validate hook with v5.2.5

erikras commented 8 years ago

@yesmeck ?

yesmeck commented 8 years ago

😒 Forgot the validate here, conceiving a solution.

benwiley4000 commented 8 years ago

Very confused - @erikras @yesmeck I am using v5.2.5 but I seem to have the problem described in the initial post. Perhaps @rpominov has the same. Is a solution coming? Including checks for undefined in validation where I shouldn't need them is getting tedious.

sascha1337 commented 8 years ago

had this problem with a version 3.X.X from react-redux-hot-example boilerplate, just updated to latest version and errors are GONE.

thx ;-)

mpolichette commented 8 years ago

I seem to be experiencing this also. Digging around it seems like the overwrite from initial values is occurring at this line: https://github.com/erikras/redux-form/blob/master/src/updateField.js#L10

I'm not sure what the value of formField is supposed to be when updateField is called... but it appears to be an empty object which causes my value to always default to "".

I may have the opportunity to dive into this at a later point.

Update Apparently the form has not been initialized yet. Maybe the right approach is to not call updateField if the the form has not been initialized yet. Thoughts?

Final Update I apologize, this was fixed in the v5.2.5. Not sure how I missed that :/ Thanks you guys!

joefiorini commented 8 years ago

@erikras I still see this problem in v5.2.5 when using this.props.values. I saw in the debugger in the redux-form HOC that this.fields._meta.values has keys for the fields, but the values are all undefined in the first render, even though initialValues are correct. Is this expected behavior?

mericsson commented 8 years ago

Seeing an issue with v5.2.5 as well. validate is being called with undefined/null when the form mounts. When I handle that with an if statement the form mounts successfully, but any future calls of validate are not causing the form to re-render. Strangely, everything works successfully when using react-hot loader locally. It is just in the production deployment that I am having any issues. Sorry if that is vague/confusing.

Update Found a workaround. Instead of relying on values passed in to my validate function, I am using props which is passed in as a second parameter. props is has the correct values passed through from src/readFields.js.

goodbomb commented 8 years ago

still seeing this in v6.0.0.rc3

duketwo commented 8 years ago

yes this problem also exists in v6.0.0.rc3

dcifuen commented 8 years ago

In v6.0.0.rc4 there is the boolean enableReinitialize form config property, that worked for me

andrewmclagan commented 8 years ago

Correct take a look at enableReinitialize

Also, be sure to not dispaly your form before you actually have initial values...

e.g.

// ...

    render() {

        const { post } = this.props;

       return (
            <div className="redux-form">
                {post && <UpdatePostForm initialValues={post} />}
            </div>
       );
    }

// ...
slavab89 commented 8 years ago

Does anyone still see this issue? Or is it only me and i'm missing something?

I have a form and using enableReinitialize with initialValues and a validation of required fields. Once the INITIALIZE is dispatched, the validate function gets a values with an empty object. After all the fields are registered, a second call is made to the validate function (not sure how and why..?) but it does not update the form state so there are still sync errors present. The issue that i see is after all this initialisation, if i go to the form, click on a field and then on something else, the error is shown. Even though the value there is actually valid

thj-dk commented 8 years ago

I'm having an issue as well which is caused by this, I believe.

In my form, i define initialValues:

FindOrdersForm = reduxForm({
  form: FIND_ORDER_FORM,
  initialValues: {
    startDate: moment().subtract(1, 'months'),
    endDate: moment()
  }
})(FindOrdersForm);

In a parent component that renders this form, I need to access these initial values in componentDidMount. I've tried doing it using getFormValues, but the values are undefined. So no initial values present.

Any ideas?

rvirani1 commented 7 years ago

I'm still having this issue in 6.0.5.

gridworkz commented 7 years ago

I'm having a similar issue with the second render cycle. I've got an outstanding SO question here outlining my issue: http://stackoverflow.com/questions/40565652/display-server-message-on-initial-redux-form-load I'm simply trying to initialise the form submit to subsequently display a react-toaster alert on the first cycle of loading a registration form. It works great if I click a second time on my submit button. (Using redux-form 6.1.1)

I'm testing for the existence of the registering user then I pop up a notification from the server if the user already exists. Chrome auto-fills the email/password fields in my form and when I click 'Register' (the first time), nothing happens until I click submit a second time. I've tried all sorts of combinations to try and get the props set properly on initial load from the server-side user check, but no luck yet.

thj-dk commented 7 years ago

@erikras any news on this?

codemanki commented 7 years ago

same problem here @erikras