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

IsValid doesn't work when using array of objects? #115

Closed
Zerowalker opened this issue Apr 27, 2021 · 10 comments · Fixed by #116 or #117
Closed

IsValid doesn't work when using array of objects? #115

Zerowalker opened this issue Apr 27, 2021 · 10 comments · Fixed by #116 or #117
Labels
bug Something isn't working released

Comments

@Zerowalker
Copy link

I am not sure if this is a bug, or if it's supposed to work like this, so before i give an example i want to make sure it's not on my part.

Basically if i have an array of objects, like the FormArray example, it doesn't seem to be any kind of error,
but the "isValid" will still be false.

As the FormArray example doesn't use it i can't tell if this is expected behavior or not.

@Zerowalker Zerowalker added the bug Something isn't working label Apr 27, 2021
@larrybotha
Copy link
Collaborator

@Zerowalker that sounds like there may well be an oversight during initialisation. It's difficult to tell, however, as it may depend on some properties passed to the form. Please provide a minimal repro - you can use this CodeSandbox: https://codesandbox.io/s/svelte-forms-lib-template-zhqs4?file=/App.svelte

@Zerowalker
Copy link
Author

Zerowalker commented Apr 28, 2021

@larrybotha
Think this should do, i noticed that validateField doesn't seem to work either for the array,
but if you try to write something in the second input and them empty it you can see that it detects that it's invalid.
But even though both inputs are valid it still doesn't set "isValid" to true.

EDIT: forgot the link xd
https://codesandbox.io/s/goofy-meadow-5hf1e?file=/App.svelte

@DhyeyMoliya
Copy link
Contributor

DhyeyMoliya commented Apr 29, 2021

Here is another reproduction taken from the official example of array validation. I just added disabled={!$isValidation} to submit button. It never gets enabled, whatever we do.

Codesandbox : https://codesandbox.io/s/quiet-sound-qsetv?file=/App.svelte

Update :
I also added $: console.log($isValid); to log the value of isValid but it never changes to true.

@DhyeyMoliya
Copy link
Contributor

It is happening because of the following lines of code :

isValid store's value calculation :

const isValid = derived(errors, ($errors) => {
const noErrors = util
.getValues($errors)
.every((field) => field === NO_ERROR);
return noErrors;
});

util.getValue function :

function getValues(object) {
let result = [];
for (const [, value] of Object.entries(object)) {
result = [...result, typeof value === 'object' ? getValues(value) : value];
}
return result;
}

Explanation :

  1. isValid is a derived store made out of errors store.
  2. It is checking that every prop in the errors store should be "" blank string. Only then it returns true as isValid.
  3. Because the array props will not have "" as error value. They have [ { prop1: "", prop2: "" } ] or [ 0: "", 1: "" ] kind of values. Those are considered as errors. And isValid stays false.
  4. Although it uses util.getValues($error), the getValues function does not handle array type values.

Solution :

  1. getValues function needs to be updated to process array values.
  2. Some logic similar to getErrorsFromSchema function will work.
    function getErrorsFromSchema(initialValues, schema, errors = {}) {

I am will try to fix this problem and will create a pull request if I can.

Little Side-note: This library is the best out of all the other Svelte Forms Library.

@Zerowalker
Copy link
Author

@DhyeyMoliya
Oh that explains it, then the validation itself is correct, but the way to tell if it's actually valid is not so to speak.
Thanks for helping out, if i can do anything just say the word:)

@larrybotha
Copy link
Collaborator

larrybotha commented May 1, 2021

@Zerowalker excellent, thanks for the repro - adding tests was tricky until I realised that the issue presents itself on nested arrays

@DhyeyMoliya thanks for your investigation and your PR! You did all the hard work in finding the issue - much appreciated!

EDIT: @DhyeyMoliya also, thanks for your kind words about svelte-forms-lib, too!

@tjinauyeung
Copy link
Owner

🎉 This issue has been resolved in version 1.10.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Zerowalker
Copy link
Author

Zerowalker commented May 7, 2021

Hmm, am i missing something, or does is the array values always valid?

https://codesandbox.io/s/goofy-meadow-5hf1e?file=/App.svelte

EDIT:

Okay i was just stupid and forgot about the initial state of the validation.

@larrybotha
Copy link
Collaborator

@Zerowalker is there a UX issue that could be improved upon?

@Zerowalker
Copy link
Author

@larrybotha UX?
There's no issue, i was just stupid.
If anything i would like an inbuilt way to validate everything initially, but there's already requests.
The array validation works as expected, many thanks to everyone involved:)

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