Skip to content

Commit

Permalink
chore(deps): drop node-fetch dependency (#3217)
Browse files Browse the repository at this point in the history
Co-authored-by: Luca Greco <[email protected]>
  • Loading branch information
fregante and rpl authored Aug 23, 2024
1 parent 19694d2 commit b438d60
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 135 deletions.
127 changes: 0 additions & 127 deletions package-lock.json

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

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@
"jose": "5.6.3",
"jszip": "3.10.1",
"multimatch": "6.0.0",
"node-fetch": "3.3.2",
"node-notifier": "10.0.1",
"open": "9.1.0",
"parse-json": "7.1.1",
Expand Down
37 changes: 32 additions & 5 deletions src/util/submit-addon.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { basename } from 'path';
import { createHash } from 'crypto';
import { createWriteStream, promises as fsPromises } from 'fs';
import { createWriteStream, promises as fsPromises, readFileSync } from 'fs';
import { promises as streamPromises } from 'stream';

// eslint-disable-next-line no-shadow
import fetch, { FormData, fileFromSync } from 'node-fetch';
import { SignJWT } from 'jose';
import JSZip from 'jszip';
import { HttpsProxyAgent } from 'https-proxy-agent';
Expand All @@ -15,6 +14,29 @@ const log = createLogger(import.meta.url);

export const defaultAsyncFsReadFile = fsPromises.readFile;

// Used by fileFromSync method to make sure the form-data entry will
// include a filename derived from the file path.
//
// TODO: Get rid of this hack when we will bump the web-ext nodejs
// version required to nodejs v20 (where the native File constructor
// exists).
export class FileBlob extends Blob {
#name = '';

constructor(fileBits, fileName, options) {
super(fileBits, options);
this.#name = String(fileName);
}

get name() {
return this.#name;
}

get [Symbol.toStringTag]() {
return 'File';
}
}

export class JwtApiAuth {
#apiKey;
#apiSecret;
Expand Down Expand Up @@ -88,8 +110,13 @@ export default class Client {
this.userAgentString = userAgentString;
}

fileFromSync(path) {
return fileFromSync(path);
fileFromSync(filePath) {
// create a File blob from a file path, and ensure it to have the file path basename
// as the associated filename, the AMO server API will be checking it on the form-data
// submitted and fail with the error message:
// "Unsupported file type, please upload a supported file (.crx, .xpi, .zip)."
const fileData = readFileSync(filePath);
return new FileBlob([fileData], basename(filePath));
}

nodeFetch(url, { method, headers, body, agent }) {
Expand Down
36 changes: 34 additions & 2 deletions tests/unit/test-util/test.submit-addon.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import { assert, expect } from 'chai';
import JSZip from 'jszip';
import { afterEach, before, beforeEach, describe, it } from 'mocha';
import * as sinon from 'sinon';
// eslint-disable-next-line no-shadow
import { File, FormData, Response } from 'node-fetch';

import { AMO_BASE_URL } from '../../../src/program.js';
import Client, {
Expand All @@ -18,6 +16,9 @@ import Client, {
saveIdToFile,
saveUploadUuidToFile,
signAddon,

// eslint-disable-next-line no-shadow -- Not actually available under Node 20. Use global once possible.
FileBlob as File,
} from '../../../src/util/submit-addon.js';
import { withTempDir } from '../../../src/util/temp-dir.js';

Expand All @@ -27,6 +28,19 @@ class JSONResponse extends Response {
}
}

// Used to test responses with status < 100 (nodejs native constructor
// enforces status to be in the 200-599 range and throws if it is not).
class BadResponse extends Response {
constructor(data, fakeStatus) {
super(data);
this.fakeStatus = fakeStatus;
}

get status() {
return this.fakeStatus;
}
}

const mockNodeFetch = (nodeFetchStub, url, method, responses) => {
// Trust us... You don't want to know why... but if you really do like nightmares
// take a look to the details and links kindly provided in this comment
Expand All @@ -42,6 +56,9 @@ const mockNodeFetch = (nodeFetchStub, url, method, responses) => {
for (let i = 0; i < responses.length; i++) {
const { body, status } = responses[i];
stubMatcher.onCall(i).callsFake(async () => {
if (status < 200) {
return new BadResponse(body, status);
}
if (typeof body === 'string') {
return new Response(body, { status });
}
Expand Down Expand Up @@ -342,6 +359,21 @@ describe('util.submit-addon', () => {
assert.equal(client.apiUrl.href, new URL(`${cleanUrl}/addons/`).href);
});

describe('fileFromSync', () => {
it('should return a File with name set to the file path basename', () =>
withTempDir(async (tmpDir) => {
const client = new Client(clientDefaults);
const FILE_BASENAME = 'testfile.txt';
const FILE_CONTENT = 'somecontent';
const filePath = path.join(tmpDir.path(), FILE_BASENAME);
await fsPromises.writeFile(filePath, FILE_CONTENT);
const fileRes = client.fileFromSync(filePath);
assert.equal(fileRes.name, FILE_BASENAME);
assert.equal(await fileRes.text(), FILE_CONTENT);
assert.equal(String(fileRes), '[object File]');
}));
});

describe('getPreviousUuidOrUploadXpi', () => {
it('calls doUploadSubmit if previous hash is different to current', async () => {
const oldHash = 'some-hash';
Expand Down

0 comments on commit b438d60

Please sign in to comment.