fix: update VTU to v1, update types

[afenton90]

Upgrades version of @vue/test-utils to ^1.0.1.

• Replace attachToDocument with attachTo and mirror default behaviour
• Remove guard for wrapper.destroy as destroy must always be called now is using attachTo see @vue/test-utils docs here.
• Adds config.showDeprecationWarnings = false as isVueInstance is still used internally by @vue/test-utils and gives warnings.

closes #138

[afenton90]

☝️ test failed because Cannot find module '@babel/compat-data/corejs3-shipped-proposals' doesn't seem to have change in package-lock.json file though.

[afontcu]

Hm, did this test start to fail? 🤔 I mean, did Vuetify changed anything? I'm worried this is related to attachTo

[afenton90]

Yeah it did start to fail. When I debugged though it seemed like once menu list was open it just added display none to hide it.
Let me debug with attachToDocument to see what it was outputting

[afenton90]

So with master this works fine. Using debug shows that the menu item is removed from the DOM.
However, with the latest version of VTU and attachToDocument still show this in the DOM.

So looks like an issue with the latest release of VTU rather than use of attachTo

[lmiller1990]

Ok, I spent some time on this. The breaking change is actually from vue/test-utils@1.0.0-beta.32 -> vue/test-utils@1.0.0-beta.33.

I will debug a bit more.

[lmiller1990]

Don't ask how I figured this out but THIS LINE is the problem. Adding it back in fixes this test.

I do not know why this was removed or what this line does.

[afontcu]

Hey! 👋 yeah, I'm currently looking into it. I started to think that toBeInTheDocument() was actually not the right assertion.

AFAICT, Vuetify does render the element but hides it, so toBeInTheDocument() should have passed. Now, I don't have much experience with Vuetify, so I might be wrong.

I am unable, though, to make this pass (and I think it should):

test('opens a menu', async () => {
  const {getByText, queryByText} = renderWithVuetify(VuetifyDemoComponent)

  const openMenuButton = getByText('open menu')

  // Menu item is initially not rendered
  expect(queryByText('menu item')).not.toBeInTheDocument()

  await fireEvent.click(openMenuButton)

  const menuItem = getByText('menu item')

  expect(menuItem).toBeInTheDocument()
  expect(menuItem).toBeVisible() // <-- this fails (it still has "display: none;")

  await fireEvent.click(openMenuButton)

  expect(menuItem).not.toBeVisible()
})

[afenton90]

@afontcu so just tried the menu in a project I have locally, the browser behaviour is:

1. Mounted v-menu does not appear in the document
2. Users clicks activator. In my case a v-text-field. v-menu is inserted into the DOM
3. User clicks activator again to close the menu. v-menu is still in the DOM but has display: none

So the edit to the test looks valid based on this behaviour.
Looks like the test before was reporting a false positive.

[afontcu]

Looks like the test before was reporting a false positive.

Yes! That's what I thought, too 🙌 . The snippet I posted above is currently failing, I'll try to dig a bit more and see if I can figure out why. Thought it could have something to with transitions, even though I tried with <v-menu :transition="false" /> but nothing changed.

[afenton90]

Tried a lot of different versions of this test, then figured:

"I wonder how Vuetify are testing the menu component"

Turns out the definition of this "working" is that it renders with display: none even when clicked whilst testing with VTU.

There is a snapshot here with display: none https://github.com/vuetifyjs/vuetify/blob/9ac1158f857fd1f531f1177ba623602973f81b55/packages/vuetify/src/components/VMenu/__tests__/__snapshots__/VMenu.spec.ts.snap#L15
With the corresponding test here https://github.com/vuetifyjs/vuetify/blob/9ac1158f857fd1f531f1177ba623602973f81b55/packages/vuetify/src/components/VMenu/__tests__/VMenu.spec.ts#L32

[afontcu]

okay, this is interesting. Now I'm leaning towards simplifying our Vuetify test (which is only aimed at showing a way of configuring VTL and Vuetify) to make it more resilient. If they are happy with display:none, we should be, too. I'm gonna push some changes soon to this branch, and make the test pass.

Thanks for your help, @afenton90! Much appreciated :)

[marcpeabody]

Does this still need work before it can be merged? I see the CI failing Error: Cannot find module '@babel/compat-data/corejs3-shipped-proposals'

[afontcu]

Does this still need work before it can be merged? I see the CI failing Error: Cannot find module '@babel/compat-data/corejs3-shipped-proposals'

hey! yes, it is still WIP. It's been a busy week and I couldn't move it forward. I'll try to find some time, but happy to see it move forward if anyone's interested!

Things I'd like to see fixed before merging this

• Is the failing vuetify.js test an upstream issue with VTU, or it is on Vue Testing Lib? I'm a bit worried it is related with how we're attaching to the container (here).
• Fix CI

[afenton90]

@afontcu I tried a few different versions of the upgrade to see if I could figure out why the test has to be changed. Unfortunately, I have no extra information on top of my last comment here #139 (comment) which is my best theory at the moment.

As for fixing of CI it seems to be specific to node 14 build. I can't see anything directly correlated with code changes in the PR.

Any help to get this over the line would be appreciated.

[afontcu]

As for fixing of CI it seems to be specific to node 14 build. I can't see anything directly correlated with code changes in the PR.

Looks like it was fixed in kcd-scripts v5.11.1: kentcdodds/kcd-scripts#139. Just pushed a version bump, all green now :)

[codecov]

Codecov Report

Merging #139 into master will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #139   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           68        69    +1     
  Branches        14        13    -1     
=========================================
+ Hits            68        69    +1     

Impacted Files	Coverage Δ
src/vue-testing-library.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22c34f7...dc38df6. Read the comment docs.

[kuddl]

Please merge?? Sounds like it's ready to go ? And our consoles are cluttered with error messages!

[afontcu]

Please merge?? Sounds like it's ready to go ? And our consoles are cluttered with error messages!

I really want to figure out what's going on with Vuetify test (https://github.com/testing-library/vue-testing-library/pull/139/files#r420852127) before merging this!

[spacedawwwg]

@afenton90 …fancy seeing you here! 🤣

[afontcu]

🎉 This PR is included in version 5.0.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[mediafreakch]

@afontcu Just to let you know: @vue/test-utils@1.0.0 now stubs transitions by default. After updating, some of my tests were failing because of this. The solution was to disable stubbing for transitions for those tests:

it('should mount', () => {
  /* MyComponent wrapped <v-snackbar /> which uses <transition> */
  const { getByText } = render(MyComponent, {
    props: {},
    stubs: {
      transition: false;
    }
  })
})

Maybe this belongs to the release docs here to avoid someone else having to google the solution :)

After updating I also have Typescript errors coming from @types/vue-testing-library__vue:

ERROR in .../node_modules/@types/testing-library__vue/node_modules/vuex/types/vue.d.ts(10,5):
10:5 Subsequent property declarations must have the same type.  Property 'store' must be of type 'Store<any> | undefined', but here has type 'Store<any> | undefined'.
     8 | declare module "vue/types/options" {
     9 |   interface ComponentOptions<V extends Vue> {
  > 10 |     store?: Store<any>;
       |     ^
    11 |   }
    12 | }
    13 | 

I temporarily disabled the errors by adding the following to my tsconfig.json:

...,
compilerOptions: {
  skipLibCheck: true
}

[afontcu]

Hi! Thanks for the insights. How did your test break? Sync vs. async behaviour? Could you share the failing component / test?

---

Property 'store' must be of type 'Store<any> | undefined', but here has type 'Store<any> | undefined'.

you gotta love typescript errors 😅

[mediafreakch]

@afontcu Here's the failing test:

describe('ErrorMessage', () => {
  it('should not be visible by default', () => {
    const { queryByTestId } = render(ErrorMessage, {})

    expect(queryByTestId('snackbar')).toBe(null)
  })
})

The test failed with:

Expected: null
Received: <transition-stub data-testid="snackbar" name="v-snack-transition" />

Here the tested component (I removed some parts that are unrelated to the initial render):

<template>
  <v-snackbar :value="isVisible" multi-line :timeout="0" color="secondary" data-testid="snackbar">
    <span>
      {{ message }}
    </span>
  </v-snackbar>
</template>

<script lang="ts">
  import Vue from 'vue'

  export default Vue.extend({
    props: {
      errorCode: Number,
      errorIsIntrusive: {
        type: Boolean,
        default: false
      },
    },
    data() {
      return {
        isVisible: false
      }
    },
    watch: {
      errorCode: {
        handler: function(newVal) {
          if (newVal) {
            // we show only if error is not intrusive (intrusive errors are handled elsewhere)
            this.isVisible = !!newVal && !this.errorIsIntrusive
          }
        },
        immediate: true
      },
    },
    computed: {
      message(): string {
        return `${this.$t('Error.TextForCode', { code: this.errorCode })}`
      }
    },
  })
</script>

[afontcu]

Cool! Thanks for the info. I'll add the information in the v5.0.3 release, so that people can find it easily.