sindresorhus / ky

🌳 Tiny & elegant JavaScript HTTP client based on the Fetch API
MIT License
11.91k stars 342 forks source link

TypeError: signal.throwIfAborted is not a function #548

Closed darrinmn9 closed 6 months ago

darrinmn9 commented 7 months ago

Starting from ky v1.0.0, I observe TypeError: signal.throwIfAborted is not a function when running in a node environment (versions 18.16.1 and 18.19.0), running tests in jest. v0.33.0 is not affected as the issue appears to introduced from this commit: https://github.com/sindresorhus/ky/commit/a8a3a269d2e260999b06105ede3f1cef668f0cf2#diff-5f5888d08c3c4e7b3d66c3b672b62f62c3123a4237d5e1c6fb7bbdc34e7a6cbcR15

TypeError: signal.throwIfAborted is not a function
    at /node_modules/ky/source/utils/delay.ts:25:4
    at new Promise (<anonymous>)
    at delay (/node_modules/ky/source/utils/delay.ts:22:4)
    at Ky._retry (/node_modules/ky/source/core/Ky.ts:273:7)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at Promise.result.<computed> [as json] (/node_modules/ky/source/core/Ky.ts:102:7)
sindresorhus commented 7 months ago

Sounds like you're running it in an older Node.js version than you think you are. throwIfAborted is definitely available on Node.js 18.

darrinmn9 commented 7 months ago

hmm 🤔 , in the jest output right after this error stack trace it says Node.js v18.19.0 (jest version 29.7.0). This also happens in both my local environment and CI (where i am confident only 1 version of node.js is ever installed). Maybe i can try different jest versions and see if maybe its an issue/regression related to my specific jest testing environment.

darrinmn9 commented 7 months ago

ahhh silly me, I should have suspected the issue was with the jsdom environment. (jsdom doesnt currently support the global Fetch api).

Reference to issue and suggested workarounds for jsdom fetch support for others who may come across this: https://github.com/jsdom/jsdom/issues/1724

marc-at-brightnight commented 7 months ago

ahhh silly me, I should have suspected the issue was with the jsdom environment. (jsdom doesnt currently support the global Fetch api).

Reference to issue and suggested workarounds for jsdom fetch support for others who may come across this: jsdom/jsdom#1724

anyone still have an issue even after implementing this? I added the AbortController as well, as suggested by @darrinmn9 but no luck. I still get the error when running tests.

This is blocking me from upgrading my ky to v1, any help is appreciated.

Using node 18.12.1. Below is my jest.config.ts

import type { Config } from 'jest';

const config: Config = {
  testEnvironment: './FixJSDOMEnvironment.ts',
};

export default config;
darrinmn9 commented 7 months ago

@marc-at-brightnight what is the exact error + stack trace you are getting, and what does ur FixJSDOMEnvironment.ts file look like?

marc-at-brightnight commented 7 months ago

@marc-at-brightnight what is the exact error + stack trace you are getting, and what does ur FixJSDOMEnvironment.ts file look like?

----stack trace-----
/Users/mnhmbp/PycharmProjects/my-app/node_modules/ky/distribution/utils/delay.js:13
      signal.throwIfAborted();

TypeError: signal.throwIfAborted is not a function
    at throwIfAborted (/Users/mnhmbp/PycharmProjects/my-app/node_modules/ky/source/utils/delay.ts:15:11)
    at new Promise (<anonymous>)
    at delay (/Users/mnhmbp/PycharmProjects/my-app/node_modules/ky/source/utils/delay.ts:13:9)
    at Ky._retry (/Users/mnhmbp/PycharmProjects/my-app/node_modules/ky/source/core/Ky.ts:259:16)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at Promise.result.<computed> [as json] (/Users/mnhmbp/PycharmProjects/my-app/node_modules/ky/source/core/Ky.ts:86:27)
// FixJSDOMEnvironment.ts

import JSDOMEnvironment from 'jest-environment-jsdom';

// https://github.com/facebook/jest/blob/v29.4.3/website/versioned_docs/version-29.4/Configuration.md#testenvironment-string
export default class FixJSDOMEnvironment extends JSDOMEnvironment {
  constructor(...args: ConstructorParameters<typeof JSDOMEnvironment>) {
    super(...args);

    // FIXME https://github.com/jsdom/jsdom/issues/1724
    this.global.fetch = fetch;
    this.global.Headers = Headers;
    this.global.Request = Request;
    this.global.Response = Response;
    this.global.AbortController = AbortController;
  }
}
// package.json

"dependencies: {
  "ky": "^1.1.3",
  "jest-environment-jsdom": "^29.7.0"
  ...
}

Appreciate you taking a look!

darrinmn9 commented 7 months ago

hmm... not sure, looks just like mine. (additionally i have this.global.FormData = FormData;, but that doesnt seem like ur issue here)

sholladay commented 6 months ago

I would recommend just running your tests in an actual browser with something like Playwright, rather than trying to emulate one with something like JSDOM.

JSDOM was designed to facilitate simple DOM based unit tests, nothing more. And it made sense at the time because browser automation tools barely existed and were cumbersome. But that's not the case anymore.

Check out how Ky's own tests are implemented. We do run many tests in Node, because they don't necessarily need a DOM or web API polyfills, especially in recent versions of Node. But for anything complicated, or where the test was really critical, I would put it in our browser tests. The boilerplate and performance overhead is minimal.

darrinmn9 commented 6 months ago

totally agree! I am just trying to upgrade a large, existing jest test suite. But yes the longterm goal is to get off JSDOM... because as you've mentioned we have better solutions in 2024.