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

do not touch array on FieldArray.changeValue #362

Closed
wants to merge 1 commit into from
Closed

do not touch array on FieldArray.changeValue #362

wants to merge 1 commit into from

Conversation

timc13
Copy link
Contributor

@timc13 timc13 commented Jan 22, 2018

Touching the array field changes the structure of touched to not match values anymore. I ran into this bug when adding new objects into my array.

Given values:

{
  items: [
    { foo: 'foo0', bar: 'bar0' },
    { foo: 'foo1', bar: 'bar1' },
  ]
}

I expect touched to look something like:

{
  items: [
    { foo: false, bar: false },
    { foo: false, bar: false },
  ]
}

Instead, by touching the array field on every FieldArray.changeValue, the current behavior turns touched into this:

{
  items: true
}

@timc13 timc13 changed the title do not touch array value on add do not touch array on FieldArray.changeValue Jan 22, 2018
@timc13
Copy link
Contributor Author

timc13 commented Jan 23, 2018

I see the problem with this PR. Once an array field has changed, validations are run immediately - and for that to be clear, we need to touch the fields.

I actually think running ArrayHelper actions should NOT trigger validations. What's the argument for validating upon changing the array items?

@jaredpalmer
Copy link
Owner

Imagine a form with validation where an array needs to be at least n items long.

@timc13
Copy link
Contributor Author

timc13 commented Jan 26, 2018

I see your point, but I think for consistency, we should validate & touch arrays deeply in the same way that we do for nested objects. I don't think mutating a nested objects sets their touched value to true, does it? - or maybe there is no way to add/remove keys to the object in a "form" way.

It seems that for MVP purposes, it's simply more usable to control the length/min/max of an array via access to the array helper methods. Currently, FieldArray is useless because anytime you touch an array item field, dlv crashes.

I do agree though, that we probably want a way to track/validate the array itself, as well as track/validate the items inside.

@timc13
Copy link
Contributor Author

timc13 commented Jan 29, 2018

done in b32ebfd

@timc13 timc13 closed this Jan 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants