Skip to content

Commit

Permalink
chore: replace node-fetch with undici
Browse files Browse the repository at this point in the history
There are several reasons to replace `node-fetch` with `undici`:

- `undici`'s `fetch()` implementation is set to become node's standard `fetch()` implementation, which means we can just remove the dependency in the future (or optionally load it depending on which version of node is being used)
- `node-fetch` pollutes the global type space with a number of standard types
- we already bundle `undici` via `miniflare`/pages, so this means our bundle size could ostensibly become smaller.

This replaces `node-fetch` with `undici`.

- All instances of `import fetch from "node-fetch"` are replaced with `import {fetch} from "undici"`
- `undici` also comes with spec compliant forms of `FormData` and `File`, so we could also remove `formdata-node` in `form_data.ts`
- All the global types that were injected by `node-fetch` are now imported from `undici` (as well as some mistaken ones from `node:url`)
- NOTE: this also turns on `skipLibCheck` in `tsconfig.json`. Some dependencies oddly depend on browser globals like `Request`, `Response` (like `@miniflare/core`, `jest-fetch-mock`, etc), which now fail because `node-fetch` isn't injecting those globals anymore. So we enable `skipLibCheck` to bypass them. (I'd thought `skipLibCheck` completely ignores 'third party' types, but that's not true - it still uses the module graph to scan types. So we're still typesafe. We should enable `strict` sometime to avoid `any`s, but that's for later.)
- The bundle size isn't smaller because we're bundling 2 different versions of `undici`, but we'll fix that by separately upping the version of `undici` that miniflare bundles.
  • Loading branch information
threepointone committed Jan 31, 2022
1 parent cfd8ba5 commit 870d1b2
Show file tree
Hide file tree
Showing 18 changed files with 76 additions and 208 deletions.
19 changes: 19 additions & 0 deletions .changeset/rare-eggs-bathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
"wrangler": patch
---

chore: replace `node-fetch` with `undici`

There are several reasons to replace `node-fetch` with `undici`:

- `undici`'s `fetch()` implementation is set to become node's standard `fetch()` implementation, which means we can just remove the dependency in the future (or optionally load it depending on which version of node is being used)
- `node-fetch` pollutes the global type space with a number of standard types
- we already bundle `undici` via `miniflare`/pages, so this means our bundle size could ostensibly become smaller.

This replaces `node-fetch` with `undici`.

- All instances of `import fetch from "node-fetch"` are replaced with `import {fetch} from "undici"`
- `undici` also comes with spec compliant forms of `FormData` and `File`, so we could also remove `formdata-node` in `form_data.ts`
- All the global types that were injected by `node-fetch` are now imported from `undici` (as well as some mistaken ones from `node:url`)
- NOTE: this also turns on `skipLibCheck` in `tsconfig.json`. Some dependencies oddly depend on browser globals like `Request`, `Response` (like `@miniflare/core`, `jest-fetch-mock`, etc), which now fail because `node-fetch` isn't injecting those globals anymore. So we enable `skipLibCheck` to bypass them. (I'd thought `skipLibCheck` completely ignores 'third party' types, but that's not true - it still uses the module graph to scan types. So we're still typesafe. We should enable `strict` sometime to avoid `any`s, but that's for later.)
- The bundle size isn't smaller because we're bundling 2 different versions of `undici`, but we'll fix that by separately upping the version of `undici` that miniflare bundles.
191 changes: 12 additions & 179 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
"root": true,
"ignorePatterns": [
"packages/*/vendor",
"packages/*/*-dist"
"packages/*/*-dist",
"packages/wrangler/pages/functions/template-worker.ts"
],
"parser": "@typescript-eslint/parser",
"parserOptions": {
Expand Down
6 changes: 2 additions & 4 deletions packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
"faye-websocket": "^0.11.4",
"finalhandler": "^1.1.2",
"find-up": "^6.2.0",
"formdata-node": "^4.3.1",
"ignore": "^5.2.0",
"ink": "^3.2.0",
"ink-select-input": "^4.2.1",
Expand All @@ -74,14 +73,13 @@
"ink-text-input": "^4.0.2",
"jest-fetch-mock": "^3.0.3",
"mime": "^3.0.0",
"node-fetch": "3.1.1",
"open": "^8.4.0",
"react": "^17.0.2",
"react-error-boundary": "^3.1.4",
"serve-static": "^1.14.1",
"signal-exit": "^3.0.6",
"tmp-promise": "^3.0.3",
"undici": "^4.11.1",
"undici": "4.13.0",
"ws": "^8.3.0",
"yargs": "^17.3.0"
},
Expand Down Expand Up @@ -111,7 +109,7 @@
"testTimeout": 30000,
"testRegex": ".*.(test|spec)\\.[jt]sx?$",
"transformIgnorePatterns": [
"node_modules/(?!node-fetch|fetch-blob|find-up|locate-path|p-locate|p-limit|yocto-queue|path-exists|data-uri-to-buffer|formdata-polyfill|execa|strip-final-newline|npm-run-path|path-key|onetime|mimic-fn|human-signals|is-stream)"
"node_modules/(?!find-up|locate-path|p-locate|p-limit|yocto-queue|path-exists|execa|strip-final-newline|npm-run-path|path-key|onetime|mimic-fn|human-signals|is-stream)"
],
"moduleNameMapper": {
"clipboardy": "<rootDir>/src/__tests__/clipboardy-mock.js"
Expand Down
7 changes: 6 additions & 1 deletion packages/wrangler/src/__tests__/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import { confirm, prompt } from "../dialogs";
import { fetchInternal } from "../cfetch/internal";
import fetchMock from "jest-fetch-mock";

jest.mock("node-fetch", () => jest.requireActual("jest-fetch-mock"));
jest.mock("undici", () => {
return {
...jest.requireActual("undici"),
fetch: jest.requireActual("jest-fetch-mock"),
};
});
fetchMock.doMock(() => {
// Any un-mocked fetches should throw
throw new Error("Unexpected fetch request");
Expand Down
Loading

0 comments on commit 870d1b2

Please sign in to comment.