Skip to content

Commit

Permalink
chore: replace node-fetch by undici hook package (verdaccio#2292)
Browse files Browse the repository at this point in the history
* chore: replace node-fetch by undici hook package

* fix types

* test something

* test 12

* add flag

* restore fail fast

* remove 12 from tests
  • Loading branch information
juanpicado authored Jun 12, 2021
1 parent 2197d08 commit d6e44a4
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 54 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ jobs:
fail-fast: true
matrix:
os: [ubuntu-latest]
node_version: [12, 14]
node_version: [14]
name: ${{ matrix.os }} / Node ${{ matrix.node_version }}
runs-on: ${{ matrix.os }}
steps:
Expand Down
4 changes: 3 additions & 1 deletion packages/hooks/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const config = require('../../jest/config');

module.exports = Object.assign({}, config, {});
module.exports = Object.assign({}, config, {
testEnvironment: 'node',
});
9 changes: 4 additions & 5 deletions packages/hooks/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,22 @@
"verdaccio"
],
"engines": {
"node": ">=10",
"node": ">=12",
"npm": ">=6"
},
"dependencies": {
"@verdaccio/commons-api": "workspace:11.0.0-alpha.3",
"@verdaccio/logger": "workspace:6.0.0-6-next.4",
"debug": "^4.2.0",
"handlebars": "4.5.3",
"node-fetch": "^2.6.1",
"request": "2.87.0"
"undici": "^4.0.0-rc.1",
"undici-fetch": "^1.0.0-rc.1"
},
"devDependencies": {
"@verdaccio/auth": "workspace:6.0.0-6-next.9",
"@verdaccio/commons-api": "workspace:11.0.0-alpha.3",
"@verdaccio/config": "workspace:6.0.0-6-next.7",
"@verdaccio/types": "workspace:11.0.0-6-next.7",
"nock": "^13.0.4"
"@verdaccio/types": "workspace:11.0.0-6-next.7"
},
"scripts": {
"clean": "rimraf ./build",
Expand Down
11 changes: 8 additions & 3 deletions packages/hooks/src/notify-request.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
import fetch, { RequestInit } from 'node-fetch';
import buildDebug from 'debug';

import { logger } from '@verdaccio/logger';
import { HTTP_STATUS } from '@verdaccio/commons-api';

const debug = buildDebug('verdaccio:hooks:request');
export type NotifyRequestOptions = RequestInit;
const fetch = require('undici-fetch');

export async function notifyRequest(url: string, options: NotifyRequestOptions): Promise<boolean> {
export type FetchOptions = {
body: string;
headers?: {};
method?: string;
};

export async function notifyRequest(url: string, options: FetchOptions): Promise<boolean> {
let response;
try {
debug('uri %o', url);
Expand Down
4 changes: 2 additions & 2 deletions packages/hooks/src/notify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import Handlebars from 'handlebars';
import buildDebug from 'debug';
import { Config, Package, RemoteUser, Notification } from '@verdaccio/types';
import { logger } from '@verdaccio/logger';
import { notifyRequest, NotifyRequestOptions } from './notify-request';
import { notifyRequest, FetchOptions } from './notify-request';

const debug = buildDebug('verdaccio:hooks');

Expand Down Expand Up @@ -50,7 +50,7 @@ export async function handleNotify(
content = await compileTemplate(notifyEntry.content, metadata);
}

const options: NotifyRequestOptions = {
const options: FetchOptions = {
body: JSON.stringify(content),
};

Expand Down
55 changes: 29 additions & 26 deletions packages/hooks/test/notify-request.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import nock from 'nock';
import { setup } from '@verdaccio/logger';
import { Config } from '@verdaccio/types';
import { parseConfigFile, createRemoteUser } from '@verdaccio/config';
import { notify } from '../src/notify';
Expand All @@ -12,36 +12,33 @@ const singleHeaderNotificationConfig = parseConfigFile(
);
const multiNotificationConfig = parseConfigFile(parseConfigurationNotifyFile('multiple.notify'));

const mockInfo = jest.fn();
jest.mock('@verdaccio/logger', () => ({
setup: jest.fn(),
logger: {
child: jest.fn(),
debug: jest.fn(),
trace: jest.fn(),
warn: jest.fn(),
info: () => mockInfo(),
error: jest.fn(),
fatal: jest.fn(),
},
}));
setup([]);

const domain = 'http://slack-service';
const { MockAgent } = require('undici');
const { setGlobalDispatcher } = require('undici-fetch');

describe('Notifications:: notifyRequest', () => {
beforeEach(() => {
nock.cleanAll();
});
const options = {
path: '/foo?auth_token=mySecretToken',
method: 'POST',
};

describe('Notifications:: notifyRequest', () => {
test('when sending a empty notification', async () => {
nock(domain).post('/foo?auth_token=mySecretToken').reply(200, { body: 'test' });
const mockAgent = new MockAgent({ connections: 1 });
setGlobalDispatcher(mockAgent);
const mockClient = mockAgent.get(domain);
mockClient.intercept(options).reply(200, { body: 'test' });

const notificationResponse = await notify({}, {}, createRemoteUser('foo', []), 'bar');
expect(notificationResponse).toEqual([false]);
});

test('when sending a single notification', async () => {
nock(domain).post('/foo?auth_token=mySecretToken').reply(200, { body: 'test' });
const mockAgent = new MockAgent({ connections: 1 });
setGlobalDispatcher(mockAgent);
const mockClient = mockAgent.get(domain);
mockClient.intercept(options).reply(200, { body: 'test' });

const notificationResponse = await notify(
{},
Expand All @@ -50,10 +47,14 @@ describe('Notifications:: notifyRequest', () => {
'bar'
);
expect(notificationResponse).toEqual([true]);
await mockClient.close();
});

test('when notification endpoint is missing', async () => {
nock(domain).post('/foo?auth_token=mySecretToken').reply(200, { body: 'test' });
const mockAgent = new MockAgent({ connections: 1 });
setGlobalDispatcher(mockAgent);
const mockClient = mockAgent.get(domain);
mockClient.intercept(options).reply(200, { body: 'test' });
const name = 'package';
const config: Partial<Config> = {
// @ts-ignore
Expand All @@ -68,14 +69,16 @@ describe('Notifications:: notifyRequest', () => {
});

test('when multiple notifications', async () => {
nock(domain).post('/foo?auth_token=mySecretToken').reply(200, { body: 'test' });
nock(domain).post('/foo?auth_token=mySecretToken').reply(400, {});
nock(domain)
.post('/foo?auth_token=mySecretToken')
.reply(500, { message: 'Something bad happened' });
const mockAgent = new MockAgent({ connections: 1 });
setGlobalDispatcher(mockAgent);
const mockClient = mockAgent.get(domain);
mockClient.intercept(options).reply(200, { body: 'test' });
mockClient.intercept(options).reply(400, {});
mockClient.intercept(options).reply(500, { message: 'Something bad happened' });

const name = 'package';
const responses = await notify({ name }, multiNotificationConfig, { name: 'foo' }, 'bar');
expect(responses).toEqual([true, false, false]);
await mockClient.close();
});
});
31 changes: 15 additions & 16 deletions pnpm-lock.yaml

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

0 comments on commit d6e44a4

Please sign in to comment.