Skip to content

Commit 52284fe

Browse files
feedthejimijjk
andauthored
misc: remove vendored node-fetch usages (#75741)
## What This PR replaces the usage of node-fetch in the next/font/google loader code, which runs as part of every next build. `node-fetch` relies on `punycode`, which shows a deprecration warning from Node.js 22, affecting every users on Next.js and Node 22. This PR also removes any other references to the vendored node-fetch used in other parts of the codebase. ~~Unfortunately, we can't remove it because other next.js dependencies like the amp optimiser actually use node-fetch, so they would need to be updated separately.~~ not a problem anymore because of @wowczarczyk suggestion of also rewriting the external in the dependencies to point to the correct one This PR does not remove the node-fetch usage in tests. ## How This replaces the node-fetch call with a call to the native fetch exposed in Node.js. However, this fetch does not support proxies without tapping into Undici, so I've had to "polyfill" a usage because I did not want to add a dependency to Undici for a minor use case. <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Adding or Updating Examples - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- --> Fixes #66289 Fixes #75313 Closes NDX-730 --------- Co-authored-by: JJ Kasper <[email protected]>
1 parent b76775a commit 52284fe

File tree

8 files changed

+92
-84
lines changed

8 files changed

+92
-84
lines changed
Lines changed: 12 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
// @ts-ignore
2-
import fetch from 'next/dist/compiled/node-fetch'
31
import { nextFontError } from '../next-font-error'
4-
import { getProxyAgent } from './get-proxy-agent'
2+
import { fetchResource } from './fetch-resource'
53
import { retry } from './retry'
64

75
/**
@@ -16,56 +14,23 @@ export async function fetchCSSFromGoogleFonts(
1614
fontFamily: string,
1715
isDev: boolean
1816
): Promise<string> {
19-
// Check if mocked responses are defined, if so use them instead of fetching from Google Fonts
20-
let mockedResponse: string | undefined
2117
if (process.env.NEXT_FONT_GOOGLE_MOCKED_RESPONSES) {
2218
const mockFile = require(process.env.NEXT_FONT_GOOGLE_MOCKED_RESPONSES)
23-
mockedResponse = mockFile[url]
19+
const mockedResponse = mockFile[url]
2420
if (!mockedResponse) {
2521
nextFontError('Missing mocked response for URL: ' + url)
2622
}
23+
return mockedResponse
2724
}
2825

29-
let cssResponse: string
30-
if (mockedResponse) {
31-
// Just use the mocked CSS if it's set
32-
cssResponse = mockedResponse
33-
} else {
34-
// Retry the fetch a few times in case of network issues as some font files
35-
// are quite large:
36-
// https://github.com/vercel/next.js/issues/45080
37-
cssResponse = await retry(async () => {
38-
const controller =
39-
isDev && typeof AbortController !== 'undefined'
40-
? new AbortController()
41-
: undefined
42-
const signal = controller?.signal
43-
const timeoutId = controller
44-
? setTimeout(() => controller.abort(), 3000)
45-
: undefined
26+
const buffer = await retry(async () => {
27+
return fetchResource(
28+
url,
29+
isDev,
30+
`Failed to fetch font \`${fontFamily}\`: ${url}\n` +
31+
`Please check your network connection.`
32+
)
33+
}, 3)
4634

47-
const res = await fetch(url, {
48-
agent: getProxyAgent(),
49-
// Add a timeout in dev
50-
signal,
51-
headers: {
52-
// The file format is based off of the user agent, make sure woff2 files are fetched
53-
'user-agent':
54-
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36',
55-
},
56-
}).finally(() => {
57-
timeoutId && clearTimeout(timeoutId)
58-
})
59-
60-
if (!res.ok) {
61-
nextFontError(
62-
`Failed to fetch font \`${fontFamily}\`.\nURL: ${url}\n\nPlease check if the network is available.`
63-
)
64-
}
65-
66-
return res.text()
67-
}, 3)
68-
}
69-
70-
return cssResponse
35+
return buffer.toString('utf8')
7136
}
Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,24 @@
1-
// @ts-ignore
2-
import fetch from 'next/dist/compiled/node-fetch'
3-
import { getProxyAgent } from './get-proxy-agent'
1+
import fs from 'node:fs'
42
import { retry } from './retry'
3+
import { fetchResource } from './fetch-resource'
54

65
/**
7-
* Fetch the url and return a buffer with the font file.
6+
* Fetches a font file and returns its contents as a Buffer.
7+
* If NEXT_FONT_GOOGLE_MOCKED_RESPONSES is set, we handle mock data logic.
88
*/
99
export async function fetchFontFile(url: string, isDev: boolean) {
10-
// Check if we're using mocked data
1110
if (process.env.NEXT_FONT_GOOGLE_MOCKED_RESPONSES) {
12-
// If it's an absolute path, read the file from the filesystem
1311
if (url.startsWith('/')) {
14-
return require('fs').readFileSync(url)
12+
return fs.readFileSync(url)
1513
}
16-
// Otherwise just return a unique buffer
1714
return Buffer.from(url)
1815
}
1916

2017
return await retry(async () => {
21-
const controller = new AbortController()
22-
const timeoutId = setTimeout(() => controller.abort(), 3000)
23-
const arrayBuffer = await fetch(url, {
24-
agent: getProxyAgent(),
25-
// Add a timeout in dev
26-
signal: isDev ? controller.signal : undefined,
27-
})
28-
.then((r: any) => r.arrayBuffer())
29-
.finally(() => {
30-
clearTimeout(timeoutId)
31-
})
32-
return Buffer.from(arrayBuffer)
18+
return fetchResource(
19+
url,
20+
isDev,
21+
`Failed to fetch font file from \`${url}\`.`
22+
)
3323
}, 3)
3424
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import http from 'node:http'
2+
import https from 'node:https'
3+
import { getProxyAgent } from './get-proxy-agent'
4+
5+
/**
6+
* Makes a simple GET request and returns the entire response as a Buffer.
7+
* - Throws if the response status is not 200.
8+
* - Applies a 3000 ms timeout when `isDev` is `true`.
9+
*/
10+
export function fetchResource(
11+
url: string,
12+
isDev: boolean,
13+
errorMessage?: string
14+
): Promise<Buffer> {
15+
return new Promise((resolve, reject) => {
16+
const { protocol } = new URL(url)
17+
const client = protocol === 'https:' ? https : http
18+
const timeout = isDev ? 3000 : undefined
19+
20+
const req = client.request(
21+
url,
22+
{
23+
agent: getProxyAgent(),
24+
headers: {
25+
// The file format is based off of the user agent, make sure woff2 files are fetched
26+
'User-Agent':
27+
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) ' +
28+
'AppleWebKit/537.36 (KHTML, like Gecko) ' +
29+
'Chrome/104.0.0.0 Safari/537.36',
30+
},
31+
},
32+
(res) => {
33+
if (res.statusCode !== 200) {
34+
reject(
35+
new Error(
36+
errorMessage ||
37+
`Request failed: ${url} (status: ${res.statusCode})`
38+
)
39+
)
40+
return
41+
}
42+
const chunks: Buffer[] = []
43+
res.on('data', (chunk) => chunks.push(Buffer.from(chunk)))
44+
res.on('end', () => resolve(Buffer.concat(chunks)))
45+
}
46+
)
47+
48+
if (timeout) {
49+
req.setTimeout(timeout, () => {
50+
req.destroy(new Error(`Request timed out after ${timeout}ms`))
51+
})
52+
}
53+
54+
req.on('error', (err) => reject(err))
55+
req.end()
56+
})
57+
}

packages/font/src/google/loader.test.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import nextFontGoogleFontLoader from './loader'
2-
// @ts-ignore
3-
import fetch from 'next/dist/compiled/node-fetch'
2+
import { fetchResource } from './fetch-resource'
43

5-
jest.mock('next/dist/compiled/node-fetch')
4+
jest.mock('./fetch-resource')
5+
6+
const mockFetchResource = fetchResource as jest.Mock
67

78
describe('next/font/google loader', () => {
89
afterEach(() => {
@@ -120,10 +121,7 @@ describe('next/font/google loader', () => {
120121
fontFunctionArguments: any,
121122
expectedUrl: any
122123
) => {
123-
fetch.mockResolvedValue({
124-
ok: true,
125-
text: async () => 'OK',
126-
})
124+
mockFetchResource.mockResolvedValue(Buffer.from('OK'))
127125
const { css } = await nextFontGoogleFontLoader({
128126
functionName,
129127
data: [
@@ -141,8 +139,12 @@ describe('next/font/google loader', () => {
141139
variableName: 'myFont',
142140
})
143141
expect(css).toBe('OK')
144-
expect(fetch).toHaveBeenCalledTimes(1)
145-
expect(fetch).toHaveBeenCalledWith(expectedUrl, expect.any(Object))
142+
expect(mockFetchResource).toHaveBeenCalledTimes(1)
143+
expect(mockFetchResource).toHaveBeenCalledWith(
144+
expectedUrl,
145+
false,
146+
expect.stringContaining('Failed to fetch font')
147+
)
146148
}
147149
)
148150
})

packages/next/src/compiled/node-fetch/index.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/next/src/trace/trace-uploader.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import findUp from 'next/dist/compiled/find-up'
22
import fsPromise from 'fs/promises'
33
import child_process from 'child_process'
44
import assert from 'assert'
5-
import fetch from 'next/dist/compiled/node-fetch'
65
import os from 'os'
76
import { createInterface } from 'readline'
87
import { createReadStream } from 'fs'

packages/next/taskfile.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ const externals = {
7777
'terser-webpack-plugin':
7878
'next/dist/build/webpack/plugins/terser-webpack-plugin/src',
7979

80+
punycode: 'punycode/',
8081
// TODO: Add @swc/helpers to externals once @vercel/ncc switch to swc-loader
8182
}
8283
// eslint-disable-next-line camelcase

packages/next/types/$$compiled.internal.d.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -351,12 +351,6 @@ declare module 'next/dist/compiled/@next/react-refresh-utils/dist/ReactRefreshWe
351351
export = m
352352
}
353353

354-
declare module 'next/dist/compiled/node-fetch' {
355-
import fetch from 'node-fetch'
356-
export * from 'node-fetch'
357-
export default fetch
358-
}
359-
360354
declare module 'next/dist/compiled/commander' {
361355
import commander from 'commander'
362356
export * from 'commander'

0 commit comments

Comments
 (0)