QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.83k stars 1.31k forks source link

[🐞] `userEvent` from `createDOM` (qwik/testing) util doesn't trigger on input type="checkbox" #5221

Open riencoertjens opened 1 year ago

riencoertjens commented 1 year ago

Which component is affected?

Qwik Rollup / Vite plugin

Describe the bug

I'm trying to trigger a click on a checkbox in a vitest using userEvent doesn't change the checked value of a checkbox

import { createDOM } from '@builder.io/qwik/testing';
import { test, expect } from 'vitest';
import { Checkbox } from './checkbox';

test(`Should select / deselect (but doesn't)`, async () => {
  const { screen, render, userEvent } = await createDOM();
  await render(<Checkbox variant="action" />);

  const selector = 'input[type="checkbox"]';
  const checkbox = screen.querySelector(selector) as HTMLInputElement;

  expect(checkbox.checked).toBeFalsy();

  await userEvent(selector, 'click'); // doesn't work
  // await userEvent(checkbox, 'click'); // doesn't work
  // await userEvent('input[type="checkbox"]', 'click'); // doesn't work
  // await userEvent(screen.querySelector(selector) as HTMLInputElement, 'click'); // doesn't work

  expect(checkbox.checked).toBeTruthy(); // fails
});

replacing userEvent(selector, 'click') with screen.querySelector(selector).click() works as expected

Reproduction

https://github.com/riencoertjens/qwik-vitest/blob/main/src/components/example/checkbox.test.tsx#L22

Steps to reproduce

npm install + npm run test.unit on reproduction repo shows the passing and failing tests

System Info

System:
    OS: macOS 13.5.2
    CPU: (10) arm64 Apple M1 Pro
    Memory: 1.35 GB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.16.0 - ~/.nvm/versions/node/v18.16.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v18.16.0/bin/yarn
    npm: 9.5.1 - ~/.nvm/versions/node/v18.16.0/bin/npm
  Browsers:
    Brave Browser: 117.1.58.127
    Chrome: 117.0.5938.92
    Firefox: 116.0.3
    Safari: 16.6
  npmPackages:
    @builder.io/qwik: ^1.2.12 => 1.2.12 
    @builder.io/qwik-city: ^1.2.12 => 1.2.12 
    undici: 5.22.1 => 5.22.1 
    vite: 4.4.7 => 4.4.7

Additional Information

No response

mhevery commented 1 year ago

So when I run your test the generated DOM is this:

<input type="checkbox"  class="..." />

There is no click listener on the input hence nothing is happening.... I think this may be an expectation misalignment.

Normally where you have <button on:click="..."> in a test, you can't just click on it, because the DOM element is not attached to a root document. So if you do buttonElement.click() it will not do anything because there is no document that contains the qwikloader to process the click event. To work around this problem, we provide a userEvent which emulates how qwikloader works, by walking the dom tree and looking for the on:click attribute and processing the QRL attached there. The userEvent does not actually fire a real DOM event, it only triggers the Qwik's event handler attributes.

In your case, the <input type="checkbox" /> has no on:click events; hence there is no action. It seems like what you are looking for is to fire an actual browser click event so that the default browser checkbox can flip state. As of right now, this is not the purpose of userEvent

Solution

I am not sure what the right solution is so I would like to start a discussion. Here are the potential outcomes.

  1. Document this as works as intended....
  2. Change the behavior of userEvent to not only process the on:event attributes but also fire the actually event so that it can be closer to the expected behavior.

Would love to hear your thoughts.

riencoertjens commented 1 year ago

if the behaviour of userEvent is not changed I would at least rethink the naming, when do something as a user I don't about whether it should be a qwik event or browser event. That being said, I expected solution no. 2 to be the case.

riencoertjens commented 1 year ago

it seems they had a similar discussion in the testing-library lib repo, there they have fireEvent which does the same as the qwik userEvent. Out of that discussion came testing-library/user-event which implements userEvent how I was expecting it to work, having used testing library in the past that's probably why I was expecting it in the first place

mhevery commented 1 year ago

I see, so your suggestion is that we should incorporate https://github.com/testing-library/user-event into userEvent and simulate both.... That does seem reasonable...

Any chance you could take that on as a PR?