Focus zone calls error in vitest + jsdom

mattcosta7 commented 1 year ago

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

:wave::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.

Primer/react uses jest (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

mattcosta7 commented 1 year ago

I did attempt to check whether we're polyfilling AbortSignal in this failure scenario and it appears we are not.

primer-react is not passing a signal, so the internally created controller is

github-actions[bot] commented 1 year ago

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.

primer / behaviors