Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Form doesn't reinitialize when replaced #3435

Closed
eliseumds opened this issue Sep 21, 2017 · 38 comments
Closed

Form doesn't reinitialize when replaced #3435

eliseumds opened this issue Sep 21, 2017 · 38 comments
Labels

Comments

@eliseumds
Copy link
Contributor

eliseumds commented Sep 21, 2017

Are you submitting a bug report or a feature request?

Bug report. I'm seeing React lifecycle methods being called in a different order with React 16 which breaks the forms (leaving them destroyed).

Expected order

  • componentWillUnmount
  • componentWillMount
  • componentDidMount

Actual order

  • componentWillMount
  • componentWillUnmount
  • componentDidMount

It looks like it's an expected behaviour but it leads to issues because the forms don't reinitialize.

@kjanoudi
Copy link

kjanoudi commented Sep 28, 2017

This also is the case after a form submission using a pure component with initialValues. After the form is submitted, and the new data is passed to the wrapper component via props, the redux form will DESTROY after registering all the new fields. In react 15, it would DESTROY, INITIALIZE, then register all the fields. In react 16, it DESTROY after registering all fields, and there is no new INITIALIZE call.

React 15

image

React 16

image

@eliseumds
Copy link
Contributor Author

eliseumds commented Sep 28, 2017

@kjanoudi same thing here :( It's good to know that I'm not alone. This was such a massive change from React that breaks so much stuff and not only with redux-form:

@blord-fullscreen
Copy link

React 16 compatilbility is crucial. Would love to see this made a high priority.

@kjanoudi
Copy link

kjanoudi commented Oct 5, 2017

Shocked this isn't getting more attention. Guess I'll be waiting to upgrade to React 16

@erikras erikras added the bug label Oct 5, 2017
@erikras
Copy link
Member

erikras commented Oct 5, 2017

Is it obvious to anyone how to solve this? It isn't to me.

redux-form has never behaved very well under dev hot reloading in my experience. I usually turn destroyOnUnmount: false for the duration of the development of my form. Does that make any difference here?

@kjanoudi
Copy link

kjanoudi commented Oct 5, 2017

@erikras I'm less concerned with the behavior on hot reloading as opposed to in production. I'm experiencing the scenario I outlined above on a production build of my app. After saving the form and remaining on the same screen, previously the form would not clear. Presently, it clears.

@eliseumds eliseumds changed the title Form doesn't reinitialize on hot reload with React 16 Form doesn't reinitialize when replaced Oct 5, 2017
@eliseumds
Copy link
Contributor Author

@erikras sorry for the confusion. I edited the title and description. It happens every time the form instance gets replaced.

@eliseumds
Copy link
Contributor Author

And it's also not clear to me how to solve this. Maybe by initializing the form on didMount as well? If only React passed a parameter to componentWillMount telling us if it's bring replaced by another component (including its props).

@tyscorp
Copy link
Contributor

tyscorp commented Oct 10, 2017

Can we move the initialization logic from componentWillMount to componentDidMount? That would require rendering twice, though. (once again after the mount) I'm not sure what other side effects this would have.

@janpe
Copy link

janpe commented Oct 17, 2017

Any progress on this? This issue prevents me from upgrading to React 16 and forces me to still use Redux-form 6.6.3.

@bstro
Copy link

bstro commented Oct 30, 2017

Should there be an announcement on the README.md that lets people know this library doesn't quite work with React 16? I got really far down the Redux-Form rabbithole and now have to redo my form architecture, which is fine, but it might be good to save people from the same.

@janpe
Copy link

janpe commented Oct 31, 2017

Isn't this fixed in 7.1.2? Seems like the problem I was having doesn't occur anymore.

@bstro
Copy link

bstro commented Oct 31, 2017

@janpe I missed that release!!! It does solve this problem for me. Thanks for the heads up!

@gustavohenke
Copy link
Collaborator

Nice!! @eliseumds can you also confirm v7.1.2 fixes it for you?

@eliseumds
Copy link
Contributor Author

@gustavohenke unfortunately it doesn't :S On hot reload, forms still end up destroyed.

@stevenmusumeche
Copy link

This is not fixed in 7.1.2 and was a nasty surprise for me. DESTROY is being called after INITIALIZE of the new form.

@brucewpaul
Copy link
Contributor

is there any progress on addressing these issues @erikras ?

@brucewpaul
Copy link
Contributor

it seems like moving the logic into componentWillMount might work, but there are def perf implications of that. By doing that, II'm seeing:

initialize
register_field
destroy
initialize
register_field

React ensuring the order of events really screws with the logic in redux-form. Is there a way to only destroy the form if not being replaced?

@tyscorp
Copy link
Contributor

tyscorp commented Nov 14, 2017

Just a quick thought: I wonder if in the meantime until this is fixed, a user-land fix would be to use React's key prop to force (or not) mounting and unmounting? Does the same lifecycle order persist? I haven't tested anything, it's just an idea.

@brucewpaul
Copy link
Contributor

While this is still being figured out on the redux-form side, we have a solution that we think solves this. Basically we created a HOC that is meant to be a temporary replacement for reduxForm() that wraps it and calls this.props.destroy(this.props.form) on cWM. We do this in conjunction with setting destroyOnUnmount: false. This is not the most ideal, but seems to be working for us at the moment

@erikras
Copy link
Member

erikras commented Nov 27, 2017

Fix published in v7.2.0.

@erikras erikras closed this as completed Nov 27, 2017
@giedriusvickus
Copy link

7.2.0 did not fix this issue for me :/

@mickykebe
Copy link

Not fixed on 7.2.0
image

@serajam
Copy link

serajam commented Dec 14, 2017

same here

@VasiliKubarka
Copy link

VasiliKubarka commented Dec 15, 2017

The same problem with REGISTER_FIELD and UNREGISTER_FIELD for one form when mounting and unmounting components has Field with the same name
image

v7.2.0.
@erikras Are you going to fix this in the nearest future?

@AlexanderTserkovniy
Copy link

As well here with the same issue, please help at least with temporary workaround.

@rockneverdies55
Copy link

Any update on this? Why is the issue closed?

@kjanoudi
Copy link

kjanoudi commented Jan 9, 2018

Still broken for me with 7.2.0, i have the same behavior of it being destroyed as the last step after being initialized properly.

@bndnio
Copy link

bndnio commented Jan 10, 2018

I came across this funny behaviour a wee bit ago too using v7.2.0.
In particular, it arose when using a ternary expression in the return statement to conditionally render the top level element; replacing it with a conditional statement containing separate returns fixed it for me.

Original code causing bad behaviour:

class MyComponent extends Component {
  render() {
    return (
      {
        props.someBool === true
          ? (
            <MyReduxForm1 />
          ) : (
            <MyReduxForm2 />
          )
    )
  }
}

Modified code which no longer demonstrates the bad behaviour:

class MyComponent extends Component {
  render() {
    if (props.someBool === true)
      return (
        <MyReduxForm1 />
      )
    else
      return (
        <MyReduxForm2 />
      )
  }
}

@AlexanderTserkovniy
Copy link

Okay guys, I had opportunity to avoid this issue. That's kind of sucks, but it works and form is not destroyed in the end.

In my case it occured because of react-router and I used render method instead of component, that fixed the issue.

BAD Example (destroy in the end):

<Route
        path={`${props.match.url}/step-0`}
        component={compProps => <GetStartedScreen base={props.match.url} {...compProps} />}
/>

GOOD Example (NO destroy in the end):

<Route
        path={`${props.match.url}/step-0`}
        render={compProps => <GetStartedScreen base={props.match.url} {...compProps} />}
/>

So in my case this small change helped me to resolve the issue.

@kjanoudi
Copy link

Adding enableReinitialize made things work for me

@diegorodriguesvieira
Copy link

@kjanoudi but are the validations still working?

@kjanoudi
Copy link

I haven't had any issues with validations.

@tkvw
Copy link

tkvw commented Jan 22, 2018

I'm not very happy with the solution, here are my 2 cents:

The lifecycle events of react are to setup and cleanup component state. Currently redux-form binds redux state to the component using a form identifier only. This no longer works with React 16, because of the new lifecycle handling. So to bind the redux state more closely to the component rendered I think the way to go is to keep a local state identifier on the form component which is reduced in the redux form state. This identifier can be created with the initialize action creator and should be passed as an (optional) argument with the destroy action creator so the reducer can check if the identifier is stale/outdated.

As a workaround I'm using a redux middleware, which makes sure to drop the DESTROY action if the form's INITIALIZE was called already:

import { actionTypes } from 'redux-form';

const state = {};
export default () => next => action => {
    switch (action.type) {
        case actionTypes.DESTROY:
            state[action.meta.form] = (state[action.meta.form] || 0) - 1;
            if (state[action.meta.form] <= 0) {
                return next(action);
            } else {
                // Drop the action
                return false;
            }
        case actionTypes.INITIALIZE:
            state[action.meta.form] = (state[action.meta.form] || 0) + 1;
            return next(action);
        default:
            return next(action);
    }
};

@tapesec
Copy link

tapesec commented Mar 1, 2018

@tkvw Thanks so much dude, it works for me.

@ZizhangAi
Copy link

Other than @tkvw 's middleware, I also have to set enableReinitialize true

@eliseumds
Copy link
Contributor Author

@tkvw @tapesec @ZizhangAi keep in mind that you can't increment the counter on reinitilization (which is INITIALIZE itself). Ideally we'd have a proper flag indicating that the action is for reinitilization, but all I could find was lastInitialValues:

case actionTypes.INITIALIZE:
  // bypass reinitialization
  if ('lastInitialValues' in action.meta) {
    return next(action);
  }

  state[action.meta.form] = (state[action.meta.form] || 0) + 1;

  return next(action);

@lock
Copy link

lock bot commented Mar 20, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Mar 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests