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

[v4] Vue rc.5 unwrapping changes #2873

Closed
cexbrayat opened this issue Aug 26, 2020 · 7 comments
Closed

[v4] Vue rc.5 unwrapping changes #2873

cexbrayat opened this issue Aug 26, 2020 · 7 comments
Labels
🐛 bug Unintended behavior

Comments

@cexbrayat
Copy link
Contributor

Versions

  • vee-validate: 4.0.0-alpha.8
  • vue: 3.0.0-rc.8

Describe the bug

Vue 3.0.0-rc.5 introduced a breaking change on how unwrapping works vuejs/core#1682
and that breaks a lot of use cases of VeeValidate.

To reproduce

This can be easily reproduced by bumping Vue in VeeValidate repo: a lot of tests start failing.

As a simple example in our application, this used to work:

<template>
  <label for="name">Name</label>
  <input id="name" name="name" v-model="name.value" />
  <div class="error">{{ name.errorMessage }}</div>
</template>

<script lang="ts">
import { defineComponent, reactive } from 'vue';
import { useField } from 'vee-validate';

export default defineComponent({
  name: 'Register',
  setup() {
    const name = useField('name', { required: true });
    return { name };
  }
});
</script>

And now does not with Vue rc.8, as the automatic unwrapping no longer exists.
The workaround here is to wrap useField into a reactive() call:

const name = reactive(useField('name', { required: true }));

Even if this is all fixable in user land, maybe it would be worth tweaking the returned value from useField and useForm so they could work directly?

@logaretm
Copy link
Owner

Thanks for pointing this out, I have tried both proxyRefs and reactive and both breaks almost all tests if done internally which is to be expected.

And if I understand correctly this

const { value } = reactive(useField('name', { required: true }));

would break because destructing disables reactivity for the properties, unless being refs themselves prevents this. I will try to resolve the issues that come out of that breaking change but I'm not sure if this workaround would be in userland or in the library. I will resolve the issues first and see what I can do.

@logaretm logaretm added the 🐛 bug Unintended behavior label Aug 27, 2020
@logaretm
Copy link
Owner

logaretm commented Aug 28, 2020

I managed to get the lib working again with some hacks, will properly debug this later. But for the suggestion to wrap everything in reactive will actually cause a lot of issues and breaks the HOCs bundled in the library.

I think the reactive workaround would end up being a caveat of the composition API as userland code is subject to this as well. Will revise this once more libraries and practices start to form around this.

@cexbrayat
Copy link
Contributor Author

@logaretm 👍 I gave a try to alpha.9, and our tests are green again with the latest Vue RC.

@cexbrayat
Copy link
Contributor Author

@logaretm In fact I still have an issue (I can open a dedicated one if needed).

With a simple Form like the following:

<Form @submit="register($event)" v-slot="{ meta: formMeta }">
     <Field name="login" rules="required" v-slot="{ field }" v-model="user.name">
      <label for="login">Login</label>
      <input id="login" v-bind="field" />
      <ErrorMessage name="login" />
    </Field>
    <button :disabled="formMeta.invalid">Register</button>
  </Form>

The button is disabled with Vue rc.4 (as the form is invalid), but not with Vue rc.5, so I think there is a regression somewhere.

See cexbrayat/vee-infinite-loop#1 for a repro. This is working with rc.4, but not with rc.5 to rc.9.

@logaretm
Copy link
Owner

Thank you for the update, it looks like an issue with reactivity. Surprisingly adding these lines seem to fix it:

// FIXME: for whatever reason that's beyond me, this fixes the reactivity issue
watch(errors, () => { });
watch(meta, () => { });

Will try to find a real fix in the meantime, otherwise, I will tag a release with this hack.

@logaretm
Copy link
Owner

Tagged alpha 10 with the hotfix.

@cexbrayat
Copy link
Contributor Author

@logaretm I confirm that alpha.10 does fix the issue. Thank you for the quick fix and release!

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

No branches or pull requests

2 participants