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

waitForElementToBeRemoved: TypeError: Cannot read property 'parentElement' of null #870

Closed
AriPerkkio opened this issue Jan 11, 2021 · 4 comments · Fixed by #871
Closed
Labels

Comments

@AriPerkkio
Copy link
Contributor

  • Fresh project generated by create-react-app@4:
  • @testing-library/dom version: 5.11.4
  • Testing Framework and version:
    • jest@26.6.0
  • DOM Environment:
    • jest-environment-jsdom@26.6.2
    • jsdom@16.4.0

Relevant code or config:

Same code is available in codesandbox below. The real use case is a bit different but I was able to reproduce the issue with this example:

import React, { useReducer } from "react";
import Modal from "react-modal";
import userEvent from "@testing-library/user-event";
import {
    render,
    waitForElementToBeRemoved,
    screen,
} from "@testing-library/react";

beforeAll(() => {
    const root = document.createElement("div");
    root.id = "app-root";
    document.body.appendChild(root);
    Modal.setAppElement("#app-root");
});

function Component() {
    const [isOpen, toggle] = useReducer((s) => !s, false);

    return (
        <>
            <button onClick={toggle}>Open</button>

            <Modal isOpen={isOpen}>
                <button onClick={toggle}>Close</button>
                {isOpen && <div>Content</div>}
            </Modal>
        </>
    );
}

test("poc", async () => {
    render(<Component />);

    userEvent.click(screen.getByText("Open"));
    const content = await screen.findByText("Content");

    userEvent.click(screen.getByText("Close"));
    await waitForElementToBeRemoved(content);
});

What you did:

Queried element rendered inside react-modal. Closed the modal and waited for the content to disappear. The modal seems to do some direct DOM manipulation, https://github.com/reactjs/react-modal/blob/master/src/components/Modal.js.

What happened:

    TypeError: Cannot read property 'parentElement' of null

      37 | 
      38 |     userEvent.click(screen.getByText("Close"));
    > 39 |     await waitForElementToBeRemoved(content);
         |           ^
      40 | });
      41 | 

      at node_modules/@testing-library/dom/dist/wait-for-element-to-be-removed.js:30:21
          at Array.map (<anonymous>)
      at waitForElementToBeRemoved (node_modules/@testing-library/dom/dist/wait-for-element-to-be-removed.js:27:43)
      at Object.<anonymous> (src/App.test.js:39:11)

const getRemainingElements = elements.map(element => {
let parent = element.parentElement
while (parent.parentElement) parent = parent.parentElement
return () => (parent.contains(element) ? element : null)
})

Reproduction:

https://codesandbox.io/s/amazing-brook-7xssv?file=/src/App.test.js

Problem description:

react-modal is removing the parentElement of the queried element. The queried element is given to waitForElementToBeRemoved which crashes while accessing the parentElement.

I'm sure there are work-arounds which can be used to avoid the crash caused by this minimal example. This is still a valid bug in the waitForElementToBeRemoved utility.

Suggested solution:

waitForElementToBeRemoved should not crash if given element has no .parentElement available. It should resolve successfully once missing parentElement is detected.

Debug information:

    const getRemainingElements = elements.map(element => {
      let parent = element.parentElement;

      debugger
      // element.outerHTML -> '<div>Content</div>'
      // element.parentElement -> null

      while (parent.parentElement) parent = parent.parentElement;
@marcosvega91
Copy link
Member

marcosvega91 commented Jan 11, 2021

Hi @AriPerkkio thanks for reaching us :).

The problem here is that the modal has been closed before calling waitForElementToBeRemoved.

What you suggested is right, we should check that the parentElement is present.

I think that we can simply add this check

diff --git a/src/wait-for-element-to-be-removed.js b/src/wait-for-element-to-be-removed.js
index d69974f..d70e5de 100644
--- a/src/wait-for-element-to-be-removed.js
+++ b/src/wait-for-element-to-be-removed.js
@@ -19,6 +19,7 @@ async function waitForElementToBeRemoved(callback, options) {
     initialCheck(callback)
     const elements = Array.isArray(callback) ? callback : [callback]
     const getRemainingElements = elements.map(element => {
+      if (!element.parentElement) return () => null
       let parent = element.parentElement
       while (parent.parentElement) parent = parent.parentElement
       return () => (parent.contains(element) ? element : null)

We should also add a new test case for it. Do you wanna work on a PR :)?

@AriPerkkio
Copy link
Contributor Author

Thanks for confirming this @marcosvega91. I can take a look at this later this week and setup PR.

@AriPerkkio
Copy link
Contributor Author

While fixing this I came up with better minimal repro which represents my real use case even better. Here the modal is closed automatically after a small delay. Depending on environment resources the element is either present or already removed from the parent. waitForElementToBeRemoved is not suitable for this kind of assertion. In practice we are seeing tests passing on all developer machines but randomly failing on slow CIs. My work-around was to replace it with await waitFor(() => expect(element).not.toBeInTheDocument());.

This cannot be reproduced in codesandbox (https://codesandbox.io/s/2vwmk?file=/src/App.test.js) but is easily reproduced in win10 or Debian with node 15. I guess the difference comes from setTimeout implementation between node and browsers.

import React, { useEffect, useReducer } from "react";
import Modal from "react-modal";
import userEvent from "@testing-library/user-event";
import {
  render,
  waitForElementToBeRemoved,
  screen,
  waitFor
} from "@testing-library/react";

// Adjust this value between 1-50 for the error to appear
// The element(s) given to waitForElementToBeRemoved are already removed. waitForElementToBeRemoved requires that the element(s) exist(s) before waiting for removal.
const TIMEOUT = 10;

// 100ms is enough for the error to not appear on fast environments
// OK
// const TIMEOUT = 100;

beforeAll(() => {
  const root = document.createElement("div");
  root.id = "app-root";
  document.body.appendChild(root);
  Modal.setAppElement("#app-root");
});

function Component() {
  const [isOpen, toggle] = useReducer((s) => !s, false);

  useEffect(() => {
    const timeoutHandle = setTimeout(toggle, TIMEOUT);
    return () => clearTimeout(timeoutHandle);
  }, []);

  return (
    <>
      <button onClick={toggle}>Open</button>

      <Modal isOpen={isOpen}>{isOpen && <div>Content</div>}</Modal>
    </>
  );
}

test("poc", async () => {
  render(<Component />);

  userEvent.click(screen.getByText("Open"));

  // Modal should close automatically
  const content = await screen.findByText("Content");
  await waitForElementToBeRemoved(content);

  // This works always
  //const content = await screen.findByText("Content");
  //await waitFor(() => expect(content).not.toBeInTheDocument());
});

I'm happy to close this issue once #871 is merged - so that correct error message is displayed. However I'm still quite unsure whether waitForElementToBeRemoved should expect given element to be present initially.

@github-actions
Copy link

🎉 This issue has been resolved in version 7.29.3 🎉

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
Development

Successfully merging a pull request may close this issue.

2 participants