Dom-testing-library: querySelectorAll is not a function when being called by getByTestId

[Nubzor]

In the beginning, I would like to thank you for the time you folks put to develop and maintain the library.

I face an issue related to missing .querySelectorAll function while calling getByTestId method which is odd because when I check the object property then I can see that the function exists :shrug:

Code example:

```import { render, getByTestId } from "@testing-library/svelte";
import Loader from "./Loader.svelte";

describe("Loader component", () => {
test("should render component correctly", () => {
const { container } = render(Loader);

console.log(typeof container.querySelectorAll)

expect(container).toContainElement(getByTestId("spinner"));
expect(container).toBeVisible();

});
});

Output: 

``` FAIL  src/Loader/Loader.spec.js
  Loader component
    × should render component correctly (22ms)

  ● Loader component › should render component correctly

    TypeError: container.querySelectorAll is not a function

> 10 |     expect(container).toContainElement(getByTestId('spinner'));

-------

Test Suites: 2 failed, 2 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        3.162s
Ran all test suites matching /src/i.
  console.log     
    Log:  function

There is a list of dependencies and their versions:

    ...
    "@testing-library/jest-dom": "^5.5.0",
    "@testing-library/svelte": "^3.0.0",
    ...
    "jest": "^25.4.0",
    "jest-transform-svelte": "^2.1.1",
    "svelte": "^3.21.0",

Node: 12.16.1 on Windows 10

I am not sure whether this is a bug or I missed something during the configuration, any tips would be appreciated.

Greetings,

Mateusz

[mihar-22]

Hey @Nubzor,

Thanks for the kind words. Based on your example it seems like you didn't deconstruct getByTestId from the render function. If you look at the API for getByTestId then the first parameter is a container element to run on the queries on. The testing library already does this for you and returns it in the result of the render function.

const { getByTestId } = render(Loader);

// Some action here that will cause the spinner to be displayed.

expect(getByTestId('spinner')).toBeVisible();

That should resolve it for you :) Side recommendation for structural tests you can use snapshots in Jest, makes life much easier so your main tests can focus on the UX and not brittle unit tests.

const { container } = render(Loader);

expect(container).toMatchSnapshot();

[alexkrolick]

We could probably typecheck the 1st arg to throw a more specific error case in this case

[Nubzor]

@mihar-22 that was exactly the case. When I used the getByTestId function the VisualStudio Code has automatically added the import statement from "@testing-library/svelte" so I was biased by this behaviour.

Also, thank you for the side recommendation I definitely find it useful - I am on my first private project using Svelte so I am trying different solutions and approaches to get used to the library.

Thank you.

PS. I agree with Alex, type check could be helpful for future devs facing the same problem I had.

[mihar-22]

I like the idea @alexkrolick but how would we go about this? Maybe a one-time warning? Some people might actually need to import these functions for other use cases.

Another idea could be we don't export it from this library at all, and when required you'd use the DOM testing library directly. It'd also stop editors from automatically importing it and causing any confusion. What do you think?

[alexkrolick]

Just warn when the function is called with a first argument that isn't a DOM node. It will fail anyways with a confusing error

We would probably want to do this at the DOM Testing Lib level, although there could be other things added on for framework wrappers.

Removing the render-result binding in favor of screen would be a big breaking change so we might not want to do that for a while.

[Jnforja]

Hi,

I'd like to work on this issue. If I understood correctly, the goal is to have all of the base queries throw an error warning that the first argument of a base query must be a DOM node. Which includes the following queries and its variants:

• ByLabelText
• ByPlaceholderText
• ByText
• ByAltText
• ByTitle
• ByDisplayValue
• ByRole
• ByTestId

Is this what is intended?

[kentcdodds]

Every query is built on top of the queryAllBy* variant, so you'd only need to worry about that one for each type :)

Thanks!

[kentcdodds]

To be clear, these days we're encouraging people to use screen which means that most of the time people won't be passing the container argument themselve's (it'll be pre-bound). But having this runtime check definitely wouldn't hurt.

[Jnforja]

Every query is built on top of the queryAllBy* variant, so you'd only need to worry about that one for each type :)

Thanks!

Thank you for the tip!

To be clear, these days we're encouraging people to use screen which means that most of the time people won't be passing the container argument themselve's (it'll be pre-bound). But having this runtime check definitely wouldn't hurt.

Yes, I'm aware. But thank you for calling it to my attention :) I'll get to work then!

[kentcdodds]

:tada: This issue has been resolved in version 7.10.0 :tada:

The release is available on:

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

Your semantic-release bot :package::rocket: