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

Fields don't clean up their rules properly #4643

Closed
2 of 5 tasks
bgoscinski opened this issue Jan 23, 2024 · 2 comments · Fixed by #4649
Closed
2 of 5 tasks

Fields don't clean up their rules properly #4643

bgoscinski opened this issue Jan 23, 2024 · 2 comments · Fixed by #4649
Labels
🐛 bug Unintended behavior

Comments

@bgoscinski
Copy link
Contributor

bgoscinski commented Jan 23, 2024

What happened?

I created a field using the composition API (useField), and I have a template that conditionally renders a different version of it (a <Field> with the same path, but different validation rules). When I change the condition in such a way that causes the <Field> to be unmounted then vee-validate still respects the <Field>'s rules.

I dug a bit into the vee-validate internals and found out that useField registers onBeforeUnmount that should cleanup the relevant pathState from the form but the removePathState finds and removes a pathState with different id than the one created by <Field>. I think that this line

const idx = pathStates.value.findIndex(s => s.path === path);

should have been written more like this:

const idx = pathStates.value.findIndex(s => {
  return (
    s.path === path &&
    (Array.isArray(s.id) ? s.id.includes(id) : s.id === id)
  )
});

I can prepare a PR with this change if you think this is the right way to fix it

Reproduction steps

  1. Visit https://stackblitz.com/edit/vee-validate-v4-checkboxes-t1tqgz?file=src%2FApp.vue
  2. Notice that form status is "Invalid". This is expected because a <Field> which specifies rules=required is rendered.
  3. Fill in the field, notice that Form status goes to "Valid"
  4. Clear the field, notice that Form status goes back to "Invalid"
  5. Now click the "Toggle field" button. This causes <Field> to be unmounted and should result in removal of the pathState that specifies the required rule.
  6. Notice that the Form status is still "Invalid" even though the only registered field has "always true" validator

Version

Vue.js 3.x and vee-validate 4.x

What browsers are you seeing the problem on?

  • Firefox
  • Chrome
  • Safari
  • Microsoft Edge

Relevant log output

No response

Demo link

https://stackblitz.com/edit/vee-validate-v4-checkboxes-t1tqgz?file=src%2FApp.vue

Code of Conduct

@bgoscinski
Copy link
Contributor Author

bgoscinski commented Jan 23, 2024

In the meantime I created a very hacky workaround:

import { Field, FormContextKey } from 'vee-validate';
import { getCurrentInstance, inject, onBeforeUnmount } from 'vue';

const { setup } = Field;
Field.setup = function wrappedSetup(props, ctx) {
  onBeforeUnmount(() => {
    const form = inject(FormContextKey, undefined);
    if (!form) return;

    const { id } = getCurrentInstance().exposed.meta;
    const states = form.getAllPathStates();
    const idx = states.findIndex(
      (state) => (Array.isArray(state.id) ? state.id.includes(id) : state.id === id),
    );

    if (idx >= 0) {
      // Move the correct pathState to the beginning of the states array
      // so that vee-validate's removePathState finds it first
      states.unshift(states.splice(idx, 1)[0]);
    }
  });

  return setup.call(this, props, ctx);
};

Stackblitz with the workaround applied: https://stackblitz.com/edit/vee-validate-v4-checkboxes-1zts45?file=src%2Fmain.ts

@logaretm
Copy link
Owner

logaretm commented Jan 28, 2024

Thanks for reporting this, I took a quick look and seems like it was caused by a couple of assumptions we had:

  1. Always one field with the path would exist at any given time.
  2. It is unlikely a field that would get registered before one of the same name is mounted.

feel free to PR it. we would need a test to accompany it if possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants