Skip to content
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

[SDK-4485] Use native fetch, drop Node 16 support #906

Merged
merged 7 commits into from
Aug 16, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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 changes: 1 addition & 1 deletion playground/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ export async function jobs() {
const ids = [];

const { data: createImportJob } = await mgmntClient.jobs.importUsers({
users: fs.createReadStream(usersFilePath),
users: new Blob([fs.readFileSync(usersFilePath)], { type: 'application/json' }),
connection_id: connection.id as string,
});

Expand Down
20 changes: 20 additions & 0 deletions src/lib/fetch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { FetchAPI } from './models.js';

/**
* @private
*/
export const fetch = (...args: Parameters<FetchAPI>) =>
(globalThis.fetch && globalThis.fetch(...args)) ||
import('node-fetch').then(({ default: fetch }) => (fetch as FetchAPI)(...args));

/**
* @private
*/
export const getFormDataCls = async () =>
globalThis.FormData || import('node-fetch').then(({ FormData }) => FormData);

/**
* @private
*/
export const getBlobCls = async () =>
globalThis.Blob || import('node-fetch').then(({ Blob }) => Blob);
28 changes: 5 additions & 23 deletions src/lib/runtime.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { ReadStream } from 'fs';
import { retry } from './retry.js';
import { FetchError, RequiredError, TimeoutError } from './errors.js';
import {
Expand All @@ -9,19 +8,10 @@ import {
Middleware,
FetchAPI,
} from './models.js';

/**
* @private
*/
const nodeFetch = (...args: Parameters<FetchAPI>) =>
import('node-fetch').then(({ default: fetch }) => (fetch as FetchAPI)(...args));

/**
* @private
*/
export const getFormDataCls = async () => import('node-fetch').then(({ FormData }) => FormData);
import { fetch as fetchApi, getFormDataCls, getBlobCls } from './fetch.js';

export * from './models.js';
export { getFormDataCls };

/**
* @private
Expand All @@ -43,7 +33,7 @@ export class BaseAPI {
}

this.middleware = configuration.middleware || [];
this.fetchApi = configuration.fetchApi || nodeFetch;
this.fetchApi = configuration.fetchApi || fetchApi;
this.parseError = configuration.parseError;
this.timeoutDuration =
typeof configuration.timeoutDuration === 'number' ? configuration.timeoutDuration : 10000;
Expand Down Expand Up @@ -96,7 +86,7 @@ export class BaseAPI {
})),
};

const { Blob } = await import('node-fetch');
const Blob = await getBlobCls();
const init: RequestInit = {
...overriddenInit,
body:
Expand Down Expand Up @@ -312,17 +302,9 @@ export function applyQueryParams<
* @private
*/
export async function parseFormParam(
originalValue: number | boolean | string | Blob | ReadStream
originalValue: number | boolean | string | Blob
): Promise<string | Blob> {
let value = originalValue;
value = typeof value == 'number' || typeof value == 'boolean' ? '' + value : value;
if (
typeof originalValue === 'object' &&
'path' in originalValue &&
typeof originalValue.path === 'string'
) {
const { fileFrom } = await import('node-fetch');
value = await fileFrom(originalValue.path, 'application/json');
}
return value as string | Blob;
}
4 changes: 1 addition & 3 deletions src/management/__generated/models/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import type { ReadStream } from 'fs';

/**
*
*/
Expand Down Expand Up @@ -11963,7 +11961,7 @@ export interface GetJobsByIdRequest {
export interface PostUsersImportsData {
/**
*/
users: Blob | ReadStream;
users: Blob;
/**
* connection_id of the connection to which users will be imported.
*
Expand Down
89 changes: 44 additions & 45 deletions test/management/jobs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '../../src/index.js';
import { extractParts } from '../utils/index.js';
import { fileURLToPath } from 'url';
import { getBlobCls } from '../../src/lib/fetch.js';

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
Expand Down Expand Up @@ -174,9 +175,10 @@ describe('JobsManager', () => {
let data: PostUsersImportsData;
let request: nock.Scope;

beforeEach(() => {
beforeEach(async () => {
const Blob = await getBlobCls();
data = {
users: fs.createReadStream(usersFilePath) as any,
users: new Blob([fs.readFileSync(usersFilePath)], { type: 'application/json' }),
connection_id: 'con_test',
};
request = nock(API_URL).post('/jobs/users-imports').reply(200, {});
Expand Down Expand Up @@ -384,15 +386,16 @@ describe('JobsManager', () => {
describe('#importUsers with JSON data', () => {
let data: PostUsersImportsData;

beforeEach(() => {
beforeEach(async () => {
const Blob = await getBlobCls();
data = {
users: fs.createReadStream(usersFilePath) as any,
users: new Blob([fs.readFileSync(usersFilePath)], { type: 'application/json' }),
connection_id: 'con_test',
};
nock(API_URL).post('/jobs/users-imports').reply(200, {});
});

it('should correctly include user JSON from ReadStream', (done) => {
it('should correctly include user JSON from Blob', (done) => {
nock.cleanAll();
let boundary: string | null = null;

Expand Down Expand Up @@ -423,46 +426,42 @@ describe('JobsManager', () => {
});
});

(typeof Blob === 'undefined' ? it.skip : it)(
'should correctly include user JSON from Blob',
function (done) {
nock.cleanAll();
let boundary: string | null = null;

const request = nock(API_URL)
.matchHeader('Content-Type', (header) => {
boundary = `--${header.match(/boundary=([^\n]*)/)?.[1]}`;

return true;
})
.post('/jobs/users-imports', (body) => {
const parts = extractParts(body, boundary);

expect(parts.users).toContain('Content-Type: application/json');

// Validate the content of the users JSON.
const users = JSON.parse(parts.users.split('\r\n').slice(-1)[0]);
expect(users.length).toBe(2);
expect(users[0].email).toBe('[email protected]');

return true;
})
.reply(200, {});

jobs
.importUsers({
...data,
users: new Blob([fs.readFileSync(usersFilePath).toString()], {
type: 'application/json',
}),
})
.then(() => {
expect(request.isDone()).toBe(true);

done();
});
}
);
it('should correctly include user JSON from Blob', async function () {
nock.cleanAll();
let boundary: string | null = null;

const request = nock(API_URL)
.matchHeader('Content-Type', (header) => {
boundary = `--${header.match(/boundary=([^\n]*)/)?.[1]}`;

return true;
})
.post('/jobs/users-imports', (body) => {
const parts = extractParts(body, boundary);

expect(parts.users).toContain('Content-Type: application/json');

// Validate the content of the users JSON.
const users = JSON.parse(parts.users.split('\r\n').slice(-1)[0]);
expect(users.length).toBe(2);
expect(users[0].email).toBe('[email protected]');

return true;
})
.reply(200, {});

const Blob = await getBlobCls();
await jobs
.importUsers({
...data,
users: new Blob([fs.readFileSync(usersFilePath)], {
type: 'application/json',
}),
})
.then(() => {
expect(request.isDone()).toBe(true);
});
});
});

describe('#exportUsers', () => {
Expand Down
3 changes: 3 additions & 0 deletions test/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,6 @@ import nock from 'nock';
const { back: nockBack } = nock;

nockBack.fixtures = path.dirname(fileURLToPath(import.meta.url));

// @ts-expect-error Falling back to node-fetch because nock doesn't work with native fetch.
delete globalThis.fetch;
31 changes: 31 additions & 0 deletions v4_MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -356,3 +356,34 @@ await users.deleteRoles({ id: 'user' }, { roles: ['read:users'] });
| `organizations.removeMemberRoles` | `organizations.deleteMemberRoles` |

</details>

### Import users now takes a Blob

#### Before

```js
await management.jobs.importUsers({
users: fs.createReadStream('./myusers.json'),
connection_id: 'con_123',
});
```

#### After

```js
await management.jobs.importUsers({
users: new Blob[(fs.readFileSync('./myusers.json'), { type: 'application/json' })](),
adamjmcgrath marked this conversation as resolved.
Show resolved Hide resolved
connection_id: 'con_123',
});
```

If you are on Node 16, or you don't want to read the whole file into memory, you can use a library like [fetch-blob](https://github.com/node-fetch/fetch-blob).

```js
import { fileFrom } from 'fetch-blob/from.js';

await management.jobs.importUsers({
users: await fileFrom('./myusers.json', 'application/json'),
connection_id: 'con_123',
});
```