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

querySelectorAll is not a function when being called by getByTestId #537

Closed
Nubzor opened this issue Apr 28, 2020 · 10 comments
Closed

querySelectorAll is not a function when being called by getByTestId #537

Nubzor opened this issue Apr 28, 2020 · 10 comments
Labels
enhancement New feature or request help wanted Extra attention is needed released

Comments

@Nubzor
Copy link

Nubzor commented Apr 28, 2020

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 🤷

Code example:

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:

  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
Copy link

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
Copy link
Collaborator

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

@Nubzor
Copy link
Author

Nubzor commented Apr 29, 2020

@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
Copy link

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
Copy link
Collaborator

alexkrolick commented Apr 30, 2020

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.

@alexkrolick alexkrolick transferred this issue from testing-library/svelte-testing-library Apr 30, 2020
@kentcdodds kentcdodds added enhancement New feature or request help wanted Extra attention is needed labels May 10, 2020
@Jnforja
Copy link
Contributor

Jnforja commented Jun 3, 2020

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor

Jnforja commented Jun 3, 2020

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!

Jnforja added a commit to Jnforja/dom-testing-library that referenced this issue Jun 4, 2020
This is intended to give an informative warning when
a base-query is called with a container that's neitheir
an Element, Document or DocumentFragment.

Ref: testing-library#537
Jnforja added a commit to Jnforja/dom-testing-library that referenced this issue Jun 4, 2020
This is intended to give an informative warning when
a base-query is called with a container that's neitheir
an Element, Document or DocumentFragment.

Ref: testing-library#537
@kentcdodds
Copy link
Member

🎉 This issue has been resolved in version 7.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

qijixin2 added a commit to qijixin2/dom-testing-library that referenced this issue Aug 26, 2022
This is intended to give an informative warning when
a base-query is called with a container that's neitheir
an Element, Document or DocumentFragment.

Closes: testing-library/dom-testing-library#537
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed released
Projects
None yet
Development

No branches or pull requests

5 participants