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

When using user.click in @testing-library/angular, an error is thrown #103

Closed
crutchcorn opened this issue May 14, 2020 · 10 comments · Fixed by testing-library/user-event#302 or #104
Labels

Comments

@crutchcorn
Copy link

I have the following test written to test a pagination component:

import user from '@testing-library/user-event';

...

    const nextPageBtn = screen.getByLabelText('Go to the next page');
    user.click(nextPageBtn);
    expect(pageIndexChange).toHaveBeenCalledWith(3);

    const previousPageBtn = screen.getByLabelText('Go to the previous page');
    user.click(previousPageBtn);
    expect(pageIndexChange).toHaveBeenCalledWith(2);

    const firstPageBtn = screen.getByLabelText('Go to the first page');
    user.click(firstPageBtn);
    expect(pageIndexChange).toHaveBeenCalledWith(0);

    const lastPageBtn = screen.getByLabelText('Go to the last page');
    user.click(lastPageBtn);
    expect(pageIndexChange).toHaveBeenCalledWith(4);

With one of the buttons being formed like this:

  <button
    type="button"
    class="btn-paginator btn-side ml-1"
    [disabled]="pageIndex === lastPage"
    (click)="gotoPage(lastPage)"
    [attr.aria-label]="'Go to the last page' | translate"
  >
    <i class="fas fa-angle-double-right" aria-hidden="true"></i>
  </button>

(With the translate pipe coming from ngx-translate)

However, when I run the tests, I get the following error.

● PaginatorComponent › should handle paging forward and backward properly

    TypeError: Cannot read property '_defaultView' of undefined

      90 |
      91 |     const previousPageBtn = screen.getByLabelText('Go to the previous page');
    > 92 |     user.click(previousPageBtn);
         |          ^
      93 |     expect(pageIndexChange).toHaveBeenCalledWith(2);
      94 |
      95 |     const firstPageBtn = screen.getByLabelText('Go to the first page');

      at Object.exports.fireFocusEventWithTargetAdjustment (node_modules/jsdom/lib/jsdom/living/helpers/focusing.js:71:33)
      at HTMLButtonElementImpl.blur (node_modules/jsdom/lib/jsdom/living/nodes/HTMLAndSVGElementShared-impl.js:52:14)
      at HTMLButtonElement.blur (node_modules/jsdom/lib/jsdom/living/generated/HTMLElement.js:38:23)
      at clickElement (node_modules/@testing-library/user-event/dist/index.js:71:49)
      at Object.click (node_modules/@testing-library/user-event/dist/index.js:214:9)
      at src/app/@shared/paginator/paginator.component.spec.ts:92:10
      at fulfilled (node_modules/tslib/tslib.js:110:62)
      at ZoneDelegate.Object.<anonymous>.ZoneDelegate.invoke (node_modules/zone.js/dist/zone.js:386:30)
      at ProxyZoneSpec.Object.<anonymous>.ProxyZoneSpec.onInvoke (node_modules/zone.js/dist/proxy.js:117:43)

However, if I switch from user.click to fireEvent from @testing-library/angular, I get no such error and the tests pass

@kentcdodds
Copy link
Member

I'm pretty sure it's failing here:

https://github.com/testing-library/user-event/blob/e2272ab58675a5bafda3633b4cc9bf5f50d5dab9/src/index.js#L52

I'm not sure what the previousElement is... But I'm guessing that after trying to blur that element, jsdom attempts to focus a new element which is undefined. I'm not sure what's happening there. Do you mind doing a little digging? If you could add some console logs around node_modules/jsdom/lib/jsdom/living/helpers/focusing.js:71:33 that might help.

@kentcdodds
Copy link
Member

Here's the code for that: https://unpkg.com/browse/jsdom@16.2.2/lib/jsdom/living/helpers/focusing.js

// https://html.spec.whatwg.org/multipage/interaction.html#fire-a-focus-event plus the steps of
// https://html.spec.whatwg.org/multipage/interaction.html#focus-update-steps that adjust Documents to Windows
exports.fireFocusEventWithTargetAdjustment = (name, target, relatedTarget) => {
  if (target === null) {
    // E.g. firing blur with nothing previously focused.
    return;
  }

  const event = createAnEvent(name, target._globalObject, FocusEvent, {
    composed: true,
    relatedTarget,
    view: target._ownerDocument._defaultView,
    detail: 0
  });

  if (target._defaultView) {
    target = idlUtils.implForWrapper(target._defaultView);
  }

  target._dispatch(event);
};

It's either failing at:

    view: target._ownerDocument._defaultView,

Or:

  if (target._defaultView) {

If you could figure out exactly where and what that target actually is that would probably help us track down what's going on.

@crutchcorn
Copy link
Author

I'm still looking into it, but it seems that target is properly set to #document:

contentType: "text/html"
nodeType: 9

Then the:

target = idlUtils.implForWrapper(target._defaultView);

It's then that target becomes this:

image

Which is why view: target._ownerDocument._defaultView, fails, because _ownerDocument is undefined

@kentcdodds
Copy link
Member

Thanks for that investigation! I'm starting to think this is a JSDOM bug. What do you think?

@crutchcorn
Copy link
Author

I agree. I don't see anything that's implementation specific to user-event here.

I'll make a issue on jsdom outlining my findings and link it here

@timdeschryver
Copy link
Member

👋 I'm the maintainer of Angular Testing Library.
Have you also tried this with the userEvent from the Angular Testing Library?

import { userEvent } from '@testing-library/angular

I think the change detection of Angular is causing issues here.
If you use the events directly from this user-event package, you will have to manually call detectChanges to invoke a change detection cycle. Without it, the DOM won't be updated.

What I did with Angular Testing Library is to patch each invoked event, and it will run the detection cycle after each fired event. This is also the case with fireEvent.click, with as result that your test will pass.

kentcdodds referenced this issue in testing-library/dom-testing-library Jun 1, 2020
This is intended for supporting `act` in React, but should be useful for
other frameworks (I think it could help with triggering change detection
for angular for example).

Ref: testing-library/user-event#188,
testing-library/user-event#255,
https://github.com/testing-library/user-event/issues/277
kentcdodds referenced this issue in testing-library/dom-testing-library Jun 1, 2020
This is intended for supporting `act` in React, but should be useful for
other frameworks (I think it could help with triggering change detection
for angular for example).

Ref: testing-library/user-event#188,
testing-library/user-event#255,
https://github.com/testing-library/user-event/issues/277
kentcdodds referenced this issue in testing-library/react-testing-library Jun 1, 2020
Now not only will React Testing Library's `fireEvent` be wrapped in
`act`, but so will DOM Testing Library's `fireEvent` (if
`@testing-library/react` is imported). It works very similar to async
act for the `asyncWrapper` config.

Closes: testing-library/user-event#188
Closes: testing-library/user-event#255
Reference: https://github.com/testing-library/user-event/issues/277
@kentcdodds
Copy link
Member

🎉 This issue has been resolved in version 10.4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@kentcdodds
Copy link
Member

Whoops, I wasn't supposed to close this. However, the latest version should allow @testing-library/angular to use the new eventWrapper to automatically trigger change detection after fireEvent even when using @testing-library/dom directly. Very similar to this: testing-library/react-testing-library@7bd8be8

@kentcdodds kentcdodds reopened this Jun 1, 2020
@kentcdodds kentcdodds transferred this issue from testing-library/user-event Jun 1, 2020
@timdeschryver
Copy link
Member

Thanks @kentcdodds! I was thinking about providing this setup in user-event, but it's better to do so in Dom Testing Library. I will try to add it here later.

In the next major version we can then remove the custom user event actions from ATL 🎉.

@timdeschryver
Copy link
Member

🎉 This issue has been resolved in version 9.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants