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

jest-dom added additional condition to toBeVisible preventing use in component test. #102

Open
milkshakeuk opened this issue May 16, 2022 · 9 comments

Comments

@milkshakeuk
Copy link

milkshakeuk commented May 16, 2022

I'm submitting a bug report

  • Library Version:
    1.1.0

Please tell us about your environment:

  • Operating System:
    Windows 11

  • Node Version:
    16.5.0

  • NPM Version:
    7.20.0

  • JSPM OR Webpack AND Version
    webpack: 5.72.1

  • Browser:
    jest component test

  • Language:
    typescript

Current behavior:
We recently updated our dependencies and it looks like jest-dom changed the behaviour of toBeVisible and added a new condition in v5.11.10 so all versions since then (2021-03-25) have the same issue:

it is present in the document

This is causing our component tests to fail with an exception, when we locate an element from the component and try to asset that it is visible.

image

I am assuming this can be fixed somehow by a change in the aurelia-testing package.

Expected/desired behavior:
it should not fail or throw an exception

  • What is the expected behavior?

  • What is the motivation / use case for changing the behavior?
    the jest-dom package has added an extra condition to its toBeVisible method.

@milkshakeuk
Copy link
Author

@bigopon does this make sense to you? is there any way to fix this from the aurelia plugin?

@bigopon
Copy link
Member

bigopon commented May 16, 2022

you can see from the code here, the elements are supposed to be in the document if they are resolved from waitForDocumentElement

return waitFor(() => document.querySelector(selector) as Element, options);

Can you check again?

@milkshakeuk
Copy link
Author

milkshakeuk commented May 17, 2022

you can see from the code here, the elements are supposed to be in the document if they are resolved from waitForDocumentElement

return waitFor(() => document.querySelector(selector) as Element, options);

Can you check again?

If you look at the screenshot, waitForDocumentElement is what it is failing on, I have run the test multiple times, when I revert the version of jest-dom to an earlier version it works, it's only since they added the extra condition that it fails.

toBeVisible implementation calls getRootNode() which blows up saying it's not a function, I was thinking this would need to attach something to make it work.

@milkshakeuk
Copy link
Author

milkshakeuk commented May 17, 2022

@bigopon I did some digging and found a issue, it looks like this is how it was fixed for vue-test-utils testing package.

They can mount the component to the document body within the testing framework, can you add something similar to aurelia-testing? here is the source code for mount in vue-test-utils:
https://github.com/vuejs/vue-test-utils/blob/3cd81d0593f56034b96f368d8ab066a855e0b204/packages/test-utils/src/mount.js

@bigopon
Copy link
Member

bigopon commented May 17, 2022

As you can see from this line

document.body.appendChild(this.host);

All tests attach their root node to the document. So it's unlikely that the issue is in this package. Maybe your jsdom dep is old, and doesn't have getRootNode() implemented, if so, maybe upgrade jsdom?

@milkshakeuk
Copy link
Author

milkshakeuk commented May 17, 2022

it looks like jsdom first introduced getRootNode in version 11.11.0, however in my package lock I see this:

image

image

it looks like theaurelia-pal-nodejs (which we don't directly reference) depends on an old version of jsdom which doesn't support getRootNode.
it looks like aurelia-pal-nodejs is a dependency of aurelia-store and we do reference that.

@milkshakeuk
Copy link
Author

@bigopon I have opened a ticket with jest-dom as its likely something they should deal with.

testing-library/jest-dom#458

@milkshakeuk
Copy link
Author

milkshakeuk commented May 19, 2022

@bigopon I just realise we use a jest-pretest.ts file which adds some DOM polyfils for node:

import "aurelia-polyfills";

import { Options } from "aurelia-loader-nodejs";
import { globalize } from "aurelia-pal-nodejs";

Options.relativeToDir = "app";
globalize();

I had a look at aurelia-pal-nodejs and found this
shouldn't the getRootNode polyfill be included here?

or is that abstraction just for the things that aurelia needs?

@bigopon
Copy link
Member

bigopon commented May 19, 2022

That could possibly be the case. Maybe it should. Can you try add that in your local node_modules and see if the error goes away?

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

No branches or pull requests

2 participants