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

Autocomplete breaks in tests run with jsdom and vitest #2454

Closed
mattcosta7 opened this issue Oct 19, 2022 · 4 comments
Closed

Autocomplete breaks in tests run with jsdom and vitest #2454

mattcosta7 opened this issue Oct 19, 2022 · 4 comments
Assignees

Comments

@mattcosta7
Copy link
Collaborator

mattcosta7 commented Oct 19, 2022

Description

This issue may better exist on primer/behaviors (or even upstreamed to jsdom if we can repro simply enough)

Initially reported in slack by @itsbagpack
https://github.slack.com/archives/C01L618AEP9/p1666186967969609

👋:skin-tone-2: hi friends, i’m running into some testing issues around using the Autocomplete component and i was wondering what we were missing
for context, here’s the PR we’re working with: https://github.com/github/delorean/pull/127

TypeError: Failed to execute 'addEventListener' on 'EventTarget': parameter 3 dictionary has member 'signal' that is not of type 'AbortSignal'.
❯ exports.convert node_modules/jsdom/lib/jsdom/living/generated/AddEventListenerOptions.js:53:11
❯ HTMLDivElement.addEventListener node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:139:46
❯ Module.focusZone node_modules/@primer/behaviors/dist/esm/focus-zone.js:248:15
   246|     });
   247|     let elementIndexFocusedByClick = undefined;
   248|     container.addEventListener('mousedown', event => {
      |               ^
   249|         if (event.target instanceof HTMLElement && event.target !== document.activeElement) {
   250|             elementIndexFocusedByClick = focusableElements.indexOf(event.target);
❯ node_modules/@primer/react/lib-esm/hooks/useFocusZone.js:21:34

It appears that jsdom is throwing when trying to add an event listener, and appears that's due to the primer/behaviors focusZone attempting to pass a signal, which appears to conflict with the accepted signal jsdom expects. I added

Primer/react uses JSDOM (with jsdom) to test this component, which is confusing to me, given there's no indication that it currently has issues here

This leads me to think either
a version of JSDOM begins breaking
vite and jest somehow differ in the globals they send to jsdom in this scenario

I suggested a workaround, temporarily, mocking focusZone entirely -

vi.mock("@primer/behaviors", async () => {
 const behaviors = await vi.importActual<typeof import("@primer/behaviors")>(
   "@primer/behaviors",
 )

 return {
   ...behaviors,
   focusZone: vi.fn(),
 }
})

but this changes behavior of the autocomplete, and is not ideal

Steps to reproduce

start a project with vite/vitest
render an AutoComplete in a test using the jsdom environment

see jsdom throw

Version

All related versions of multiple packages

It's unclear to me whether this problem already exists and is becoming clear due to versioning of react and testing-libary, is due to something with how vite interops globals compared to jest, or if this is jest/jsdom.

"dependencies": {
    "@primer/octicons-react": "^17.7.0",
    "@primer/react": "^35.11.0-rc.65a8bb8a",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "styled-components": "^5.3.5"
  },
  "devDependencies": {
    "@testing-library/jest-dom": "^5.16.5",
    "@testing-library/react": "^13.4.0",
    "@testing-library/user-event": "^14.4.3",
    "@types/react": "^18.0.17",
    "@types/react-dom": "^18.0.6",
    "@vitejs/plugin-react": "^2.0.1",
    "typescript": "^4.6.4",
    "vite": "^3.0.7",
    "vitest": "^0.23.4"
  }

Browser

No response

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2023

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label Oct 8, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 15, 2023
@mattcosta7 mattcosta7 reopened this Oct 16, 2023
@mattcosta7 mattcosta7 removed the Stale label Oct 16, 2023
Copy link
Contributor

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label Apr 13, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 20, 2024
@github-actions github-actions bot removed the Stale label Apr 22, 2024
@keithamus
Copy link
Member

This is likely due to using an old version of jsdom. jsdom@20.0.0 is the first version of the lib to add support for signal in addeventlistener. Looks like the monolith and Delorean now use a version > 20, so I'll close this.

If in doubt, in future, please update to the latest version of jsdom as it'll likely fix issues like this.

@mattcosta7
Copy link
Collaborator Author

This is likely due to using an old version of jsdom. jsdom@20.0.0 is the first version of the lib to add support for signal in addeventlistener. Looks like the monolith and Delorean now use a version > 20, so I'll close this.

If in doubt, in future, please update to the latest version of jsdom as it'll likely fix issues like this.

makese sense. IIRC that wasn't an option at the time we opened this

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

No branches or pull requests

5 participants