pablo-abc / felte

An extensible form library for Svelte, Solid and React
https://felte.dev
MIT License
1.01k stars 44 forks source link

Incorrectly merged error stores resulting in error not shown in form despite existing #225

Open OwsOwies opened 1 year ago

OwsOwies commented 1 year ago

Describe the bug

I have encountered a bug in errors merging algorithm.

In stores.js you create errors store as a derived store:

const [errors, startErrors, stopErrors] = derived([
  immediateErrors,
  debouncedErrors,
], mergeErrors, _cloneDeep(initialErrors));

function mergeErrors(errors) {
    const merged = _mergeWith(...errors, executeCustomizer);
    return merged;
}

If immediateErrors contains { someFormField: { key: 'some-error-key' } // or any other object } and debouncedErrors contains { someFormField: [] }, the result of merging is { someFormField: [] }. ImmediateErrors value is lost, and the form control shows no errors.

I think it is the issue with _mergeWith function lines 31-38 (tho I'm not sure since this function is quite complicated):

else if (Array.isArray(source[key])) {
  obj[key] = source[key].map((val, i) => {
    if (!_isPlainObject(val))
       return val;
     const newObj = Array.isArray(obj[key]) ? obj[key][i] : obj[key];
     return _mergeWith(newObj, val, customizer);
  });
 }

obj[key] is always derived from underlying source[key] array. Shouldn't there be a .push of some sort appending the original obj[key] value to the created array?

Which package/s are you using?

felte (Svelte), @felte/reporter-svelte, @felte/validator-yup

Environment

To reproduce

No response

Small reproduction example

No response

Screenshots

No response

Additional context

No response

pablo-abc commented 1 year ago

Took me a while to get back to this. I think I understand the issue but I would love to understand how you encountered this error (if you still remember)? I got a fix but I need to understand how would someone get into this in order to see if it's appropriate.

If you don't remember that's okay. I'll probably run some tests only end to make sure nothing breaks and merge if it looks alright!