svelte-forms-lib icon indicating copy to clipboard operation
svelte-forms-lib copied to clipboard

Custom validation - Returning `{}` does not lead to `IsValid` emitting `true`

Open MattBrittan opened this issue 4 years ago • 4 comments

Summary

The documentation for custom validation states: "If an empty object is returned the form will be valid and submit" but if the validation function returns {} then, after the first validation issue occurs, the IsValid store will always be false (despite the input being valid).

Note: I have assumed the documentation is the intended behaviour; this issue could also be resolved by updating the docs but the ability to return {} simplifies user code so is beneficial.

Steps to reproduce

  • Open the below example project
  • Delete the "v" from the text box (Valid will be false)
  • Insert some text (Valid will remain false)

Example Project

Example project link (based on the example in the docs).

What is the current bug behavior?

IsValid remains false after issue corrected.

What is the expected correct behavior?

IsValid should be true when validation function returns {}.

Relevant logs and/or screenshots

Possible fixes

This issue occurs due to the use of isNullish here:

util.update(errors, field, !util.isNullish(errs) ? errs[field] : ''),

As {} is not Nullish errs[field] gets set to {} rather than '' which leads to the relevant error becoming undefined rather than an empty string.

A potential fix would be:

util.update(errors, field, util.isNullish(errs) || Object.keys(errs).length === 0 ? '' : errs[field]),

or maybe duplicating the approach taken here:

util.update(errors, field, util.isNullish(errs) || util.getValues(errs).length === 0 ? '' : errs[field]),

(or you may decide that isNullish({}) should return true).

Alternatively the isValid store could be updated to treat undefined as meaning no error (but the above seems a better option).

MattBrittan avatar Aug 26 '21 00:08 MattBrittan

I suspect that PR #131 will also resolve this.

MattBrittan avatar Aug 27 '21 03:08 MattBrittan

@MattBrittan thanks for your report and repro! I merged #134 to address #131; please check if your error is resolved with 1.10.6

I'll have to evaluate the docs, but it seems like using {} as your error object doesn't appear to operate in user-friendly manner

Let me know if the #131 helped at all

larrybotha avatar Aug 29 '21 15:08 larrybotha

Thanks very much @larrybotha. Unfortunately this does not appear to have resolved the issue (I have updated the codesandbox.io test and it still fails).

I am avoiding this in my code with the following at the end of validate:

if (Object.keys(errs).length === 0) {
                    return null;
}

and, with this, everything works. The main reason I raised the issue is that the docs state "If an empty object is returned the form will be valid and submit.". Being able to return an empty object is convenient because you can just process each field in turn and add errors to an object without needing any special processing for the no-errors case. As there is a fairly simple workaround I don't think that its essential to fix this (but if its not being fixed it would be nice if the docs were updated).

Sorry - I'd generally have a go at fixing this myself and raise a PR but will not have time for a couple of weeks.

MattBrittan avatar Aug 29 '21 21:08 MattBrittan

@MattBrittan thanks for the feedback! Ye, not great that the docs are not in line with the experience. The docs are actually in a separate repo that I don't yet have access to, and would ideally be managed in this repo

larrybotha avatar Sep 12 '21 14:09 larrybotha