Skip to content

fix: remove use of node-fetch, use built in fetch #714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2,928 changes: 2,552 additions & 376 deletions package-lock.json

Large diffs are not rendered by default.

7 changes: 0 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,9 @@
"@types/jest": "^29.5.2",
"chalk": "^4.1.2",
"inquirer": "^8.2.3",
"node-fetch": "^2.x",
"ora": "^5.4.1",
"semver": "^7.5.2"
},
"overrides": {
"node-fetch@^2.x": {
"whatwg-url": "14.x"
}
},
"devDependencies": {
"@adobe/eslint-config-aio-lib-config": "^4.0.0",
"acorn": "^8.8.2",
Expand All @@ -52,7 +46,6 @@
"eslint-plugin-promise": "^6.6.0",
"execa": "^5.1.1",
"jest": "^29.0.1",
"jest-fetch-mock": "^3.0.0",
"jest-junit": "^16.0.0",
"jest-plugin-fs": "^2.9.0",
"oclif": "^4",
Expand Down
1 change: 0 additions & 1 deletion src/commands/discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
*/

const { Command, Flags, ux } = require('@oclif/core')
const fetch = require('node-fetch')
const inquirer = require('inquirer')
const { sortValues } = require('../helpers')

Expand Down
1 change: 0 additions & 1 deletion src/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
* governing permissions and limitations under the License.
*/

const fetch = require('node-fetch')
const fs = require('fs')
const inquirer = require('inquirer')

Expand Down
3 changes: 3 additions & 0 deletions test/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
{
"globals": {
"setFetchMock": true
},
"rules": {
"node/no-unpublished-require": 0
}
Expand Down
18 changes: 9 additions & 9 deletions test/commands/discover.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
* governing permissions and limitations under the License.
*/

const fetch = require('node-fetch')
const inquirer = require('inquirer')
const TheCommand = require('../../src/commands/discover')
const { stdout } = require('stdout-stderr')
Expand All @@ -20,7 +19,6 @@ jest.mock('inquirer')
let command

beforeEach(() => {
fetch.resetMocks()
command = new TheCommand([])
command.config = {
commands: [{ pluginName: 'baz' }],
Expand All @@ -44,13 +42,14 @@ describe('sorting', () => {
]
}
beforeEach(() => {
fetch.mockResponseOnce(JSON.stringify(expectedResult))
setFetchMock(true, expectedResult)
})

test('unknown sort-field', async () => {
fetch.mockResponseOnce(JSON.stringify({
setFetchMock(true, {
objects: []
}))
})

command.argv = ['--sort-field', 'unknown']
await expect(command.run()).rejects.toThrow('Expected --sort-field=')
})
Expand Down Expand Up @@ -100,7 +99,7 @@ test('interactive install', async () => {
{ package: { scope: 'adobe', name: 'baz', description: 'some baz', version: '1.0.2', date: dayAfter } }
]
}
fetch.mockResponseOnce(JSON.stringify(expectedResult))
setFetchMock(true, expectedResult)

command.argv = ['-i']
inquirer.prompt = jest.fn().mockResolvedValue({
Expand All @@ -121,7 +120,7 @@ test('interactive install - no choices', async () => {
{ package: { scope: 'adobe', name: 'baz', description: 'some baz', version: '1.0.2', date: now } }
]
}
fetch.mockResponseOnce(JSON.stringify(expectedResult))
setFetchMock(true, expectedResult)

command.argv = ['-i']
inquirer.prompt = jest.fn().mockResolvedValue({
Expand All @@ -132,7 +131,8 @@ test('interactive install - no choices', async () => {
})

test('json result error', async () => {
fetch.mockResponse()
setFetchMock(false, 'not json')

command.argv = []
await expect(command.run()).rejects.toThrow('FetchError: invalid json response body')
await expect(command.run()).rejects.toThrow()
Copy link
Preview

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This assertion is very broad and may pass for unexpected errors; consider tightening it to match a specific error message or error type.

Suggested change
await expect(command.run()).rejects.toThrow()
await expect(command.run()).rejects.toThrowError('Invalid JSON response')

Copilot uses AI. Check for mistakes.

})
2 changes: 0 additions & 2 deletions test/commands/rollback.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
* governing permissions and limitations under the License.
*/

const fetch = require('node-fetch')
const inquirer = require('inquirer')
const { stdout } = require('stdout-stderr')
const helpers = require('../../src/helpers')
Expand All @@ -30,7 +29,6 @@ let command

beforeEach(() => {
jest.clearAllMocks()
fetch.resetMocks()
command = new TheCommand([])
command.config = {
commands: [{ pluginName: 'baz' }],
Expand Down
2 changes: 0 additions & 2 deletions test/commands/update.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
* governing permissions and limitations under the License.
*/

const fetch = require('node-fetch')
const inquirer = require('inquirer')
const helpers = require('../../src/helpers')
const { stdout } = require('stdout-stderr')
Expand All @@ -30,7 +29,6 @@ let command

beforeEach(() => {
jest.clearAllMocks()
fetch.resetMocks()
command = new TheCommand([])
command.config = {
commands: [],
Expand Down
4 changes: 1 addition & 3 deletions test/helpers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
* governing permissions and limitations under the License.
*/

const fetch = require('node-fetch')
const { prompt, sortValues, getNpmLatestVersion, getNpmLocalVersion, hideNPMWarnings } = require('../src/helpers')
const inquirer = require('inquirer')
const { stderr } = require('stdout-stderr')
Expand All @@ -20,7 +19,6 @@ jest.mock('fs')
jest.mock('inquirer')

beforeEach(() => {
fetch.resetMocks()
fs.readFileSync.mockRestore()
})

Expand Down Expand Up @@ -205,7 +203,7 @@ test('getNpmLatestVersion', async () => {
}
}

fetch.mockResponseOnce(JSON.stringify(json))
setFetchMock(true, json)
return expect(getNpmLatestVersion('foo')).resolves.toEqual(json['dist-tags'].latest)
})

Expand Down
8 changes: 6 additions & 2 deletions test/jest.setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ const { stdout } = require('stdout-stderr')
jest.setTimeout(3000)
jest.useFakeTimers()

Copy link
Preview

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding an afterEach hook in this setup file to delete or restore global.fetch so that mocks don’t leak between tests.

Suggested change
const originalFetch = global.fetch;

Copilot uses AI. Check for mistakes.

const fetch = require('jest-fetch-mock')
jest.setMock('node-fetch', fetch)
global.setFetchMock = (ok = true, mockData = {}) => {
global.fetch = jest.fn().mockResolvedValue({
ok,
json: () => ok ? Promise.resolve(mockData) : Promise.reject(mockData)
})
}

// trap console log
// note: if you use console.log, some of these tests will start failing because they depend on the order/position of the output
Expand Down
Loading