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

FieldArray removing property names if value is empty on blur (added after initialization) #3366

Closed
Olliebaba opened this issue Aug 29, 2017 · 16 comments

Comments

@Olliebaba
Copy link

Are you submitting a bug report or a feature request?

Bug Report

What is the current behavior?

When an object is pushed to a field array, it can have properties with values defined in advance. (In the example below, note 'bob' as a pre-defined firstName when adding a member. However, when a property's value is an empty string blurring the corresponding input field causes the the form values to drop the property name in question.

To reproduce:

  • open link below
  • open console
  • add a member
  • open fields array printed in console. has both firstName and lastName props in first object.
  • blur lastName without changing the value
  • open newly printed fields array in console, lastName property is missing.
    This appears to happen in @@redux-form/BLUR.

What is the expected behavior?

That the key/value persist in the object.

Sandbox Link

A reproduction of the issue.

What's your environment?

react v15.4.1
redux v3.6.0
redux-form v6.3.2

Other information

@iamdey
Copy link

iamdey commented Sep 5, 2017

This issue affect the architecture of my app too.

But it is not only related to FieldArray, it is due to how the blur action modify a value in the reducer (same in change reducer).

So If I try to manually add { foo: { bar: '' } } to form values and display the bar field only if foo exists, it will never work because bar then foo will be removed from form values.

To manually add a value we must use autofill action. (Obviously a clear action #3055 should be use to force remove an autofilled Field)

Finally I suppose the following things can fix the issue:

  • do not deleteInWithCleanUp if autofilled is true
  • apply autofill, for the arrayInsert, arrayPush, arrayUnshift, arraySplice actions

I'm pretty new to redux-form I don't know if it can bring side effects. Any idea contributors ?

@ms88privat
Copy link
Contributor

ms88privat commented Nov 17, 2017

Having the same problem. Maybe the deleteInWithCleanUp function should also compare to the initialValues before trying to do the cleanUp`. This means, when we explicitly set the initialValues with a given structure, it should not be deleted by default.

Edit: just realised that it tries to do this, but the the cleanUp also deletes the parent structure if empty. Will investigate a bit more and then create a PR...

Edit 2: related issue: #2597 (comment)

@erikras
Copy link
Member

erikras commented Nov 20, 2017

The equivalence of '' and undefined in forms is a constant source of headaches.

The "Never Filled Out" and "Left Empty" cases need to be equivalent when calculating pristine/dirty. Is there some reason that you absolutely need lastName to not vanish in the example? Might it be easier to have your app understand that there is a lastName field on your form whether or not the JSON contains one?

@iamdey
Copy link

iamdey commented Nov 20, 2017

easier ? it depends on the feature/application architecture.

(If I remember well, because I just disabled it)
I have a Select which let the user to add a field.
Since this field is in a sub property of formValues: formValues.info.birthday , the freshly added field just disappear if the user click outside (it's still usable but a UX nightmare).

So yes, In my case, I can duplicate the state of the form but with two source of trust it becomes no more maintainable.


@erikras thank you for making this great but complex library OSS. Keep the good work. I whish I can help more.

@bbenezech
Copy link
Contributor

bbenezech commented Nov 20, 2017

@erikras Can you start thinking of a v8 with a definitive solution ?

I would love

  • all empty values to be null : mandatory defaults, simply blowing up if key or value of field is missing
  • no false
  • no true (for booleans, use the string value of the ibput when checked, null if unchecked)
  • no undefined (too cumbersome with key not present being the same as value being undefined, and the React controlled/uncontrolled input switch)
  • no empty ""

Values would be either null, non empty string or string[] (for select multiple).

This would be a breaking change, but I can't state enough how huge this would be for the dirty tracking, the cleanliness of implementation and model.

There are so many issues today with falsy values I just sometimes want to rage-quit.

Plus the checkbox undefined => true <=> false is a design mistake : HTML only allows value present, value absent, plus ReduxForm does undefined <=> non-empty-value or "" <=> non-empty-value with strings, which is inconsistent with checkboxes.

@ms88privat
Copy link
Contributor

Yeah, I think its time for v8 ^^

We are also using this format function as default for our fields: const format = (value) => value == null ? null : value;

@Olliebaba
Copy link
Author

@erikras - Any status update on this issue?

@StoikovOleh
Copy link

same problem

@UlyssesInvictus
Copy link

UlyssesInvictus commented Apr 2, 2018

This is a problem for me as well, but I just resorted to managing the actual desired behavior through state rather than using the current values in the store. IMO, the only annoying part of the current implementation is that onBlur unexpectedly manipulates values, not necessarily that empty/blank values in the store are treated the same as unset ones.

It's not ideal to have manage the state at the same time as the store, but I can live with it.

@ms88privat
Copy link
Contributor

@UlyssesInvictus Looks like my PR got merged, it should work with a version where this one is included: #3619

@StoikovOleh
Copy link

StoikovOleh commented Apr 3, 2018

I just add to an input in the FieldArray: value={props.input.value || ' '} and after use trim().
I do it because I have field id in it, I know that it's not good style but it's work

@erikras
Copy link
Member

erikras commented Jun 12, 2018

Fix released in v7.4.0.

@erikras erikras closed this as completed Jun 12, 2018
@StoikovOleh
Copy link

StoikovOleh commented Jun 12, 2018 via email

@paynecodes
Copy link

paynecodes commented Jun 12, 2018

I was seeing this issue as well, so I installed v7.4.0, but something appears to be very wrong.. Forms seem to be lazily added to the store (after focus for example) leading to very bad problems for my app. I will try to be more specific, but, for now, this is all I've gathered.

@mcupito
Copy link

mcupito commented May 21, 2019

I upgraded to 7.4.0 but am still experiencing this exact behavior. Is there a flag that needs to be set somewhere or something?

@lock
Copy link

lock bot commented May 20, 2020

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 May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants