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

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

Open
OwsOwies opened this issue Mar 2, 2023 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@OwsOwies
Copy link

OwsOwies commented Mar 2, 2023

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

  • OS: Linux
  • Browser: chrome
  • Version: felte@1.2.6, @felte/reporter-svelte@1.1.3, @felte/validator-yup@1.0.10

To reproduce

No response

Small reproduction example

No response

Screenshots

No response

Additional context

No response

@OwsOwies OwsOwies added the bug Something isn't working label Mar 2, 2023
@pablo-abc
Copy link
Owner

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!

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

No branches or pull requests

2 participants