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 length wrong when using remove() and getValues with shouldActive: false #105

Open
whytspace opened this issue Jul 5, 2023 · 9 comments · May be fixed by #274
Open

FieldArray length wrong when using remove() and getValues with shouldActive: false #105

whytspace opened this issue Jul 5, 2023 · 9 comments · May be fixed by #274
Assignees
Labels
bug Something isn't working

Comments

@whytspace
Copy link

whytspace commented Jul 5, 2023

I stubled upon a strange behaviour, which seems like a bug to me.

The form that I am using is quite big and contains a bunch of ArrayFields, of which not all are rendered at the same time.
Since I want to always get all the values, regardless of whether they are rendered or not, I am using shouldActive: false wherever possible, as per your recommendation.

Now, when I remove an entry from the FieldArray via remove, I was expecting that entry to be deleted from the form store.
But it seems that that is only the case when getting the values via with shouldActive: true. With shouldActive: false the number of items is wrong.

Reproduction:
https://stackblitz.com/edit/github-uzqzot-m1rwea?file=src%2FApp.tsx


Coming from react and Formik (with the desire to switch to react-hook-form) I really like your plugin and the philosophy behind it! Thanks for the great work! 👍

But there is one thing that regularly kills my productivity:
I cannot seem to find a way of how to reliably interact with the field store and the values inside. I don't know if it is related to concept of activeness of a field, but each, reset, remove (this issue) and insert (could't fully reproduce my problem there) already caused problems to me, even when trying to opt out of that behaviour.

@whytspace whytspace changed the title FieldArray length wrong when using reset() and getValues with shouldActive: false FieldArray length wrong when using remove() and getValues with shouldActive: false Jul 5, 2023
@fabian-hiller
Copy link
Owner

Thanks for creating the issue and your feedback. I am in the process of revising the technical implementation of the acktive state and the recognition of field arrays. I am aware of the problem through several issues and technically it can all be solved. However, I am currently writing my bachelor thesis and cannot say with certainty when I will get to the implementation.

@fabian-hiller fabian-hiller self-assigned this Jul 5, 2023
@fabian-hiller fabian-hiller added the bug Something isn't working label Jul 5, 2023
@fabian-hiller
Copy link
Owner

However, I have already fixed some of the problems. Please test the whole thing once in the latest version. The length should work according to my tests.

@whytspace
Copy link
Author

whytspace commented Jul 5, 2023

Thank you, I will give the newest version a try :)

@whytspace
Copy link
Author

Yes, the newest version fixes that issue!


One question though: When I remove an item from an array via remove the array is not returned when calling getValues(form, { shouldDirty: true }). Is there even such a thing as a dirty state on an ArrayField?
Or is that something that is supposed to be implemented in userland?

@fabian-hiller
Copy link
Owner

Yes, field arrays also have their own dirty state. However, this is probably not yet fully implemented for the shouldDirty option. The dirty state changes whenever the items of a field array are changed by remove, insert and so on. Would the expected behavior on your part be that only then the array is returned or also when a field in the array is dirty?

@whytspace
Copy link
Author

Would the expected behavior on your part be that only then the array is returned or also when a field in the array is dirty?

Well, I don't know; it seems very case specific. I'll try to explain my usecase:

I have a gallery FieldArray, each item having "file" and "caption" fields. The API requires me to send the whole gallery array whenever there is a change inside (either fields values or added/removed enrties)
But when there is no change, I do not want to send the gallery array at all (null).

Currently I use <Form shouldDirty ... /> and in the onSubmit handler I check if gallery is defined. If it is defined, I use getValues(form, "gallery") to fetch all fields inside. This approach however does not work when removing items from the array, since gallery will be undefined (= no changes).


I was thinking of using a custom diff function. I could remove shouldDirty from <Form>, receiving all in the function argument and compare values with the initial values, as deeply as I need to. Basically writing my own "isDirty" check.
But for that I need a way to access the initial values from the form. Can I just use form.internal…?

Or do you have any other ideas how to solve the problem?

@fabian-hiller
Copy link
Owner

Yes, you can use form.internal. I don't recommend it for most users, but if you know what you're doing, it's fine. I think that in the long run, I may be able to solve this problem. Basically, the library needs to check if a field array is dirty or at least one contained field and if so, return the field array. However, since I am currently writing my bachelor thesis, I cannot give a timeframe for when I will get to the technical implementation.

obedm503 added a commit to obedm503/modular-forms that referenced this issue Dec 22, 2024
obedm503 added a commit to obedm503/modular-forms that referenced this issue Dec 23, 2024
@obedm503 obedm503 linked a pull request Dec 23, 2024 that will close this issue
@obedm503
Copy link

It seems the issue happens because setValues sets arrays as both fields and fieldArrays. remove affects the fieldArray but getValues then retrieves the field version instead of the fieldArray version.

I've reproduced the issue in both solid and preact but the same line exists in all packages (solid, qwik, react, preact).

To reproduce the issue in the linked stackblitz:

First, click one of the "Remove item" buttons to interact with the form's initialValues then click submit. Everything works as expected and the onSubmit handler logs only 1 list field. You'll also notice that the console shows fieldArrayNames: ['list'], fieldNames: ['list.0.name', 'list.1.name'].

Now click "Set values" and again remove one of the items in the list. Now click submit and you'll see 2 list items instead of one. In fact, even if you change the value of the remaining input field and submit again, you'll see the value logged is not correct. Also, you'll notice that the console shows fieldArrayNames: ['list'], fieldNames: ['list.0.name', 'list.1.name', 'list'], fieldNames should not include list.

I've submitted #274 which seems to fix the issue based on local testing. This might also be related to #85.

@fabian-hiller
Copy link
Owner

Sorry for the late reply. I'll try to look at it next week, but I can't guarantee it with so many other things going on at the moment. 😐

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants