Skip to content

Bug: Testing handlers should not require that you add parentheses #1769

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

Closed
lobo-tuerto opened this issue Sep 13, 2022 · 10 comments
Closed

Bug: Testing handlers should not require that you add parentheses #1769

lobo-tuerto opened this issue Sep 13, 2022 · 10 comments
Labels
bug Something isn't working needs reproduction

Comments

@lobo-tuerto
Copy link

Describe the bug
Right now if you have a child component that handles an emitted event by calling a function in the parent component, it will just fail if you don't add parenthesis to the event handler name.

To Reproduce
This doesn't work:

<MainSidebar
  :menu-items="menuItems"
  @expanded="handleExpanded"
/>

This works:

<MainSidebar
  :menu-items="menuItems"
  @expanded="(value) => handleExpanded(value)"
/>

I just found out about this unexpected behaviour by chance.
It's weird because in the Vue docs they use the non-parentheses version all over the place.

Expected behavior
This should work:

<MainSidebar
  :menu-items="menuItems"
  @expanded="handleExpanded"
/>

Related information:

This is with a newly created Vue 3 + Vite + TypeScript application (pnpm create vite).

Additional context

Alternative Solutions

For now, use parenthesis on all event handlers and manually pass any data to it... :(
Add information about this issue in a very visible place on the Vue Test Utils docs (didn't find anything about it when going through the site, it should be a big yellow warning box). :)

Previous discussions

vuejs/vue-test-utils#1901
vuejs/vue-test-utils#929
https://stackoverflow.com/questions/62696939/test-on-function-call-in-vue-template-only-passes-if-the-function-is-called-with/62703070#62703070

@lobo-tuerto lobo-tuerto added the bug Something isn't working label Sep 13, 2022
@lobo-tuerto lobo-tuerto changed the title Bug: Bug: Testing handlers should not require that you add parentheses Sep 13, 2022
@freakzlike
Copy link
Collaborator

I use @expanded="handleExpanded" without issues. Can you provide the code of your test case where you have this issue?

@lobo-tuerto
Copy link
Author

Here is a simple example:

test('MainSidebar events', async () => {
  const wrapper = mount(HomePage, {
    global: {
      plugins: [router],
    },
  })

  const spy = vi.spyOn(wrapper.vm, 'handleExpanded')

  const component = wrapper.findComponent(MainSidebar)
  component.vm.$emit('expanded')

  expect(component.emitted().expanded).toBeTruthy()
  expect(spy).toHaveBeenCalled()
})

That's basically the gist of it.
If you need a minimal reproduction case, I can create a public repo.

@cexbrayat
Copy link
Member

@lobo-tuerto Yes, a small repro would be great 👍

You can use https://stackblitz.com/edit/vitest-dev-vitest-pheruf?file=package.json to build one online in a few minutes.

@lobo-tuerto
Copy link
Author

@freakzlike @cexbrayat Sorry for taking a bit to respond.

I have repo with a brand new Vite + Vue 3 app that exhibits the behaviour discussed above right here: https://github.com./lobo-tuerto/test-utils-parentheses

Please clone it, install dependencies (I'm using pnpm), then run: pnpm vitest run

You'll see:

stdout | src/App.test.ts > event handling
this was called!

 ❯ src/App.test.ts (1)
   × event handling

 FAIL  src/App.test.ts > event handling
AssertionError: expected "someEventHandler" to be called at least once
 ❯ src/App.test.ts:17:14
     15| 
     16|   expect(component.emitted().something).toBeTruthy()
     17|   expect(spy).toHaveBeenCalled()
       |              ^
     18| })
     19| 

Test Files  1 failed (1)
     Tests  1 failed (1)
  Start at  00:30:01
  Duration  2.66s (transform 429ms, setup 0ms, collect 114ms, tests 22ms)

Please note that this line in src/App.vue does not have parentheses in its event handler:

<HelloWorld msg="Vite + Vue" @something="someEventHandler" />

Now, add parentheses to it like this:

<HelloWorld msg="Vite + Vue" @something="someEventHandler()" />

Run pnpm vitest run again and you'll see:

stdout | src/App.test.ts > event handling
this was called!

 ✓ src/App.test.ts (1)

Test Files  1 passed (1)
     Tests  1 passed (1)
  Start at  00:34:10
  Duration  2.46s (transform 1.07s, setup 0ms, collect 114ms, tests 21ms)

That's basically the issue.
Hope this helps troubleshoot what's going on.

Thank you in advance.

@cexbrayat
Copy link
Member

cexbrayat commented Sep 19, 2022

Thanks for the repro. I think this might be a tricky one. I suppose there is something going on with the spy that can't catch the call to the listener if there are no parenthesis as the code generated by Vue is { onClick: onClick } for @click="onClick" and { onClick: $event => (onClick()) } for @click="onClick()"

We already had a similar discussion in #775 and I don't think we can do much on our side...

@cexbrayat
Copy link
Member

To illustrate that this is a JS/spy problem and not a VTU one, this test fails:

  it('calls the spy', () => {
    const b = { onClick: () => console.log('on click') }
    const comp = { a: b.onClick }
    const spy = vi.spyOn(b, 'onClick')
    comp.a()
    expect(spy).toHaveBeenCalled()
  })

change the test to

it('calls the spy', () => {
    const b = { onClick: () => console.log('on click') }
    const comp = { a: () => b.onClick() }
    const spy = vi.spyOn(b, 'onClick')
    comp.a()
    expect(spy).toHaveBeenCalled()
  })

and the test succeeds. I think I'll close the issue as this is not solvable in VTU.

@cexbrayat cexbrayat closed this as not planned Won't fix, can't repro, duplicate, stale Sep 19, 2022
@lobo-tuerto
Copy link
Author

Hmm, OK thanks for the extra information.
Where would be a good place to report this? Vitest maybe? 🤔

@lobo-tuerto
Copy link
Author

Or maybe add a note in the documentation about this behaviour for the Composition API?

@cexbrayat
Copy link
Member

I think all mocking libraries will have the same behavior.
Maybe we could add a note somewhere, but I'm unsure where that would make sense.

Maybe Vue itself should generate { onClick: $event => (onClick()) } in all cases, I don't know

@lobo-tuerto
Copy link
Author

Thank you. I'll open issues in those repos and see if someone can help. ✌️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs reproduction
Projects
None yet
Development

No branches or pull requests

3 participants