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.64k forks source link

Immutable FieldArrays broken with Immutable 4.0.0-rc.5+ #3489

Closed ssilve1989 closed 6 years ago

ssilve1989 commented 7 years ago

Are you submitting a bug report or a feature request?

Bug

What is the current behavior?

Using FieldArrays results in Uncaught TypeError: original.mergeDeepWith is not a function

What is the expected behavior?

Expected to not do that.

Sandbox Link

What's your environment?

redux-form@7.0.4 immutable@4.0.0-rc.7 node@7.10.1

Other information

The method mergeDeepWith has been removed from the List API since 4.0.0-rc.5 so now any interaction with FieldArrays causes this exception to show up.

erikras commented 7 years ago

Try v7.1.0? That's the first version with immutable@4.0.0-rc.5 used in the tests.

jo-flynn commented 7 years ago

I'm also experiencing this issue when calling input.onChange in a Field component within a FieldArray.

I'm running redux-form@7.1.0 & immutable@4.0.0-rc.7.

ssilve1989 commented 7 years ago

@erikras I still see the error TypeError: original.mergeDeepWith is not a function using FieldArrays with redux-form@v7.1.0

thessem commented 6 years ago

mergeDeepWith was removed here https://github.com/facebook/immutable-js/pull/1344

TikiTDO commented 6 years ago

I tried replicating the old mergeDeep behavior here: https://github.com/TikiTDO/redux-form/commit/fd7d616efa8149a26314d74bd4c0236598cd3434

It seems to work well enough for my use case, but I'm not entirely sure what problem is being solved here, so I can't speak to other scenarios.

TikiTDO commented 6 years ago

@erikras If you get a second could you comment on the above fix? This issue is hitting us pretty hard.

bogdaniy commented 6 years ago

All my form with FieldArrays is broken after updating immutable-js version to 4.0.0-rc.7, my redux-form version is 7.2.0

IKatsuba commented 6 years ago

Any news? We are very much looking forward to solving the problem

gustavohenke commented 6 years ago

No news, sorry.

IKatsuba commented 6 years ago

@gustavohenke do you plan to solve this issue?

TikiTDO commented 6 years ago

@Defenderbass I'm still waiting for someone to comment on my PR, but we've been using it for our project for a couple of weeks now with no issues.

IKatsuba commented 6 years ago

@TikiTDO I tried to use your option, but the fields began to disappear from the array. That is, the merge was not done correctly. I had to come up with my own version

TikiTDO commented 6 years ago

@Defenderbass I tried to reproduce what the functionality of the original mergeDeepWith, but didn't spend too much time on it. I can go fix mine, but if you believe yours works then I'm good with that too. If you have a branch I can try let me know and I'll see if it solves my problem too.

IKatsuba commented 6 years ago

@TikiTDO Look here #3692 . There everything is corny, but it works

TikiTDO commented 6 years ago

@Defenderbass Sounds good. I'm in an all day meeting, but I'll give it a try this evening.

erikras commented 6 years ago

Published fix in v7.2.1.

lock[bot] commented 5 years ago

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.