FieldArray broken in v 8.2

[danielpowell4]

Are you submitting a bug report or a feature request?

bug report

What is the current behavior?

Pushing a new field into a field array is broken

1. Visit https://redux-form.com/8.2.0/examples/fieldarrays/
2. Click "Add Member"
3. See TypeError: Invalid attempt to spread non-iterable instance in console

What is the expected behavior?

A new member field should be pushed into the field array

Sandbox Link

https://codesandbox.io/s/nrzq9mzp34
https://nrzq9mzp34.codesandbox.io/

What's your environment?

Redux-form v8.2 (not 8.1)
At least Chrome 73, Safari 12.1, Firefox 65

Other information

Recent potentially related commit fc3b48d I say potentially as I see it was added in 8.2 and has the word FieldArray in it. Haven't wrapped my head around it all

[witbybit]

This is a major issue and I can confirm that it's not working 8.0.0 onwards. Must be something more fundamental due to v8 change.

[danielpowell4]

@nikhilag I have a simple codesandbox working on 8.1

https://codesandbox.io/s/30xx4588lm

Fairly certain was something with the commit I referenced above

[taylorqj]

Experiencing the same error on 8.1.

[SharudAgarwal]

Anyone have a workaround for now?

[tomasztomys]

+1

[rovnyart]

same issue here :(

[darkestblue91]

Today I have to validate if one FieldArray has elements, using this (as the documentation says):
if (!values.options || !values.options.length) { errors.options = { _error: 'You have to provide at least one option.' }; }

With that on the validator, I receive this error: Uncaught TypeError: Invalid attempt to spread non-iterable instance
I note that if I remove that condition on the validator, everything works well, so maybe that's a start to detect where is the bug or at least get a workaround.
For now, I have to go back to 8.1.0.

[kurtfurbush]

+1

Tried downgrading to 8.1 but still had problems.

[dmason30]

I assumed it was related to #4138 which was released in v8.0.0.

I have managed to get this working with 8.1.0 as well but for some reason I was still getting the error until I deleted my yarn.lock and node_modules then I was no longer getting the error.

Also, the example for 8.1.0 docs also errors when you click 'Add member':
https://redux-form.com/8.1.0/examples/fieldarrays/

[dmason30]

After some Investigation it definitely seems to be related to #4313/#4337 from @esetnik - I am going to fork and revert that change as a temporary solution.

I would recommend starting the conversion to react-final-form or another active form library since redux forms are no longer the recommended approach.

[renatoagds]

Hi all! Made the revert of the PR that caused this problem. Will release a new version in the next few hours. :)

[dmason30]

@renatoagds Great news thanks! I hadn't forked it quite yet. I would recommend that the new version includes a release for the docs so that the Field Array example works again!

[erikras]

Published fix in v8.2.1.

[esetnik]

The issue is that if you revert the PR then it will break redux-form ImmutableJS compatibility. I'm sorry for the trouble my PR #4337 caused everyone! It seems that some lower level functionality needs to be rewritten in the library to support both Immutable and non-Immutable usage. The issue with enabling loose mode is that now the immutable lists will be assumed to be arrays. This will break a number of functions when used with Immutable. There should be a warning for users of redux-form and ImmutableJS not to update to v8.2.1 or it will break their project. Also I suggest that v8.2.1 tag should be removed and the next release version should be v9.0.0 since this is a breaking change for ImmutableJS users. If ImmutableJS support is officially being dropped then that should be noted that v8.2.0 is the last version with support.

[esetnik]

The real fix here is that we shouldn't be using rest parameters on functions that can receive Immutable data structures

[Asday]

From what you're saying, @esetnik, it sounds like the best possible contribution you could make, would be a failing test with expected immutable data, and a passing test with expected non-immutable data, such that the maintainers (or anyone with some time and motivation) can provably make a general case.

Not only would this allow for fixing the re-broken immutable functionality, it would have prevented the immutable functionality initially breaking everyone else's toys.

[esetnik]

@Asday I opened #4470 for your request.

[renatoagds]

@esetnik Agreed with you that we should warn about Immutable problem, I changed the release description to include that. Btw, I would ask your help to list the current bugs that we have in Immutable part of ReduxForm. Do you think that we have more problems in Immutable parts? If yes, could you please open a issue to list them? So I could help to get them fixed.

[esetnik]

To my knowledge there are no other issues with ImmutableJS in the project. I don't have too much time at the moment, but we should do an investigation of everywhere the spread operator is used on "Array" values which may not actually be arrays (they might be ImmutableJS List type). The specific issue I fixed is related to how fromJSchanges when the project is using the immutable imports.

https://github.com/erikras/redux-form/blob/f20e4bed7e4185d7d18593732a4b5d75529f3452/src/structure/immutable/index.js#L31
https://github.com/erikras/redux-form/blob/e51957b115e545e7b92680be38c98322264d1ad0/src/structure/plain/index.js#L19

Which is used to wrap the return value of getFieldList():

https://github.com/erikras/redux-form/blob/fc3b48d0557c4bd0dac27cc5f6c22d984fdf1860/src/createReduxForm.js#L649

This means that when using immutable imports the list of fields passed to handleSubmit are ImmutableJS List type and not plain javascript arrays:
https://github.com/erikras/redux-form/blob/fc3b48d0557c4bd0dac27cc5f6c22d984fdf1860/src/createReduxForm.js#L793
https://github.com/erikras/redux-form/blob/fc3b48d0557c4bd0dac27cc5f6c22d984fdf1860/src/createReduxForm.js#L815

#4471 fixes this particular issue by detecting if handleSubmit received an ImmutableJS List and converting it to a plain array. This is the only instance I could find where the library itself calls one of the lower-level action handlers by attempting to spread an immutable list. It doesn't fix the case where a user attempts to call one of the handlers directly and passes an immutable list. The user would be responsible for converting to array before calling.

Technically any ...fields in https://github.com/erikras/redux-form/blob/master/src/actions.types.js.flow could cause ImmutableJS bugs if a List is passed and not handled. It is my opinion that to properly fix this, the public API should be changed in v9 so that these functions do not use rest parameters and rather take arrays directly. This would be a breaking change.

[lock]

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]

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.