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

chore(deps): drop node-fetch dependency #3217

Merged
merged 6 commits into from
Aug 23, 2024
Merged
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
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 @@ -75,7 +75,6 @@
"jszip": "3.10.1",
"mkdirp": "3.0.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