testing-library / vue-testing-library

🦎 Simple and complete Vue.js testing utilities that encourage good testing practices.
http://testing-library.com/vue
MIT License
1.08k stars 110 forks source link

Can't use fireEvent.update on a standard input field with Vuelidate #178

Closed kinoli closed 3 years ago

kinoli commented 3 years ago

I'm not sure if this is vuelidate related or not but fireEvent.update doesn't seem to work on a standard input field. I can fix it by updating vue-testing-library.js to change the fireEvent.input to fireEvent.change

Such as on line 247.

    case 'INPUT':
      {
        if (['checkbox', 'radio'].includes(type)) {
          elem.checked = true;
          return fireEvent.change(elem);
        } else {
          elem.value = value;
          return fireEvent.input(elem);
        }
      }

to

    case 'INPUT':
      {
        if (['checkbox', 'radio'].includes(type)) {
          elem.checked = true;
          return fireEvent.change(elem);
        } else {
          elem.value = value;
          return fireEvent.change(elem);
        }
      }

The above change fixes the issue. Any chance we can use change instead of input. Perhaps there's a bug with the input function.

kinoli commented 3 years ago

Note, the input changes as expected when I use the fireEvent.change method instead of fireEvent.input or fireEvent.update. However, I get the console warning when using fireEvent.change, so I can't use that anymore.

afontcu commented 3 years ago

Hi! Can you provide a failing test?

JeromeDeLeon commented 3 years ago

I don't think all components listen to onChange though not at least for the default v-model (value/input) case. While some expect to just input and some use change. If we emit them both, there will be a chance to introduce a bug where some people separate logic from input and change and that they only want to emit one event and not the other.

JeromeDeLeon commented 3 years ago

I believe we could at least have the ability to turn off the warning or do we really want to force all users to migrate to update?

kinoli commented 3 years ago

yes that would be a welcome ability, to turn off the warning.

afontcu commented 3 years ago

Any chance we can use change instead of input. Perhaps there's a bug with the input function.

The logic of input/change seems fine and aligned to what Vue Test Utils do and Vue components do, too. As Jerome mentions, we can't emit them both without introducing unexpected behaviors

we could at least have the ability to turn off the warning

That sounds reasonable, because actually nothing stops people to use fireEvent.change (it's just a warning, not that the method is gone or something). Fancy to open up a PR? 🙌

afontcu commented 3 years ago

181 introduced the ability to mute the warnings

I'll go ahead and close this one, as the changing the fired event on input/change is not desirable.

Thank you all!

kinoli commented 3 years ago

So, I discovered the cause of why fireEvent.input doesn't work for me. I'm using v-model.lazy. Does anyone know if there is a workaround. It prevents fireEvent.update and fireEvent.input from working, but fireEvent.change is the only event that works on it.

kinoli commented 3 years ago

You can test this by adding v-model.lazy on line 6 of Form.vue. It will make the test fail.

kinoli commented 3 years ago

This Pull Request will fix the lazy modifier issue. https://github.com/testing-library/vue-testing-library/pull/183