icssc / AntAlmanac

A course exploration and scheduling tool for UCI Anteaters
https://antalmanac.com
MIT License
54 stars 64 forks source link

Schedule test failing #827

Closed EricPedley closed 9 months ago

EricPedley commented 9 months ago

Basically, window.matchMedia is a real function but jsdom doesn't have it. I found an official way to fix this in jest, now we just have to translate it to vitest: https://stackoverflow.com/questions/39830580/jest-test-fails-typeerror-window-matchmedia-is-not-a-function image

ap0nia commented 9 months ago

I was looking at this, and I honestly don't really like the way it has to be implemented. I think the best way to do it would be something like this.

import { beforeAll } from 'vitest'

beforeAll(() => {
  const originalMatchMedia = window.matchMedia
  window.matchMedia = { ... }
  return () => {
    window.matchMedia = originalMatchMedia
  }
})

We define a custom version of match media before all the tests, and then clean it up once all the tests are done. I don't really like the strat of "permanently" appending to the global object without ever cleaning it up. It doesn't have any observable or tangible effects; it just feels weird.

However, this alternative approach doesn't work for some reason?? So my proposed solution would just be to make an SSR-safe version of the function.

function getPreferredTheme() {
  if (typeof window.matchMedia === 'undefined') {
    return 'light'
  }
  return window.matchMedia('prefers-color-scheme: dark').matches ? 'dark' : 'light'
}

I honestly think it's good practice to think in terms of SSR even if the app isn't using it, because it's pretty applicable in many modern frameworks. It's good to think about whether the code can run in the server or not, and ensure that it doesn't break if it a framework attempts to render it on the server.

KevinWu098 commented 9 months ago

Basically, window.matchMedia is a real function but jsdom doesn't have it. I found an official way to fix this in jest, now we just have to translate it to vitest: https://stackoverflow.com/questions/39830580/jest-test-fails-typeerror-window-matchmedia-is-not-a-function image

I like Brian's solution above, but I'll note here that the Vitest version is identical to the Jest one:

import { vi } from 'vitest';

Object.defineProperty(window, 'matchMedia', {
    writable: true,
    value: vi.fn().mockImplementation((query) => ({
        matches: false,
        media: query,
        onchange: null,
        addListener: vi.fn(), // deprecated
        removeListener: vi.fn(), // deprecated
        addEventListener: vi.fn(),
        removeEventListener: vi.fn(),
        dispatchEvent: vi.fn(),
    })),
});

Do we want to implement both for future tests? This would just be a setup.ts file that contains the matchMedia function

EricPedley commented 9 months ago

Do we want to implement both for future tests? This would just be a setup.ts file that contains the matchMedia function

Yeah, there are a bunch of places we use matchMedia in our codebase.

KevinWu098 commented 9 months ago

Cool, will do